From ef4053b16b9d2a8cea54605179e5320c6a5c2339 Mon Sep 17 00:00:00 2001 From: Glenn Rice Date: Mon, 26 Jun 2023 22:39:49 -0400 Subject: [PATCH 1/4] Make messages dismissible and PG editor tmp edits more clear Any message that is added by the ContentGenerator addgoodmessage or addbadmessage methods is now dismissible. There is a close button to the right that can be clicked to remove the message from the page. In additiona, a hidden button is also added to the end of the page. If there are any dismissible messages, then the button is shown by javascript, and if clicked all of the dismissible messages are removed from the page (and the button hidden again). This is a convenience for removing all of those messages at once if there are many of them. In the PG editor, when a temporary file is created by javascript the file name is changed to the temporary file name in the "Editing ..." statement and the `temporaryFile` class is added which gives the text the orange temporary file color. Furthermore, the "Editing ..." statement is now shown both above and below the codemirror editor in the page. This makes it more clear that a temporary file is being edited. Also fix a bug introduced by #1918. The PG editor revert form needs to be shown for course information files as well. Otherwise there is no way to revert to the original file once a temporary file is created other than saving the file. @somiaj: What was your rationale for doing this? Also add site nav sub links for set headers and the course information file. Now that the problem editor is directly in the site nav that link is shown as active when editing one of these files. That is confusing because if you click on that "active" link it does not open the page you are currently viewing that is editing one of these files. Instead it opens the editor with a new problem. --- htdocs/js/PGProblemEditor/pgproblemeditor.js | 17 +++++++- htdocs/themes/math4/math4.js | 10 +++++ htdocs/themes/math4/math4.scss | 1 - htdocs/themes/math4/system.html.ep | 3 ++ lib/WeBWorK/ContentGenerator.pm | 30 ++++++++++++- .../Instructor/PGProblemEditor.pm | 2 +- templates/ContentGenerator/Base/links.html.ep | 36 +++++++++++++++- .../Instructor/PGProblemEditor.html.ep | 42 ++++++++++++------- .../PGProblemEditor/revert_form.html.ep | 3 -- 9 files changed, 118 insertions(+), 26 deletions(-) diff --git a/htdocs/js/PGProblemEditor/pgproblemeditor.js b/htdocs/js/PGProblemEditor/pgproblemeditor.js index 18b6d36459..b0dedbf798 100644 --- a/htdocs/js/PGProblemEditor/pgproblemeditor.js +++ b/htdocs/js/PGProblemEditor/pgproblemeditor.js @@ -54,7 +54,16 @@ fetch(webserviceURL, { method: 'post', mode: 'same-origin', body: new URLSearchParams(request_object) }) .then((response) => response.json()) - .then((data) => showMessage(data.server_response, data.result_data)) + .then((data) => { + showMessage(data.server_response, data.result_data); + if (data.result_data) { + // Add the temporary file coloring and change the current file to the saved file. + document.querySelectorAll('.set-file-info').forEach((nfo) => nfo.classList.add('temporaryFile')); + for (const currentFile of document.querySelectorAll('.current-file')) { + currentFile.textContent = currentFile.dataset.tmpFile; + } + } + }) .catch((err) => showMessage(`Error saving temporary file: ${err?.message ?? err}`)); }; @@ -106,6 +115,12 @@ revertRadio.checked = true; } + // Add the temporary file coloring and change the current file to the saved file. + document.querySelectorAll('.set-file-info').forEach((nfo) => nfo.classList.add('temporaryFile')); + for (const currentFile of document.querySelectorAll('.current-file')) { + currentFile.textContent = currentFile.dataset.tmpFile; + } + if (editorForm) editorForm.target = 'WW_View'; } else { e.preventDefault(); diff --git a/htdocs/themes/math4/math4.js b/htdocs/themes/math4/math4.js index 48ea72a8b0..7789d12de3 100644 --- a/htdocs/themes/math4/math4.js +++ b/htdocs/themes/math4/math4.js @@ -120,6 +120,16 @@ const codeshards = document.querySelectorAll('.codeshard'); if (codeshards.length == 1) codeshards[0].setAttribute('aria-label', 'answer'); + const messages = document.querySelectorAll('#message .alert-dismissible, #message_bottom .alert-dismissible'); + if (messages.length) { + const dismissBtn = document.getElementById('dismiss-messages-btn'); + dismissBtn?.classList.remove('d-none'); + dismissBtn?.addEventListener('click', () => { + messages.forEach((message) => message.remove()); + dismissBtn.classList.add('d-none'); + }); + } + // Accessibility // Present the contents of the data-alt attribute as alternative content for screen reader users. // The icon should be formatted as diff --git a/htdocs/themes/math4/math4.scss b/htdocs/themes/math4/math4.scss index c93771cced..1dbb542ec1 100644 --- a/htdocs/themes/math4/math4.scss +++ b/htdocs/themes/math4/math4.scss @@ -447,7 +447,6 @@ h2.page-title { /* Message section */ .message:not(:empty) { - display: inline-block; margin-bottom: 0.5rem; } diff --git a/htdocs/themes/math4/system.html.ep b/htdocs/themes/math4/system.html.ep index b24573539c..c134f16c24 100644 --- a/htdocs/themes/math4/system.html.ep +++ b/htdocs/themes/math4/system.html.ep @@ -164,6 +164,9 @@ % } % if ($c->can('message')) {
<%= $c->message %>
+ % } % % # Footer diff --git a/lib/WeBWorK/ContentGenerator.pm b/lib/WeBWorK/ContentGenerator.pm index 25fd18848f..0aa4e1c328 100644 --- a/lib/WeBWorK/ContentGenerator.pm +++ b/lib/WeBWorK/ContentGenerator.pm @@ -264,7 +264,20 @@ message() template escape handler. =cut sub addgoodmessage ($c, $message) { - $c->addmessage($c->tag('p', class => 'alert alert-success p-1 my-2', $c->b($message))); + $c->addmessage($c->tag( + 'p', + class => 'alert alert-success alert-dismissible ps-1 py-1 my-2', + $c->c( + $message, + $c->tag( + 'button', + type => 'button', + class => 'btn-close p-2', + data => { bs_dismiss => 'alert' }, + 'aria-label' => $c->maketext('Dismiss') + ) + )->join('') + )); return; } @@ -276,7 +289,20 @@ message() template escape handler. =cut sub addbadmessage ($c, $message) { - $c->addmessage($c->tag('p', class => 'alert alert-danger p-1 my-2', $c->b($message))); + $c->addmessage($c->tag( + 'p', + class => 'alert alert-danger alert-dismissible ps-1 py-1 my-2', + $c->c( + $message, + $c->tag( + 'button', + type => 'button', + class => 'btn-close p-2', + data => { bs_dismiss => 'alert' }, + 'aria-label' => $c->maketext('Dismiss') + ) + )->join('') + )); return; } diff --git a/lib/WeBWorK/ContentGenerator/Instructor/PGProblemEditor.pm b/lib/WeBWorK/ContentGenerator/Instructor/PGProblemEditor.pm index 5b761fbc83..8a8f2dc3a1 100644 --- a/lib/WeBWorK/ContentGenerator/Instructor/PGProblemEditor.pm +++ b/lib/WeBWorK/ContentGenerator/Instructor/PGProblemEditor.pm @@ -1211,7 +1211,7 @@ sub revert_handler ($c) { } # Determine revert action - my $revertType = $c->param('action.revert.type') || 'do_not_revert'; + my $revertType = $c->param('action.revert.type') // ''; if ($revertType eq 'revert') { $c->{inputFilePath} = $c->{editFilePath}; diff --git a/templates/ContentGenerator/Base/links.html.ep b/templates/ContentGenerator/Base/links.html.ep index 35bf85db76..d0356369dc 100644 --- a/templates/ContentGenerator/Base/links.html.ep +++ b/templates/ContentGenerator/Base/links.html.ep @@ -135,9 +135,11 @@ % } - % # Problem Editor -
  • <%= $makelink->('instructor_problem_editor') %>
    • +
  • + <%= $makelink->('instructor_problem_editor', + param('file_type') && param('file_type') eq 'course_info' ? (active => 0) : ()) %> +
    • % if (defined $prettySetID && defined $problemID) {
    • + % } elsif (defined $prettySetID && param('file_type') && param('file_type') =~ /^(set|hardcopy)_header$/) { +
  • + +
    • + % } elsif (param('file_type') && param('file_type') eq 'course_info') { +
  • + +
    • % } % # Library Browser
  • <%= $makelink->('instructor_set_maker') %>
    • diff --git a/templates/ContentGenerator/Instructor/PGProblemEditor.html.ep b/templates/ContentGenerator/Instructor/PGProblemEditor.html.ep index 780674f320..3819a4d645 100644 --- a/templates/ContentGenerator/Instructor/PGProblemEditor.html.ep +++ b/templates/ContentGenerator/Instructor/PGProblemEditor.html.ep @@ -25,7 +25,7 @@ % } % % my %titles = ( - % blank_problem => x('Editing new problem template.'), + % blank_problem => x('Editing new problem template "[_1]".'), % set_header => x('Editing set header file "[_1]".'), % hardcopy_header => x('Editing hardcopy header file "[_1]".'), % course_info => x('Editing course information file "[_1]".'), @@ -36,23 +36,32 @@ % my $setName = stash('setID') // ''; % my $fullSetName = $c->{fullSetID} // $setName; % -% my $header = begin - - <%== $c->{file_type} eq 'problem' - ? maketext( - 'Editing problem [_1] of set [_2] in file "[_3]".', - $c->{prettyProblemNumber}, - tag('span', dir => 'ltr', format_set_name_display($fullSetName)), - tag('span', dir => 'ltr', $c->shortPath($c->{inputFilePath})) - ) - : maketext($titles{ $c->{file_type} }, $c->shortPath($c->{inputFilePath})) =%> - +% my $fileInfo = begin +
    + <%= tag 'div', class => 'set-file-info' . ($c->isTempEditFilePath($c->{inputFilePath}) ? ' temporaryFile' : ''), + begin =%> + + <%== $c->{file_type} eq 'problem' + ? maketext( + 'Editing problem [_1] of set [_2] in file "[_3]".', + $c->{prettyProblemNumber}, + tag('span', dir => 'ltr', format_set_name_display($fullSetName)), + tag('span', dir => 'ltr', class => 'current-file', + data => { tmp_file => $c->shortPath($c->{tempFilePath}) }, + $c->shortPath($c->{inputFilePath})) + ) + : maketext( + $titles{ $c->{file_type} }, + tag('span', dir => 'ltr', class => 'current-file', + data => { tmp_file => $c->shortPath($c->{tempFilePath}) }, + $c->shortPath($c->{inputFilePath})) + ) =%> + + <% end =%> +
    % end -% $header = $c->isTempEditFilePath($c->{inputFilePath}) - % ? tag('div', class => 'temporaryFile', $header) # Use colors if this is a temporary file. - % : $header->(); +<%= $fileInfo->() %> % -
    <%= $header %>
    <%= form_for current_route, method => 'POST', id => 'editor', name => 'editor', enctype => 'application/x-www-form-urlencoded', class => 'col-12', begin =%> <%= $c->hidden_authen_fields =%> @@ -148,6 +157,7 @@ <%= generate_codemirror_controls_html($c) =%> + <%= $fileInfo->() %> % % # Output action forms % my $default_choice; diff --git a/templates/ContentGenerator/Instructor/PGProblemEditor/revert_form.html.ep b/templates/ContentGenerator/Instructor/PGProblemEditor/revert_form.html.ep index 7e40822797..e96f1f7d89 100644 --- a/templates/ContentGenerator/Instructor/PGProblemEditor/revert_form.html.ep +++ b/templates/ContentGenerator/Instructor/PGProblemEditor/revert_form.html.ep @@ -1,6 +1,3 @@ -% # Don't show the revert form when editing an existing course info file. -% next if $c->{file_type} eq 'course_info'; -% % if (!-r $c->{editFilePath}) { <%== maketext( 'Error: The original file [_1] cannot be read.', From 804ff9b6d5352085a5b6e582c78d38b22f53f79f Mon Sep 17 00:00:00 2001 From: Glenn Rice Date: Wed, 28 Jun 2023 16:04:37 -0400 Subject: [PATCH 2/4] Add the fade and show classes to the alerts to get animation when they are dismissed. Also make sure that the dismiss button is removed when all alerts have been dismissed. --- htdocs/themes/math4/math4.js | 16 ++++++++++++---- lib/WeBWorK/ContentGenerator.pm | 4 ++-- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/htdocs/themes/math4/math4.js b/htdocs/themes/math4/math4.js index 7789d12de3..b180770fc9 100644 --- a/htdocs/themes/math4/math4.js +++ b/htdocs/themes/math4/math4.js @@ -124,10 +124,18 @@ if (messages.length) { const dismissBtn = document.getElementById('dismiss-messages-btn'); dismissBtn?.classList.remove('d-none'); - dismissBtn?.addEventListener('click', () => { - messages.forEach((message) => message.remove()); - dismissBtn.classList.add('d-none'); - }); + + // Hide the dismiss button when the last alert is dismissed. + for (const message of messages) { + message.addEventListener('closed.bs.alert', () => { + if (!document.querySelector('#message .alert-dismissible, #message_bottom .alert-dismissible')) + dismissBtn.remove(); + }, { once: true }); + } + + dismissBtn?.addEventListener('click', () => + messages.forEach((message) => bootstrap.Alert.getOrCreateInstance(message)?.close()) + ); } // Accessibility diff --git a/lib/WeBWorK/ContentGenerator.pm b/lib/WeBWorK/ContentGenerator.pm index 0aa4e1c328..d45126d004 100644 --- a/lib/WeBWorK/ContentGenerator.pm +++ b/lib/WeBWorK/ContentGenerator.pm @@ -266,7 +266,7 @@ message() template escape handler. sub addgoodmessage ($c, $message) { $c->addmessage($c->tag( 'p', - class => 'alert alert-success alert-dismissible ps-1 py-1 my-2', + class => 'alert alert-success alert-dismissible fade show ps-1 py-1 my-2', $c->c( $message, $c->tag( @@ -291,7 +291,7 @@ message() template escape handler. sub addbadmessage ($c, $message) { $c->addmessage($c->tag( 'p', - class => 'alert alert-danger alert-dismissible ps-1 py-1 my-2', + class => 'alert alert-danger alert-dismissible fade show ps-1 py-1 my-2', $c->c( $message, $c->tag( From 9e9f875b613408a31e9ea99b24de6edad3ccec89 Mon Sep 17 00:00:00 2001 From: Glenn Rice Date: Wed, 28 Jun 2023 16:15:08 -0400 Subject: [PATCH 3/4] Tweak styles a bit. --- htdocs/themes/math4/math4.scss | 2 +- htdocs/themes/math4/system.html.ep | 8 +++++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/htdocs/themes/math4/math4.scss b/htdocs/themes/math4/math4.scss index 1dbb542ec1..648b8322bb 100644 --- a/htdocs/themes/math4/math4.scss +++ b/htdocs/themes/math4/math4.scss @@ -447,7 +447,7 @@ h2.page-title { /* Message section */ .message:not(:empty) { - margin-bottom: 0.5rem; + display: inline-block; } .font-visible { font-weight: bold; } diff --git a/htdocs/themes/math4/system.html.ep b/htdocs/themes/math4/system.html.ep index c134f16c24..60f1afef87 100644 --- a/htdocs/themes/math4/system.html.ep +++ b/htdocs/themes/math4/system.html.ep @@ -164,9 +164,11 @@ % } % if ($c->can('message')) {
    <%= $c->message %>
    - +
    + +
    % } % % # Footer From b0742af254b7285eb3d0a1290e11e4671f2a5238 Mon Sep 17 00:00:00 2001 From: Glenn Rice Date: Thu, 29 Jun 2023 17:51:24 -0500 Subject: [PATCH 4/4] Remove margins on all elements added in `addmessage` calls. The `.message` divs in the main layout (system.html.ep) now is display flex with a column flex direction and a 0.25rem gap. Margins are added to the bottom of that div (when it is non empty). Also remplace more of the `addmessage` calls with `addgooodmessage` or `addbadmessage` calls. --- htdocs/themes/math4/math4.scss | 9 +- htdocs/themes/math4/system.html.ep | 2 +- lib/WeBWorK/ContentGenerator.pm | 4 +- lib/WeBWorK/ContentGenerator/CourseAdmin.pm | 4 +- .../Instructor/AchievementList.pm | 88 +++----- .../Instructor/AchievementUserEditor.pm | 20 +- .../Instructor/ProblemGrader.pm | 6 +- .../Instructor/ProblemSetList.pm | 192 ++++++------------ lib/WeBWorK/ContentGenerator/Problem.pm | 6 +- lib/WeBWorK/ContentGenerator/ProblemSet.pm | 8 +- lib/WeBWorK/ContentGenerator/ProblemSets.pm | 4 +- .../Instructor/PGProblemEditor.html.ep | 2 +- 12 files changed, 117 insertions(+), 228 deletions(-) diff --git a/htdocs/themes/math4/math4.scss b/htdocs/themes/math4/math4.scss index 648b8322bb..70fd6de6f6 100644 --- a/htdocs/themes/math4/math4.scss +++ b/htdocs/themes/math4/math4.scss @@ -447,7 +447,14 @@ h2.page-title { /* Message section */ .message:not(:empty) { - display: inline-block; + display: inline-flex; + flex-direction: column; + gap: 0.25rem; + margin: 0 0 0.5rem; + + p { + margin: 0; + } } .font-visible { font-weight: bold; } diff --git a/htdocs/themes/math4/system.html.ep b/htdocs/themes/math4/system.html.ep index 60f1afef87..25eecb6f4b 100644 --- a/htdocs/themes/math4/system.html.ep +++ b/htdocs/themes/math4/system.html.ep @@ -164,7 +164,7 @@ % } % if ($c->can('message')) {
    <%= $c->message %>
    -
    +
    diff --git a/lib/WeBWorK/ContentGenerator.pm b/lib/WeBWorK/ContentGenerator.pm index d45126d004..9c0d7dad01 100644 --- a/lib/WeBWorK/ContentGenerator.pm +++ b/lib/WeBWorK/ContentGenerator.pm @@ -266,7 +266,7 @@ message() template escape handler. sub addgoodmessage ($c, $message) { $c->addmessage($c->tag( 'p', - class => 'alert alert-success alert-dismissible fade show ps-1 py-1 my-2', + class => 'alert alert-success alert-dismissible fade show ps-1 py-1', $c->c( $message, $c->tag( @@ -291,7 +291,7 @@ message() template escape handler. sub addbadmessage ($c, $message) { $c->addmessage($c->tag( 'p', - class => 'alert alert-danger alert-dismissible fade show ps-1 py-1 my-2', + class => 'alert alert-danger alert-dismissible fade show ps-1 py-1', $c->c( $message, $c->tag( diff --git a/lib/WeBWorK/ContentGenerator/CourseAdmin.pm b/lib/WeBWorK/ContentGenerator/CourseAdmin.pm index 729cc71948..2b00aa19a2 100644 --- a/lib/WeBWorK/ContentGenerator/CourseAdmin.pm +++ b/lib/WeBWorK/ContentGenerator/CourseAdmin.pm @@ -44,8 +44,10 @@ sub pre_header_initialize ($c) { return unless $authz->hasPermissions($user, 'create_and_delete_courses'); # Get result and send to message + # FIXME: I am pretty sure this is not used anymore. All methods add their messages directly to the page except the + # table messages, and those are added below and not here. my $status_message = $c->param('status_message'); - $c->addmessage($c->tag('p', class => 'my-2', $c->b($status_message))) if $status_message; + $c->addmessage($c->tag('p', $c->b($status_message))) if $status_message; # Check that the non-native tables are present in the database. # These are the tables which are not course specific. diff --git a/lib/WeBWorK/ContentGenerator/Instructor/AchievementList.pm b/lib/WeBWorK/ContentGenerator/Instructor/AchievementList.pm index d5a18e2ea9..68c950ca4a 100644 --- a/lib/WeBWorK/ContentGenerator/Instructor/AchievementList.pm +++ b/lib/WeBWorK/ContentGenerator/Instructor/AchievementList.pm @@ -122,8 +122,12 @@ sub initialize ($c) { } my $actionHandler = "${actionID}_handler"; - $c->addmessage($c->tag('p', class => 'mb-1', $c->maketext('Results of last action performed: '))); - $c->addmessage($c->$actionHandler); + my ($success, $action_result) = $c->$actionHandler; + if ($success) { + $c->addgoodmessage($c->b($c->maketext('Result of last action performed: [_1]', $action_result))); + } else { + $c->addbadmessage($c->b($c->maketext('Result of last action performed: [_1]', $action_result))); + } } else { $c->addgoodmessage($c->maketext('Please select action to be performed.')); } @@ -155,7 +159,7 @@ sub edit_handler ($c) { } $c->{editMode} = 1; - return $c->tag('div', class => 'alert alert-success p-1 mb-0', $result); + return (1, $result); } # Handler for assigning achievements to users @@ -215,7 +219,7 @@ sub assign_handler ($c) { } } - return $c->tag('div', class => 'alert alert-success p-1 mb-0', $c->maketext('Assigned achievements to users')); + return (1, $c->maketext('Assigned achievements to users')); } # Handler for scoring @@ -249,11 +253,7 @@ sub score_handler ($c) { $scoreFilePath = WeBWorK::Utils::surePathToFile($ce->{courseDirs}->{scoring}, $scoreFilePath); my $SCORE = Mojo::File->new($scoreFilePath)->open('>:encoding(UTF-8)') - or return $c->tag( - 'div', - class => 'alert alert-danger p-1 mb-0', - $c->maketext("Failed to open [_1]", $scoreFilePath) - ); + or return (0, $c->maketext("Failed to open [_1]", $scoreFilePath)); # Print out header info print $SCORE $c->maketext("username, last name, first name, section, achievement level, achievement score,"); @@ -308,9 +308,8 @@ sub score_handler ($c) { $SCORE->close; # Include a download link - return $c->tag( - 'div', - class => 'alert alert-success p-1 mb-0', + return ( + 1, $c->b($c->maketext( 'Achievement scores saved to [_1]', $c->link_to( @@ -352,11 +351,7 @@ sub delete_handler ($c) { $c->{selectedAchievementIDs} = [ keys %selectedAchievementIDs ]; my $num = @achievementIDsToDelete; - return $c->tag( - 'div', - class => 'alert alert-success p-1 mb-0', - $c->maketext('Deleted [quant,_1,achievement]', $num) - ); + return (1, $c->maketext('Deleted [quant,_1,achievement]', $num)); } # Handler for creating an ahcievement @@ -367,16 +362,10 @@ sub create_handler ($c) { # Create achievement my $newAchievementID = $c->param('action.create.id'); - return $c->tag( - 'div', - class => 'alert alert-danger p-1 mb-0', - $c->maketext("Failed to create new achievement: no achievement ID specified!") - ) unless $newAchievementID =~ /\S/; - return $c->tag( - 'div', - class => 'alert alert-danger p-1 mb-0', - $c->maketext("Achievement [_1] exists. No achievement created", $newAchievementID) - ) if $db->existsAchievement($newAchievementID); + return (0, $c->maketext("Failed to create new achievement: no achievement ID specified!")) + unless $newAchievementID =~ /\S/; + return (0, $c->maketext("Achievement [_1] exists. No achievement created", $newAchievementID)) + if $db->existsAchievement($newAchievementID); my $newAchievementRecord = $db->newAchievement; my $oldAchievementID = $c->{selectedAchievementIDs}->[0]; @@ -390,11 +379,8 @@ sub create_handler ($c) { $newAchievementRecord->test('blankachievement.at'); $db->addAchievement($newAchievementRecord); } elsif ($type eq "copy") { - return $c->tag( - 'div', - class => 'alert alert-danger p-1 mb-0', - $c->maketext("Failed to duplicate achievement: no achievement selected for duplication!") - ) unless $oldAchievementID =~ /\S/; + return (0, $c->maketext("Failed to duplicate achievement: no achievement selected for duplication!")) + unless $oldAchievementID =~ /\S/; $newAchievementRecord = $db->getAchievement($oldAchievementID); $newAchievementRecord->achievement_id($newAchievementID); $db->addAchievement($newAchievementRecord); @@ -410,17 +396,9 @@ sub create_handler ($c) { # Add to local list of achievements push @{ $c->{allAchievementIDs} }, $newAchievementID; - return $c->tag( - 'div', - class => 'alert alert-danger p-1 mb-0', - $c->maketext("Failed to create new achievement: [_1]", $@) - ) if $@; + return (0, $c->maketext("Failed to create new achievement: [_1]", $@)) if $@; - return $c->tag( - 'div', - class => 'alert alert-success p-1 mb-0', - $c->maketext('Successfully created new achievement [_1]', $newAchievementID) - ); + return (1, $c->maketext('Successfully created new achievement [_1]', $newAchievementID)); } # Handler for importing achievements @@ -436,8 +414,7 @@ sub import_handler ($c) { # Open file name my $fh = Mojo::File->new($filePath)->open('<:encoding(UTF-8)') - or - return $c->tag('div', class => 'alert alert-danger p-1 mb-0', $c->maketext("Failed to open [_1]", $filePath)); + or return (0, $c->maketext("Failed to open [_1]", $filePath)); # Read in lines from file my $count = 0; @@ -510,11 +487,7 @@ sub import_handler ($c) { $c->{allAchievementIDs} = [ keys %allAchievementIDs ]; - return $c->tag( - 'div', - class => 'alert alert-success p-1 mb-0', - $c->maketext('Imported [quant,_1,achievement]', $count) - ); + return (1, $c->maketext('Imported [quant,_1,achievement]', $count)); } # Export handler @@ -532,14 +505,14 @@ sub export_handler ($c) { } $c->{exportMode} = 1; - return $c->tag('div', class => 'alert alert-success p-1 mb-0', $result); + return (1, $result); } # Handler for leaving the export page. sub cancel_export_handler ($c) { $c->{exportMode} = 0; - return $c->tag('div', class => 'alert alert-danger p-1 mb-0', $c->maketext('export abandoned')); + return (0, $c->maketext('export abandoned')); } # Handler actually exporting achievements. @@ -563,8 +536,7 @@ sub save_export_handler ($c) { $FilePath = WeBWorK::Utils::surePathToFile($ce->{courseDirs}{achievements}, $FilePath); my $fh = Mojo::File->new($FilePath)->open('>:encoding(UTF-8)') - or - return $c->tag('div', class => 'alert alert-danger p-1 mb-0', $c->maketext('Failed to open [_1]', $FilePath)); + or return (0, $c->maketext('Failed to open [_1]', $FilePath)); my $csv = Text::CSV->new({ eol => "\n" }); @@ -585,17 +557,13 @@ sub save_export_handler ($c) { $c->{exportMode} = 0; - return $c->tag( - 'div', - class => 'alert alert-success p-1 mb-0', - $c->maketext('Exported achievements to [_1]', $FileName) - ); + return (1, $c->maketext('Exported achievements to [_1]', $FileName)); } # Handler for cancelling edits. sub cancel_edit_handler ($c) { $c->{editMode} = 0; - return $c->tag('div', class => 'alert alert-danger p-1 mb-0', $c->maketext('changes abandoned')); + return (1, $c->maketext('changes abandoned')); } # Handler for saving edits. @@ -630,7 +598,7 @@ sub save_edit_handler ($c) { $c->{editMode} = 0; - return $c->tag('div', class => 'alert alert-success p-1 mb-0', $c->maketext('changes saved')); + return (1, $c->maketext('changes saved')); } # Get list of files that can be imported. diff --git a/lib/WeBWorK/ContentGenerator/Instructor/AchievementUserEditor.pm b/lib/WeBWorK/ContentGenerator/Instructor/AchievementUserEditor.pm index 737261d1ce..41f52611f6 100644 --- a/lib/WeBWorK/ContentGenerator/Instructor/AchievementUserEditor.pm +++ b/lib/WeBWorK/ContentGenerator/Instructor/AchievementUserEditor.pm @@ -42,11 +42,7 @@ sub initialize ($c) { #Check and see if we need to assign or unassign things if (defined $c->param('assignToAll')) { - $c->addmessage($c->tag( - 'p', - class => 'alert alert-success p-1 mb-0', - $c->maketext('Achievement has been assigned to all users.') - )); + $c->addgoodmessage($c->maketext('Achievement has been assigned to all users.')); %selectedUsers = map { $_ => 1 } @all_users; $doAssignToSelected = 1; } elsif (defined $c->param('unassignFromAll') @@ -54,22 +50,14 @@ sub initialize ($c) { && $c->param('unassignFromAllSafety') == 1) { %selectedUsers = (); - $c->addmessage($c->tag( - 'p', - class => 'alert alert-danger p-1 mb-0', - $c->maketext('Achievement has been unassigned to all students.') - )); + $c->addbadmessage($c->maketext('Achievement has been unassigned to all students.')); $doAssignToSelected = 1; } elsif (defined $c->param('assignToSelected')) { - $c->addmessage($c->tag( - 'p', - class => 'alert alert-success p-1 mb-0', - $c->maketext('Achievement has been assigned to selected users.') - )); + $c->addgoodmessage($c->maketext('Achievement has been assigned to selected users.')); $doAssignToSelected = 1; } elsif (defined $c->param('unassignFromAll')) { # no action taken - $c->addmessage($c->tag('p', class => 'alert alert-danger p-1 mb-0', $c->maketext('No action taken'))); + $c->addbadmessage($c->maketext('No action taken')); } #do actual assignment and unassignment diff --git a/lib/WeBWorK/ContentGenerator/Instructor/ProblemGrader.pm b/lib/WeBWorK/ContentGenerator/Instructor/ProblemGrader.pm index 1a8a1487a7..617c09389a 100644 --- a/lib/WeBWorK/ContentGenerator/Instructor/ProblemGrader.pm +++ b/lib/WeBWorK/ContentGenerator/Instructor/ProblemGrader.pm @@ -109,11 +109,7 @@ async sub initialize ($c) { # Update grades if saving. if ($c->param('assignGrades')) { - $c->addmessage($c->tag( - 'p', - class => 'alert alert-success p-1 my-2', - $c->maketext('Grades have been saved for all current users.') - )); + $c->addgoodmessage($c->maketext('Grades have been saved for all current users.')); for my $user (@{ $c->stash->{users} }) { my $userID = $user->user_id; diff --git a/lib/WeBWorK/ContentGenerator/Instructor/ProblemSetList.pm b/lib/WeBWorK/ContentGenerator/Instructor/ProblemSetList.pm index 6c4ef3644a..903e2df08d 100644 --- a/lib/WeBWorK/ContentGenerator/Instructor/ProblemSetList.pm +++ b/lib/WeBWorK/ContentGenerator/Instructor/ProblemSetList.pm @@ -156,7 +156,7 @@ sub pre_header_initialize ($c) { my @setsToScore; if ($scope eq 'none') { - return $c->maketext('No sets selected for scoring'); + return; } elsif ($scope eq 'all') { @setsToScore = @{ $c->{allSetIDs} }; } elsif ($scope eq 'visible') { @@ -165,6 +165,8 @@ sub pre_header_initialize ($c) { @setsToScore = $c->param('selected_sets'); } + return unless @setsToScore; + $c->reply_with_redirect($c->systemLink( $c->url_for('instructor_scoring'), params => { scoreSelected => 'ScoreSelected', selectedSet => \@setsToScore } @@ -233,8 +235,12 @@ sub initialize ($c) { # Check permissions if (not FORM_PERMS()->{$actionID} or $authz->hasPermissions($user, FORM_PERMS()->{$actionID})) { my $actionHandler = "${actionID}_handler"; - $c->addmessage($c->tag('p', class => 'mb-1', $c->maketext("Results of last action performed") . ": ")); - $c->addmessage($c->$actionHandler); + my ($success, $action_result) = $c->$actionHandler; + if ($success) { + $c->addgoodmessage($c->b($c->maketext('Result of last action performed: [_1]', $action_result))); + } else { + $c->addbadmessage($c->b($c->maketext('Result of last action performed: [_1]', $action_result))); + } } else { $c->addbadmessage($c->maketext('You are not authorized to perform this action.')); } @@ -300,7 +306,7 @@ sub filter_handler ($c) { $c->{visibleSetIDs} = [ map { $_->[0] } $db->listGlobalSetsWhere({ visible => 0 }) ]; } - return $c->tag('div', class => 'alert alert-success p-1 mb-0', $result); + return (1, $result); } sub sort_handler ($c) { @@ -318,11 +324,7 @@ sub sort_handler ($c) { visible => $c->maketext("Visibility"), ); - return $c->tag( - 'div', - class => 'alert alert-success p-1 mb-0', - $c->maketext("Sort by [_1] and then by [_2]", $names{$primary}, $names{$secondary}) - ); + return (1, $c->maketext("Sort by [_1] and then by [_2]", $names{$primary}, $names{$secondary})); } sub edit_handler ($c) { @@ -341,91 +343,53 @@ sub edit_handler ($c) { } $c->{editMode} = 1; - return $c->tag('div', class => 'alert alert-success p-1 mb-0', $result); + return (1, $result); } sub publish_handler ($c) { my $db = $c->db; - my $result = ""; + my @result; my $scope = $c->param('action.publish.scope'); my $value = $c->param('action.publish.value'); - my $verb = $value ? $c->maketext("made visible for") : $c->maketext("hidden from"); - my @setIDs; if ($scope eq "none") { @setIDs = (); - $result = $c->tag('div', class => 'alert alert-danger p-1 mb-0', $c->maketext("No change made to any set")); + @result = (0, $c->maketext("No change made to any set")); } elsif ($scope eq "all") { @setIDs = @{ $c->{allSetIDs} }; - $result = $value - ? $c->tag( - 'div', - class => 'alert alert-success p-1 mb-0', - $c->maketext("All sets made visible for all students") - ) - : $c->tag( - 'div', - class => 'alert alert-success p-1 mb-0', - $c->maketext("All sets hidden from all students") - ); + @result = + $value + ? (1, $c->maketext("All sets made visible for all students")) + : (1, $c->maketext("All sets hidden from all students")); } elsif ($scope eq "visible") { @setIDs = @{ $c->{visibleSetIDs} }; - $result = $value - ? $c->tag( - 'div', - class => 'alert alert-success p-1 mb-0', - $c->maketext("All listed sets were made visible for all the students") - ) - : $c->tag( - 'div', - class => 'alert alert-success p-1 mb-0', - $c->maketext("All listed sets were hidden from all the students") - ); + @result = + $value + ? (1, $c->maketext("All listed sets were made visible for all the students")) + : (1, $c->maketext("All listed sets were hidden from all the students")); } elsif ($scope eq "selected") { @setIDs = $c->param('selected_sets'); - $result = $value - ? $c->tag( - 'div', - class => 'alert alert-success p-1 mb-0', - $c->maketext("All selected sets made visible for all students") - ) - : $c->tag( - 'div', - class => 'alert alert-success p-1 mb-0', - $c->maketext("All selected sets hidden from all students") - ); + @result = + $value + ? (1, $c->maketext("All selected sets made visible for all students")) + : (1, $c->maketext("All selected sets hidden from all students")); } # Can we use UPDATE here, instead of fetch/change/store? my @sets = $db->getGlobalSets(@setIDs); map { $_->visible($value); $db->putGlobalSet($_); } @sets; - return $c->tag('div', class => 'alert alert-success p-1 mb-0', $result); + return @result; } sub score_handler ($c) { - my $courseName = $c->stash('courseID'); - - my $scope = $c->param('action.score.scope'); - my @setsToScore; - - if ($scope eq 'none') { - @setsToScore = (); - return $c->maketext('No sets selected for scoring'); - } elsif ($scope eq 'all') { - @setsToScore = @{ $c->{allSetIDs} }; - } elsif ($scope eq 'visible') { - @setsToScore = @{ $c->{visibleSetIDs} }; - } elsif ($scope eq 'selected') { - @setsToScore = $c->param('selected_sets'); - } - - return $c->systemLink($c->url_for('instructor_scoring'), - params => { scoreSelected => 'Score Selected', selectedSet => \@setsToScore }); + # The only time this is called is if "no sets" is selected (do we really need that option), + # or one of the other options was selected but there were no sets to score. + return (0, $c->maketext('No sets selected for scoring.')); } sub delete_handler ($c) { @@ -455,7 +419,7 @@ sub delete_handler ($c) { $c->{selectedSetIDs} = [ keys %selectedSetIDs ]; my $num = @setIDsToDelete; - return $c->tag('div', class => 'alert alert-success p-1 mb-0', $c->maketext('deleted [_1] sets', $num)); + return (1, $c->maketext('deleted [_1] sets', $num)); } sub create_handler ($c) { @@ -463,28 +427,19 @@ sub create_handler ($c) { my $ce = $c->ce; my $newSetID = format_set_name_internal($c->param('action.create.name') // ''); - return $c->tag( - 'div', - class => 'alert alert-danger p-1 mb-0', - $c->maketext("Failed to create new set: Set name cannot exceed 100 characters.") - ) if (length($newSetID) > 100); - return $c->tag( - 'div', - class => 'alert alert-danger p-1 mb-0', - $c->maketext("Failed to create new set: No set name specified.") - ) unless $newSetID =~ /\S/; - return $c->tag( - 'div', - class => 'alert alert-danger p-1 mb-0', + return (0, $c->maketext("Failed to create new set: Set name cannot exceed 100 characters.")) + if (length($newSetID) > 100); + return (0, $c->maketext("Failed to create new set: No set name specified.")) unless $newSetID =~ /\S/; + return ( + 0, $c->maketext( 'Failed to create new set: Invalid characters in set name "[_1]". ' . 'A set name may only contain letters, numbers, hyphens, periods, and spaces.', $newSetID =~ s/_/ /gr ) ) unless $newSetID =~ m/^[-a-zA-Z0-9_.]*$/; - return $c->tag( - 'div', - class => 'alert alert-danger p-1 mb-0', + return ( + 0, $c->maketext( 'The set name "[_1]" is already in use. Pick a different name if you would like to start a new set. ' . 'No set created.', @@ -524,11 +479,7 @@ sub create_handler ($c) { $newSetRecord->assignment_type('default'); $db->addGlobalSet($newSetRecord); } elsif ($type eq "copy") { - return $c->tag( - 'div', - class => 'alert alert-danger p-1 mb-0', - $c->maketext('Failed to duplicate set: no set selected for duplication!') - ) unless $oldSetID =~ /\S/; + return (0, $c->maketext('Failed to duplicate set: no set selected for duplication!')) unless $oldSetID =~ /\S/; $newSetRecord = $db->getGlobalSet($oldSetID); $newSetRecord->set_id($newSetID); $db->addGlobalSet($newSetRecord); @@ -570,12 +521,10 @@ sub create_handler ($c) { push @{ $c->{visibleSetIDs} }, $newSetID; push @{ $c->{allSetIds} }, $newSetID; - return $c->tag('div', class => 'alert alert-danger p-1 mb-0', $c->maketext('Failed to create new set: [_1]', $@)) - if $@; + return (0, $c->maketext('Failed to create new set: [_1]', $@)) if $@; - return $c->tag( - 'div', - class => 'alert alert-success p-1 mb-0', + return ( + 1, $c->b($c->maketext( 'Successfully created new set [_1]', $c->tag('span', dir => 'ltr', format_set_name_display($newSetID)) @@ -600,9 +549,8 @@ sub import_handler ($c) { my $numAdded = @$added; my $numSkipped = @$skipped; - return $c->tag( - 'div', - class => 'alert alert-success p-1 mb-0', + return ( + 1, $c->maketext( '[_1] sets added, [_2] sets skipped. Skipped sets: ([_3])', $numAdded, $numSkipped, join(', ', @$skipped) @@ -627,7 +575,7 @@ sub export_handler ($c) { } $c->{exportMode} = 1; - return $c->tag('div', class => 'alert alert-success p-1 mb-0', $result); + return (1, $result); } sub cancel_export_handler ($c) { @@ -640,7 +588,7 @@ sub cancel_export_handler ($c) { } $c->{exportMode} = 0; - return $c->tag('div', class => 'alert alert-danger p-1 mb-0', $c->maketext('export abandoned')); + return (0, $c->maketext('export abandoned')); } sub save_export_handler ($c) { @@ -660,13 +608,11 @@ sub save_export_handler ($c) { my $numExported = @$exported; my $numSkipped = @$skipped; - my $resultFont = $numSkipped ? 'alert-danger' : 'alert-success'; my @reasons = map { "set $_ - " . $reason->{$_} } keys %$reason; - return $c->tag( - 'div', - class => "alert $resultFont p-1 mb-0", + return ( + !$numSkipped, $c->b($c->maketext( '[_1] sets exported, [_2] sets skipped. Skipped sets: ([_3])', $numExported, $numSkipped, @@ -685,7 +631,7 @@ sub cancel_edit_handler ($c) { } $c->{editMode} = 0; - return $c->tag('div', class => 'alert alert-danger p-1 mb-0', $c->maketext('changes abandoned')); + return (0, $c->maketext('changes abandoned')); } sub save_edit_handler ($c) { @@ -722,37 +668,20 @@ sub save_edit_handler ($c) { my $curr_time = time; my $seconds_per_year = 31_556_926; my $cutoff = $curr_time + $seconds_per_year * 10; - return $c->tag( - 'div', - class => 'alert alert-danger p-1 mb-0', - $c->maketext("Error: open date cannot be more than 10 years from now in set [_1]", $setID) - ) if $Set->open_date > $cutoff; - return $c->tag( - 'div', - class => 'alert alert-danger p-1 mb-0', - $c->maketext("Error: close date cannot be more than 10 years from now in set [_1]", $setID) - ) if $Set->due_date > $cutoff; - return $c->tag( - 'div', - class => 'alert alert-danger p-1 mb-0', - $c->maketext("Error: answer date cannot be more than 10 years from now in set [_1]", $setID) - ) if $Set->answer_date > $cutoff; + return (0, $c->maketext("Error: open date cannot be more than 10 years from now in set [_1]", $setID)) + if $Set->open_date > $cutoff; + return (0, $c->maketext("Error: close date cannot be more than 10 years from now in set [_1]", $setID)) + if $Set->due_date > $cutoff; + return (0, $c->maketext("Error: answer date cannot be more than 10 years from now in set [_1]", $setID)) + if $Set->answer_date > $cutoff; # Check that the open, due and answer dates are in increasing order. # Bail if this is not correct. if ($Set->open_date > $Set->due_date) { - return $c->tag( - 'div', - class => 'alert alert-danger p-1 mb-0', - $c->maketext("Error: Close date must come after open date in set [_1]", $setID) - ); + return (0, $c->maketext("Error: Close date must come after open date in set [_1]", $setID)); } if ($Set->due_date > $Set->answer_date) { - return $c->tag( - 'div', - class => 'alert alert-danger p-1 mb-0', - $c->maketext("Error: Answer date must come after close date in set [_1]", $setID) - ); + return (0, $c->maketext("Error: Answer date must come after close date in set [_1]", $setID)); } # check that the reduced scoring date is in the right place @@ -769,9 +698,8 @@ sub save_edit_handler ($c) { || $Set->reduced_scoring_date < $Set->open_date) ) { - return $c->tag( - 'div', - class => 'alert alert-danger p-1 mb-0', + return ( + 0, $c->maketext( "Error: Reduced scoring date must come between the open date and close date in set [_1]", $setID @@ -792,7 +720,7 @@ sub save_edit_handler ($c) { $c->{editMode} = 0; - return $c->tag('div', class => 'alert alert-success p-1 mb-0', $c->maketext("changes saved")); + return (1, $c->maketext("changes saved")); } # Utilities diff --git a/lib/WeBWorK/ContentGenerator/Problem.pm b/lib/WeBWorK/ContentGenerator/Problem.pm index f08454734e..3cbf75816b 100644 --- a/lib/WeBWorK/ContentGenerator/Problem.pm +++ b/lib/WeBWorK/ContentGenerator/Problem.pm @@ -407,8 +407,8 @@ async sub pre_header_initialize ($c) { } $c->addmessage($c->{set}->visible - ? $c->tag('span', class => 'font-visible', $c->maketext('This set is visible to students.')) - : $c->tag('span', class => 'font-hidden', $c->maketext('This set is hidden from students.'))); + ? $c->tag('p', class => 'font-visible', $c->maketext('This set is visible to students.')) + : $c->tag('p', class => 'font-hidden', $c->maketext('This set is hidden from students.'))); } else { # Test for additional problem validity if it's not already invalid. @@ -460,7 +460,7 @@ async sub pre_header_initialize ($c) { $c->{formFields} = $formFields; # Get the status message and add it to the messages. - $c->addmessage($c->tag('p', class => 'my-2', $c->b($c->param('status_message')))) if $c->param('status_message'); + $c->addmessage($c->tag('p', $c->b($c->param('status_message')))) if $c->param('status_message'); # Now that the necessary variables are set, return if the set or problem is invalid. return if $c->{invalidSet} || $c->{invalidProblem}; diff --git a/lib/WeBWorK/ContentGenerator/ProblemSet.pm b/lib/WeBWorK/ContentGenerator/ProblemSet.pm index 7d2f865e65..33f275488b 100644 --- a/lib/WeBWorK/ContentGenerator/ProblemSet.pm +++ b/lib/WeBWorK/ContentGenerator/ProblemSet.pm @@ -50,13 +50,13 @@ async sub initialize ($c) { $c->{displayMode} = $user->displayMode || $ce->{pg}{options}{displayMode}; # Display status messages. - $c->addmessage($c->tag('p', class => 'my-2', $c->b($c->param('status_message')))) if $c->param('status_message'); + $c->addmessage($c->tag('p', $c->b($c->param('status_message')))) if $c->param('status_message'); if ($authz->hasPermissions($userID, 'view_hidden_sets')) { if ($c->{set}->visible) { - $c->addmessage($c->tag('span', class => 'font-visible', $c->maketext('This set is visible to students.'))); + $c->addmessage($c->tag('p', class => 'font-visible', $c->maketext('This set is visible to students.'))); } else { - $c->addmessage($c->tag('span', class => 'font-hidden', $c->maketext('This set is hidden from students.'))); + $c->addmessage($c->tag('p', class => 'font-hidden', $c->maketext('This set is hidden from students.'))); } } @@ -76,7 +76,7 @@ async sub initialize ($c) { $screenSetHeader = "$ce->{courseDirs}{templates}/$screenSetHeader" unless $screenSetHeader =~ m!^/!; die 'sourceFilePath is unsafe!' unless path_is_subdir($screenSetHeader, $ce->{courseDirs}{templates}); $c->addmessage($c->tag( - 'div', + 'p', class => 'temporaryFile', $c->maketext('Viewing temporary file: [_1]', $screenSetHeader) )); diff --git a/lib/WeBWorK/ContentGenerator/ProblemSets.pm b/lib/WeBWorK/ContentGenerator/ProblemSets.pm index c0f693e4fc..ac06246e4b 100644 --- a/lib/WeBWorK/ContentGenerator/ProblemSets.pm +++ b/lib/WeBWorK/ContentGenerator/ProblemSets.pm @@ -60,7 +60,7 @@ sub initialize ($c) { if ($authz->hasPermissions($user, 'access_instructor_tools')) { my $status_message = $c->param('status_message'); - $c->addmessage($c->tag('p', class => 'my-2', $c->b($status_message))) if $status_message; + $c->addmessage($c->tag('p', $c->b($status_message))) if $status_message; } if ($authz->hasPermissions($user, 'navigation_allowed')) { @@ -105,7 +105,7 @@ sub initialize ($c) { $c->addmessage($c->tag( 'p', - class => 'temporaryFile my-2', + class => 'temporaryFile', $c->maketext('Viewing temporary file: [_1]', $course_info_path) )); } diff --git a/templates/ContentGenerator/Instructor/PGProblemEditor.html.ep b/templates/ContentGenerator/Instructor/PGProblemEditor.html.ep index 3819a4d645..00a8d3db44 100644 --- a/templates/ContentGenerator/Instructor/PGProblemEditor.html.ep +++ b/templates/ContentGenerator/Instructor/PGProblemEditor.html.ep @@ -199,6 +199,6 @@
    <%= submit_button maketext($actionFormTitles->{$default_choice}), name => 'submit', id => 'take_action', - class => 'btn btn-primary' =%> + class => 'btn btn-primary mb-2' =%>
    <% end =%>