From 03dfdebc8e562b7954eeddc459b51d52b0b2d15d Mon Sep 17 00:00:00 2001 From: Glenn Rice Date: Wed, 29 Nov 2023 07:44:17 -0600 Subject: [PATCH] Implement suggestions made in my review of openwebwork/webwork2#2266 This uses `flex` rules to make the category input groups behave better. The categories are wrapped in a div with fixed width. The width is chosen to be as small as possible such that the input groups do not wrap. Thus this dispenses with trying to make the category widths match the scrolling record list which really wasn't working. `flex-wrap` is used with "around" justification. So at extra large window widths all three categories will be in the same row. However, when the categories don't fit, they wrap. The justification gives nicer positioning in any case. I merged the input texts, removed capitalization, and changed some of the wording in a few cases. For example, I don't think that the "Library Browser Exercises" is good. I think the reference to the library browser is unnecessary, and using "exercises" instead of "problems" is inconsistent the wording used everywhere else in webwork2. The other changes made are more explicit in the comments made in my review. --- .../Instructor/ProblemSetDetail.pm | 6 +- lib/WeBWorK/HTML/ScrollingRecordList.pm | 1 + lib/WeBWorK/Utils/FilterRecords.pm | 4 +- lib/WeBWorK/Utils/FormatRecords.pm | 32 +- .../ContentGenerator/Instructor/Index.html.ep | 356 ++++++++---------- .../Instructor/SendMail/main_form.html.ep | 13 +- templates/HelpFiles/Hardcopy.html.ep | 2 +- .../HelpFiles/InstructorAssigner.html.ep | 2 +- templates/HelpFiles/InstructorIndex.html.ep | 6 +- .../HelpFiles/InstructorSendMail.html.ep | 2 +- 10 files changed, 200 insertions(+), 224 deletions(-) diff --git a/lib/WeBWorK/ContentGenerator/Instructor/ProblemSetDetail.pm b/lib/WeBWorK/ContentGenerator/Instructor/ProblemSetDetail.pm index 5f2085c5c1..c44e665417 100644 --- a/lib/WeBWorK/ContentGenerator/Instructor/ProblemSetDetail.pm +++ b/lib/WeBWorK/ContentGenerator/Instructor/ProblemSetDetail.pm @@ -16,8 +16,6 @@ package WeBWorK::ContentGenerator::Instructor::ProblemSetDetail; use Mojo::Base 'WeBWorK::ContentGenerator', -signatures; -use Exporter qw(import); - =head1 NAME WeBWorK::ContentGenerator::Instructor::ProblemSetDetail - Edit general set and @@ -25,10 +23,12 @@ specific user/set information as well as problem information =cut +use Exporter qw(import); + use WeBWorK::Utils qw(cryptPassword jitar_id_to_seq seq_to_jitar_id x format_set_name_internal format_set_name_display); use WeBWorK::Utils::Instructor qw(assignProblemToAllSetUsers addProblemToSet); -our @EXPORT_OK = ('FIELD_PROPERTIES'); +our @EXPORT_OK = qw(FIELD_PROPERTIES); # These constants determine which fields belong to what type of record. use constant SET_FIELDS => [ diff --git a/lib/WeBWorK/HTML/ScrollingRecordList.pm b/lib/WeBWorK/HTML/ScrollingRecordList.pm index 675f14d5e1..1878d99826 100644 --- a/lib/WeBWorK/HTML/ScrollingRecordList.pm +++ b/lib/WeBWorK/HTML/ScrollingRecordList.pm @@ -75,6 +75,7 @@ sub scrollingRecordList ($options, @records) { } $formattedRecords = formatRecords( + $c, $c->param("$name!format") || $options{default_format}, sortRecords( $c->param("$name!sort") || $options{default_sort} || (@$sorts ? $sorts->[0][1] : ''), diff --git a/lib/WeBWorK/Utils/FilterRecords.pm b/lib/WeBWorK/Utils/FilterRecords.pm index 7b569e59fe..a55b9f1308 100644 --- a/lib/WeBWorK/Utils/FilterRecords.pm +++ b/lib/WeBWorK/Utils/FilterRecords.pm @@ -51,7 +51,7 @@ use warnings; use Carp; use WeBWorK::Utils qw(sortByName); -use WeBWorK::ContentGenerator::Instructor::ProblemSetDetail qw/FIELD_PROPERTIES/; +use WeBWorK::ContentGenerator::Instructor::ProblemSetDetail qw(FIELD_PROPERTIES); our @EXPORT_OK = qw( getFiltersForClass @@ -118,7 +118,7 @@ sub getFiltersForClass { if (keys %visibles > 1) { for my $vis (sortByName(undef, keys %visibles)) { - push @filters, [ ($vis ? 'Visible' : "Not Visible") => "visible:$vis" ]; + push @filters, [ ($vis ? 'Visible' : 'Not Visible') => "visible:$vis" ]; } } } diff --git a/lib/WeBWorK/Utils/FormatRecords.pm b/lib/WeBWorK/Utils/FormatRecords.pm index b0dd899aaa..0da65077ad 100644 --- a/lib/WeBWorK/Utils/FormatRecords.pm +++ b/lib/WeBWorK/Utils/FormatRecords.pm @@ -55,7 +55,7 @@ use warnings; use Carp; -use WeBWorK::Utils qw/format_set_name_display formatDateTime/; +use WeBWorK::Utils qw/format_set_name_display/; use WeBWorK::ContentGenerator::Instructor::ProblemSetDetail qw/FIELD_PROPERTIES/; our @EXPORT_OK = qw( @@ -115,9 +115,9 @@ use constant PRESET_FORMATS => { field_order => [qw/assignment_type set_id due_date/], format_function => sub { join('', - FIELD_PROPERTIES()->{assignment_type}{labels}{ $_[0] }, - ': ', format_set_name_display($_[1]), - ', ', formatDateTime($_[2])); + FIELD_PROPERTIES()->{assignment_type}{labels}{ $_[1] }, + ': ', format_set_name_display($_[2]), + ', ', $_[0]->formatDateTime($_[3])); } } ], @@ -126,7 +126,7 @@ use constant PRESET_FORMATS => { name => 'due_date: set_id', field_order => [qw/due_date set_id/], format_function => sub { - join('', formatDateTime($_[0]), ': ', format_set_name_display($_[1])); + join('', $_[0]->formatDateTime($_[1]), ': ', format_set_name_display($_[2])); } } ], @@ -134,7 +134,9 @@ use constant PRESET_FORMATS => { 'sid' => { name => 'set_id', field_order => [qw/set_id/], - format_function => \&format_set_name_display + format_function => sub { + return format_set_name_display($_[1]); + } } ], ], @@ -173,7 +175,7 @@ sub getFormatsForClass { return \@presets; } -=item formatRecords($preset_format, @records) +=item formatRecords($c, $preset_format, @records) Given the name of a preset format and an array of database records, returns a reference to a list of two element lists. The first element in each list is a @@ -181,15 +183,16 @@ formatted string representing the record, and the second element is a string that is obtained by joining the key fields of the database record with exclamation marks. -The C<$preset_format> must be provided. It must either be one of the presets -defined above, or the name of a field in the database record class. +The arguments C<$c> and C<$preset_format> must be provided. C<$c> must be a +C object, and C<$preset_format> must either be one of the +presets defined above, or the name of a field in the database record class. =back =cut sub formatRecords { - my ($preset_format, @records) = @_; + my ($c, $preset_format, @records) = @_; return unless @records; @@ -233,8 +236,13 @@ sub formatRecords { if ($format_function) { # If a format_function was passed, then call it on each record. for my $value (@records) { - push(@formattedRecords, - [ $format_function->(map { $value->$_ } @field_order), join('!', map { $value->$_ } @keyfields) ]); + push( + @formattedRecords, + [ + $format_function->($c, map { $value->$_ } @field_order), + join('!', map { $value->$_ } @keyfields) + ] + ); } } else { # Otherwise, use sprintf and format_string. diff --git a/templates/ContentGenerator/Instructor/Index.html.ep b/templates/ContentGenerator/Instructor/Index.html.ep index ddb4941c88..31e261601e 100644 --- a/templates/ContentGenerator/Instructor/Index.html.ep +++ b/templates/ContentGenerator/Instructor/Index.html.ep @@ -16,13 +16,13 @@ . 'or select a tool from the list to the left.' ) =%>
- <%= maketext('Select user(s) and/or sets(s) below and click the action button of your choice.') =%> + <%= maketext('Select user(s) and/or set(s) below and click the action button of your choice.') =%>

% <%= form_for current_route, method => 'POST', id => 'instructor-tools-form', begin =%> <%= $c->hidden_authen_fields =%> % -
+
<%= label_for selected_users => maketext('Users') %>
<%= scrollingRecordList( @@ -59,200 +59,168 @@ ) =%>
-
-
-
-
- <%= submit_button maketext('Edit'), - name => 'sets_assigned_to_user', - class => 'btn btn-sm btn-secondary col-3', - data => { users_needed => 'exactly one', error_users => maketext($E_ONE_USER) } =%> - \ - <%== maketext('Assignments and Dates') =%>\ - - \ - <%== maketext('for one user') =%>\ - +
+ % # TODO: When bootstrap is upgraded to 5.3 switch the "column-gap" style to the "column-gap-3" class. +
+
+
+
+ <%= submit_button maketext('Edit'), + name => 'sets_assigned_to_user', + class => 'btn btn-sm btn-secondary w-25', + data => { users_needed => 'exactly one', error_users => maketext($E_ONE_USER) } =%> + \ + <%== maketext('assignments and dates for one user') =%>\ + +
+
+ <%= submit_button maketext('Edit'), + name => 'edit_users', + class => 'btn btn-sm btn-secondary w-25', + formaction => $c->systemLink(url_for 'instructor_user_list'), + data => { users_needed => 'at least one', error_users => maketext($E_MIN_ONE_USER) } =%> + \ + <%== maketext('account data for all selected users') =%>\ + +
+
+ <%= submit_button maketext('Edit'), + name => 'user_options', + class => 'btn btn-sm btn-secondary w-25', + data => { users_needed => 'exactly one', error_users => maketext($E_ONE_USER) } =%> + \ + <%== maketext('account settings for one user') =%>\ + +
+
+ <%= submit_button maketext('View'), + name => 'user_progress', + class => 'btn btn-sm btn-secondary w-25', + data => { users_needed => 'exactly one', error_users => maketext($E_ONE_USER) } =%> + \ + <%== maketext('progress for one user') =%>\ + +
+
+ <%= submit_button maketext('Add'), name => 'add_users', class => 'btn btn-sm btn-secondary w-25' =%> + <%= number_field number_of_students => 1, min => 1, max => 100, + class => 'form-control form-control-sm text-center' =%> + \ + <%== maketext('new user accounts') =%>\ + +
-
- <%= submit_button maketext('Edit'), - name => 'edit_users', - class => 'btn btn-sm btn-secondary col-3', - formaction => $c->systemLink(url_for 'instructor_user_list'), - data => { users_needed => 'at least one', error_users => maketext($E_MIN_ONE_USER) } =%> - \ - <%== maketext('Account Data') =%>\ - - \ - <%== maketext('for all selected users') =%>\ - +
+
+
+ <%= submit_button maketext('Assign'), + # This name is the same as the name of the submit button in Assigner.pm and the form is + # directly submitted to that module without modification. + name => 'assign', + class => 'btn btn-sm btn-secondary w-25', + formaction => $c->systemLink(url_for 'instructor_set_assigner'), + data => { + users_needed => 'at least one', + error_users => maketext($E_MIN_ONE_USER), + sets_needed => 'at least one', + error_sets => maketext($E_MIN_ONE_SET) + } =%> + \ + <%== maketext('all selected users to all selected sets') =%>\ + +
+
+ <%= submit_button maketext('Act'), + name => 'act_as_user', + class => 'btn btn-sm btn-secondary w-25', + data => { + users_needed => 'exactly one', + error_users => maketext($E_ONE_USER), + sets_needed => 'at most one', + error_sets => maketext($E_MAX_ONE_SET) + } =%> + \ + <%== maketext('as one user on up to one set') =%>\ + +
+
+ <%= submit_button maketext('Edit'), + name => 'edit_set_for_users', + class => 'btn btn-sm btn-secondary w-25', + data => { + sets_needed => 'exactly one', + error_sets => maketext($E_ONE_SET) + } =%> + \ + <%== maketext(q{one set's details for some or all users}) =%>\ + +
+
+ <%= submit_button maketext('Score'), + name => 'score_sets', + class => 'btn btn-sm btn-secondary w-25', + formaction => $c->systemLink(url_for 'instructor_scoring'), + data => { sets_needed => 'at least one', error_sets => maketext($E_MIN_ONE_SET) } =%> + \ + <%== maketext('all selected sets for all users') =%>\ + +
-
- <%= submit_button maketext('Edit'), - name => 'user_options', - class => 'btn btn-sm btn-secondary col-3', - data => { users_needed => 'exactly one', error_users => maketext($E_ONE_USER) } =%> - \ - <%== maketext('Account Settings') =%>\ - - \ - <%== maketext('for one user') =%>\ - -
-
- <%= submit_button maketext('View'), - name => 'user_progress', - class => 'btn btn-sm btn-secondary col-3', - data => { users_needed => 'exactly one', error_users => maketext($E_ONE_USER) } =%> - \ - <%== maketext('Progress') =%>\ - - \ - <%== maketext('for one user') =%>\ - -
-
- <%= submit_button maketext('Add'), name => 'add_users', class => 'btn btn-sm btn-secondary col-3' =%> - %= number_field number_of_students => 1, min => 1, max => 100, class => 'col-2 flex-grow-1 text-center' - \ - <%== maketext('new user accounts') =%>\ - -
-
-
-
-
- <%= submit_button maketext('Assign'), - # This name is the same as the name of the submit button in Assigner.pm and the form is - # directly submitted to that module without modification. - name => 'assign', - class => 'btn btn-sm btn-secondary col-2', - formaction => $c->systemLink(url_for 'instructor_set_assigner'), - data => { - users_needed => 'at least one', - error_users => maketext($E_MIN_ONE_USER), - sets_needed => 'at least one', - error_sets => maketext($E_MIN_ONE_SET) - } =%> - \ - <%== maketext('all selected users') =%>\ - - \ - <%== maketext('to all selected sets') =%>\ - -
-
- <%= submit_button maketext('Act'), - name => 'act_as_user', - class => 'btn btn-sm btn-secondary col-2', - data => { - users_needed => 'exactly one', - error_users => maketext($E_ONE_USER), - sets_needed => 'at most one', - error_sets => maketext($E_MAX_ONE_SET) - } =%> - \ - <%== maketext('as one user') =%>\ - - \ - <%== maketext('on up to one set') =%>\ - -
-
- <%= submit_button maketext('Edit'), - name => 'edit_set_for_users', - class => 'btn btn-sm btn-secondary col-2', - data => { - sets_needed => 'exactly one', - error_sets => maketext($E_ONE_SET) - } =%> - \ - <%== maketext('Set Detail for one set') =%>\ - - \ - <%== maketext('for zero or more users') =%>\ - -
-
- <%= submit_button maketext('Score'), - name => 'score_sets', - class => 'btn btn-sm btn-secondary col-2', - formaction => $c->systemLink(url_for 'instructor_scoring'), - data => { sets_needed => 'at least one', error_sets => maketext($E_MIN_ONE_SET) } =%> - \ - <%== maketext('all selected sets') =%>\ - - \ - <%== maketext(' for all users') =%>\ - -
-
-
-
-
- <%= submit_button maketext('Edit'), - name => 'users_assigned_to_set', - class => 'btn btn-sm btn-secondary col-3', - data => { sets_needed => 'exactly one', error_sets => maketext($E_ONE_SET) } =%> - \ - <%== maketext('Assigned Users') =%>\ - - \ - <%== maketext('for one set') =%>\ - -
-
- <%= submit_button maketext('Add'), - name => 'prob_lib', - class => 'btn btn-sm btn-secondary col-3', - data => { sets_needed => 'exactly one', error_sets => maketext($E_ONE_SET) } =%> - \ - <%== maketext('Library Browser Exercises') =%>\ - - \ - <%== maketext('to one set') =%>\ - -
-
- <%= submit_button maketext('View'), - name => 'set_stats', - class => 'btn btn-sm btn-secondary col-3', - data => { sets_needed => 'exactly one', error_sets => maketext($E_ONE_SET) } =%> - \ - <%== maketext('Statistics') =%>\ - - \ - <%== maketext('for one set') =%>\ - -
-
- <%= submit_button maketext('View'), - name => 'set_progress', - class => 'btn btn-sm btn-secondary col-3', - data => { sets_needed => 'exactly one', error_sets => maketext($E_ONE_SET) } =%> - \ - <%== maketext('Progress') =%>\ - - \ - <%== maketext('for one set') =%>\ - -
-
- <%= submit_button maketext('Create'), - name => 'create_set', - class => 'btn btn-sm btn-secondary col-3', - data => { - set_name_needed => 'true', - error_set_name => maketext($E_SET_NAME), - error_invalid_set_name => maketext($E_BAD_NAME) - } =%> - <%= label_for new_set_name => maketext('New Set:'), class => 'input-group-text' =%> - <%= text_field new_set_name => '', - id => 'new_set_name', - placeholder => maketext('Name for new set here'), - size => 20, - class => 'form-control form-control-sm', - dir => 'ltr' =%> +
+
+
+ <%= submit_button maketext('Edit'), + name => 'users_assigned_to_set', + class => 'btn btn-sm btn-secondary w-25', + data => { sets_needed => 'exactly one', error_sets => maketext($E_ONE_SET) } =%> + \ + <%== maketext('assigned users for one set') =%>\ + +
+
+ <%= submit_button maketext('Add'), + name => 'prob_lib', + class => 'btn btn-sm btn-secondary w-25', + data => { sets_needed => 'exactly one', error_sets => maketext($E_ONE_SET) } =%> + \ + <%== maketext('problems to one set') =%>\ + +
+
+ <%= submit_button maketext('View'), + name => 'set_stats', + class => 'btn btn-sm btn-secondary w-25', + data => { sets_needed => 'exactly one', error_sets => maketext($E_ONE_SET) } =%> + \ + <%== maketext('statistics for one set') =%>\ + +
+
+ <%= submit_button maketext('View'), + name => 'set_progress', + class => 'btn btn-sm btn-secondary w-25', + data => { sets_needed => 'exactly one', error_sets => maketext($E_ONE_SET) } =%> + \ + <%== maketext('progress for one set') =%>\ + +
+
+ <%= submit_button maketext('Create'), + name => 'create_set', + class => 'btn btn-sm btn-secondary w-25', + data => { + set_name_needed => 'true', + error_set_name => maketext($E_SET_NAME), + error_invalid_set_name => maketext($E_BAD_NAME) + } =%> + <%= label_for new_set_name => maketext('new set:'), class => 'input-group-text' =%> + <%= text_field new_set_name => '', + id => 'new_set_name', + placeholder => maketext('New set name'), + size => 20, + class => 'form-control form-control-sm', + dir => 'ltr' =%> +
diff --git a/templates/ContentGenerator/Instructor/SendMail/main_form.html.ep b/templates/ContentGenerator/Instructor/SendMail/main_form.html.ep index 719d408da8..aa0b86d1a1 100644 --- a/templates/ContentGenerator/Instructor/SendMail/main_form.html.ep +++ b/templates/ContentGenerator/Instructor/SendMail/main_form.html.ep @@ -126,13 +126,12 @@
<%= scrollingRecordList( { - name => 'classList', - controller => $c, - default_sort => 'lnfn', - default_format => 'lnfn_uid', - default_filters => ['all'], - refresh_button_name => maketext('Change Display Settings'), - attrs => { size => 5, multiple => undef } + name => 'classList', + controller => $c, + default_sort => 'lnfn', + default_format => 'lnfn_uid', + default_filters => ['all'], + attrs => { size => 5, multiple => undef } }, @{ $c->{ra_user_records} } ) =%> diff --git a/templates/HelpFiles/Hardcopy.html.ep b/templates/HelpFiles/Hardcopy.html.ep index e1ecb92c74..289765d6e3 100644 --- a/templates/HelpFiles/Hardcopy.html.ep +++ b/templates/HelpFiles/Hardcopy.html.ep @@ -23,7 +23,7 @@

<%= maketext('In both cases, one can select the sort field, the format of the display list, and the filter. ' - . '"Change Display Settings" must be clicked to update the list.') =%> + . '"Refresh List" must be clicked to update the list.') =%>

<%= maketext('There are many options available at the bottom:') =%> diff --git a/templates/HelpFiles/InstructorAssigner.html.ep b/templates/HelpFiles/InstructorAssigner.html.ep index 6a9fceef6b..22745a7b11 100644 --- a/templates/HelpFiles/InstructorAssigner.html.ep +++ b/templates/HelpFiles/InstructorAssigner.html.ep @@ -21,7 +21,7 @@ . 'unassign the given sets. The list of users or the list of sets can be reordered under the "sort" ' . 'option or the format of the user or set list. Additionally, the set lists can be filtered ' . '(by section number or recitation). The displayed user and set lists will only be updated ' - . 'upon clicking "Change Display Settings".') =%> + . 'upon clicking "Refresh List".') =%>

<%= maketext('Multiple users or sets may be selected by holding down the shift key or the ctrl key while ' diff --git a/templates/HelpFiles/InstructorIndex.html.ep b/templates/HelpFiles/InstructorIndex.html.ep index b3f8ca3d71..b8d816c3aa 100644 --- a/templates/HelpFiles/InstructorIndex.html.ep +++ b/templates/HelpFiles/InstructorIndex.html.ep @@ -21,14 +21,14 @@ . 'common actions that allow selecting multiple users or sets to act on at once. Because multiple users ' . 'and sets can be acted on at the same time, it is often more efficient to use this page when modifying ' . 'multiple items. For example, after selecting several users and a set you can change the close date for ' - . 'all selected users for that set. This page also gives access to "Edit Assignments and Dates for one user", ' + . 'all selected users for that set. This page also gives access to "Edit assignments and dates for one user", ' . 'which can be used to access settings for all sets including test versions for that user.') =%>

<%= maketext('Use the "Users" and "Sets" forms to select which users and sets to act on. Multiple items can be ' . 'selected using ctrl-click or shift-click. The action buttons are grouped into three categories. The ' - . 'first is a set of actions that act on the selected users, the last is a set of actions that act on the ' - . 'selected sets, and the middle is a set of actions for acting on a combination of sets and users. If an ' + . 'first is a set of actions that act on the selected users, the second is a set of actions for acting on a ' + . 'combination of sets and users, and the third is a set of actions that act on the selected sets. If an ' . 'invalid number of users or sets are selected, the action will not be preformed, and an error message ' . 'will be placed on the page.') =%>

diff --git a/templates/HelpFiles/InstructorSendMail.html.ep b/templates/HelpFiles/InstructorSendMail.html.ep index 9ba4e0e322..68d839c564 100644 --- a/templates/HelpFiles/InstructorSendMail.html.ep +++ b/templates/HelpFiles/InstructorSendMail.html.ep @@ -19,7 +19,7 @@

<%= maketext('Use this page to send emails to active (enrolled or auditing) students. Emails can be sent to ' . 'all active students or selected students. Use the "Students" form to sort, filter, or format how ' - . 'the user name is displayed. Click "Change Display Settings" to apply any changes. Use control-click ' + . 'the user name is displayed. Click "Refresh List" to apply any changes. Use control-click ' . 'or shift-click to select multiple students to email.') =%>