Add sort by column header click to the homework sets editor. #2233
Conversation
| @@ -225,12 +219,10 @@ sub pre_header_initialize ($c) { | |||
| if (!FORM_PERMS()->{$actionID} || $authz->hasPermissions($user, FORM_PERMS()->{$actionID})) { | |||
| # Call the action handler | |||
| my $actionHandler = "${actionID}_handler"; | |||
| $c->addgoodmessage($c->maketext('Result of last action performed: [_1]', $c->tag('i', $c->$actionHandler))); | |||
| $c->addgoodmessage($c->$actionHandler); | |||
There was a problem hiding this comment.
I don't think $actionHandler is used elsewhere. Can we simplify these two lines to:
$c->addgoodmessage($c->{"${actionID}_handler"});
There was a problem hiding this comment.
That is a method call, and does not work in the way that you propose.
|
Overall, looks good. A feature request might be to allow a second click on a header to reverse sort by that column with an icon indicating the sort direction. There are many javascript driven tables that do this automatically. Lastly, does it make sense to also have the "Sort" option in the tab list now? I know it does a secondary sort, which the clicking of the headers will not do. |
|
Actually, the header sort moves the last sort methods used down in priority, so if you click on set name, then on due date you will get the table sorted first by due date, then by set name the same as selecting due date for the first sort field and set name for the second sort field in the sort form. The message now tells you that too. Although, it is nice to also have the sort form, as you can achieve the same thing in one submission. |
|
As to a reverse sort, that is more complicated. In this pull request I used the existing sort. There is nothing new with that. Implementing a reverse sort on a second click would take quite a bit of re-engineering of the sort mechanism as it is. |
I was thinking it would be more work. I realize that the sorted column and direction would need to be stored, so it is clear what clicking on a column would accomplish. I'm okay if this isn't added. |
drgrice1
force-pushed
the
table-header-sort
branch
2 times, most recently
from
a87849a to
7b65b76
Compare
|
@pstaabp: You asked for it ... you got it. This now includes the capability to reverse sort columns. This is implemented for both the homework sets editor and the classlist editor. The sort form now has an additional order dropdown for each of the sort priorities to choose if the sort is ascending or descending for that field. If the header of the primary sort field in the table is clicked, then the only change in the sorting is that the order (ascending or descending) of the primary sort is toggled. There is also a button added in each of the table headers corresponding to the fields used for sorting. The button shows a number (1, 2, or 3) indication the sort priority of that field, and a chevron up or down indicating if the sort order is descending or ascending, respectively. Clicking on that button sorts again, but only toggles if the sort for that priority level is ascending or descending. |
drgrice1
force-pushed
the
table-header-sort
branch
4 times, most recently
from
d36c386 to
884f4ad
Compare
This uses the same method as the classlist editor for this. In fact the javascript used by the classlist editor for this is now in the actiontabs.js file, and is used by both the classlist editor and the homework sets editor. The column headers in the homework sets editor did not make sense. The headers should say what is in the column, not what clicking on an extra item in the column does. This was made worse by making it so that clicking on the column header sorts by set name. It doesn't make sends to click on "Edit Set Data" to sort the column by set name. Other headers also were adjusted to remove the "Edit" wording, and reflect contents not actions. We need to find a better way to convey what clicking on something in the column does. A tooltip is added to the pencil icon by the set name to convey what it does. Using the set name header as a label for the select all is problematic. So instead a double check icon is added and a tooltip on the select all input itself. This is also done in the other tables like this (the classlist editor and achievement list). Remove the "Please select action to be performed" messages that were displayed when the homework sets editor, classlist editor, or achievement editor pages were initially loaded. Those messages aren't needed. Furthermore, remove the "Result of last action performed:" prefix that was added to all action messages. That doesn't add anything useful to the message, and the action messages are clear on their own. The messages were improved with capitalization and periods to make them look better when shown without that prefix.
This is implemented for both the homework sets editor and the classlist editor. The sort form now has an additional order dropdown for each of the sort priorities to choose if the sort is ascending or descending for that field. If the header of the primary sort field in the table is clicked, then the only change in the sorting is that the order (ascending or descending) of the primary sort is toggled. There is also a button added in each of the table headers corresponding to the fields used for sorting. The button shows a number (1, 2, or 3) indication the sort priority of that field, and a chevron up or down indicating if the sort order is descending or ascending, respectively. Clicking on that button sorts again, but only toggles if the sort for that priority level is ascending or descending.
drgrice1
force-pushed
the
table-header-sort
branch
from
884f4ad to
652d661
Compare
|
Overall I like this and it works well. Some thoughts I have, first why exclude some columns from being sortable? I assume this is because of what was sortable before this change, so not part of this pull request. Though might be nice to just let all columns be sortable, and then the user can decide if it is useful or not. |
|
Yes, the same columns are sortable that were before. If we want other columns to be sortable, that can be done in another pull request. Note that for most of the columns that are not sortable, implementing sort for them is non trivial. On the problem sets page, the current sorting is done with the database, and that can not be done with the other columns. |
|
I should add that I don't see any real point of sorting on any of the columns on the problem sets page that aren't already sorted. In my opinion no effort should be wasted trying to implement sorting on assigned users or problems. So really all columns worth sorting are already sortable. On the class list editor the same goes for assigned sets. |
|
A future change might be to just move all the sorting to javascript, with some sortable tables setup. But this works well and is a nice addition. |
|
Sorting by JavaScript without a form submission would be nice, but most likely that will have to wait for WeBWorK 3. It can be done, but would take a lot of effort. For WeBWorK 3 it will be much easier with the tools we are using there |
This uses the same method as the classlist editor for this. In fact the javascript used by the classlist editor for this is now in the actiontabs.js file, and is used by both the classlist editor and the homework sets editor.
The column headers in the homework sets editor did not make sense. The headers should say what is in the column, not what clicking on an extra item in the column does. This was made worse by making it so that clicking on the column header sorts by set name. It doesn't make sends to click on "Edit Set Data" to sort the column by set name. Other headers also were adjusted to remove the "Edit" wording, and reflect contents not actions. We need to find a better way to convey what clicking on something in the column does. A tooltip is added to the pencil icon by the set name to convey what it does.
Using the set name header as a label for the select all is problematic. So instead a double check icon is added and a tooltip on the select all input itself. This is also done in the other tables like this (the classlist editor and achievement list).
Remove the "Please select action to be performed" messages that were displayed when the homework sets editor, classlist editor, or achievement editor pages were initially loaded. Those messages aren't needed. Furthermore, remove the "Result of last action performed:" prefix that was added to all action messages. That doesn't add anything useful to the message, and the action messages are clear on their own. The messages were improved with capitalization and periods to make them look better when shown without that prefix.