Skip to content

Commit 0c8f3b8

Browse files
committed
Rework the single problem grader interface.
The problem grader is now always visible for users that have the permission to use it and in the case that they are acting for another user. This does mean that there is no way to open the problem grader when viewing your own problem. However, the problem grader is in a collapse. The state of the collapse is stored in local storage, and whenever you open another problem or change effective users, the collapse goes back to the state that it was in the last time that you had a page open that showed the problem grader. Correct answers in feedback are now always shown with the reveal button, even when the problem grader is on the page. However, the reveal button is removed by JavaScript behind the scenes while the problem grader is expanded, and put back if the feedback button is not opened while the problem grader is open. So if you open a feedback button while the problem grader is open, the reveal button is not shown, and the correct answer is immediately visible. To summarize the reveal button visibility, the reveal button will not be shown anytime that a feedback button is opened while the problem grader is open, and in that case will never return until the page reloads, but any feedback button that is not opened while the problem grader is open will still show the reveal button, and as usual once the reveal button is used, it will never come back until the page is reloaded. The problem grader is now below the problem in homework sets as it is in tests. With the collapse and the grader always in the page, I really do not want it above the problem as it currently is. The original reason for the problem grader being on top was so that it would be close to the old results table with the answers. With that gone, that reason no longer applies. Also remove the code for the `output_hidden_info` method in the `Problem.html.ep` template. This is because the answer to the question `$c->can('output_hidden_info')` is `$c` can't. There is no such method anywhere in the code anymore.
1 parent 499a44c commit 0c8f3b8

File tree

15 files changed

+207
-128
lines changed

15 files changed

+207
-128
lines changed

htdocs/js/GatewayQuiz/gateway.scss

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ table.attemptResults {
6060
border: 1px solid #ddd;
6161
border-radius: 3px;
6262

63-
h2 {
63+
h2.gw-problem-number {
6464
display: inline-block;
6565
font-size: 16px;
6666
margin-right: 5px;

htdocs/js/ProblemGrader/singleproblemgrader.js

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -216,4 +216,139 @@
216216
}
217217
});
218218
}
219+
220+
const settingStoreID = `WW.${document.getElementsByName('courseID')[0]?.value ?? 'unknownCourse'}.${
221+
document.getElementsByName('user')[0]?.value ?? 'unknownUser'
222+
}.problem_grader`;
223+
let gradersOpen = localStorage.getItem(`${settingStoreID}.open`) === 'true';
224+
225+
const graderCollapses = [];
226+
227+
for (const grader of document.querySelectorAll('.problem-grader')) {
228+
const problemId = grader.id.replace('problem-grader-');
229+
230+
grader.classList.add('accordion');
231+
232+
const accordionItem = document.createElement('div');
233+
accordionItem.classList.add('accordion-item');
234+
235+
const accordionHeader = document.createElement('h2');
236+
accordionHeader.classList.add('accordion-header');
237+
238+
const accordionButton = document.createElement('button');
239+
accordionButton.classList.add('accordion-button');
240+
accordionButton.type = 'button';
241+
accordionButton.textContent = grader.dataset.graderTitle ?? 'Problem Grader';
242+
accordionButton.dataset.bsToggle = 'collapse';
243+
accordionButton.dataset.bsTarget = `#problem-grader-collapse-${problemId}`;
244+
accordionButton.setAttribute('aria-controls', `#problem-grader-collapse-${problemId}`);
245+
accordionButton.setAttribute('aria-expanded', gradersOpen);
246+
if (!gradersOpen) accordionButton.classList.add('collapsed');
247+
248+
accordionHeader.append(accordionButton);
249+
250+
const accordionCollapse = document.createElement('div');
251+
accordionCollapse.classList.add('accordion-collapse', 'collapse');
252+
accordionCollapse.id = `problem-grader-collapse-${problemId}`;
253+
accordionCollapse.dataset.bsParent = `problem-grader-${problemId}`;
254+
if (gradersOpen) accordionCollapse.classList.add('show');
255+
256+
const accordionBody = grader.querySelector('.problem-grader-table');
257+
accordionBody.classList.add('accordion-body');
258+
accordionCollapse.append(accordionBody);
259+
260+
accordionItem.append(accordionHeader, accordionCollapse);
261+
grader.append(accordionItem);
262+
263+
const graderCollapse = new bootstrap.Collapse(accordionCollapse, { toggle: false });
264+
graderCollapses.push(graderCollapse);
265+
266+
// Expand or collapse all problem graders on the page when any one of them is expanded or collapsed.
267+
let transitioning = false;
268+
accordionCollapse.addEventListener('show.bs.collapse', () => {
269+
if (transitioning) return;
270+
transitioning = true;
271+
for (const grader of graderCollapses) {
272+
if (grader !== graderCollapse) grader.show();
273+
}
274+
transitioning = false;
275+
});
276+
accordionCollapse.addEventListener('hide.bs.collapse', () => {
277+
if (transitioning) return;
278+
transitioning = true;
279+
for (const grader of graderCollapses) {
280+
if (grader !== graderCollapse) grader.hide();
281+
}
282+
transitioning = false;
283+
});
284+
285+
// Make sure that the "Reveal" button in feedback is not shown if a feedback button is used while the problem
286+
// grader is open. However, also make sure that the "Reveal" button is shown for any feedback button that is
287+
// not used while the problem grader is open.
288+
289+
const unrevealedFeedbackBtns = [];
290+
291+
for (const feedbackBtn of document.querySelectorAll('.ww-feedback-btn')) {
292+
const container = document.createElement('div');
293+
container.innerHTML = feedbackBtn.dataset.bsContent;
294+
const button = container.querySelector('.reveal-correct-btn');
295+
if (!button) continue;
296+
297+
button.nextElementSibling?.classList.remove('d-none');
298+
button.remove();
299+
300+
const fragment = new DocumentFragment();
301+
fragment.append(container);
302+
303+
unrevealedFeedbackBtns.push([feedbackBtn, fragment.firstElementChild.innerHTML]);
304+
305+
const handler = () => {
306+
const index = unrevealedFeedbackBtns.findIndex((data) => data[0] === feedbackBtn);
307+
if (index !== -1) {
308+
if (gradersOpen) {
309+
unrevealedFeedbackBtns.splice(index, 1);
310+
feedbackBtn.removeEventListener('shown.bs.popover', handler);
311+
} else {
312+
bootstrap.Popover.getInstance(feedbackBtn)
313+
?.tip?.querySelector('.reveal-correct-btn')
314+
?.addEventListener(
315+
'click',
316+
() => {
317+
unrevealedFeedbackBtns.splice(index, 1);
318+
feedbackBtn.removeEventListener('shown.bs.popover', handler);
319+
},
320+
{ once: true }
321+
);
322+
}
323+
}
324+
};
325+
326+
feedbackBtn.addEventListener('shown.bs.popover', handler);
327+
}
328+
329+
const removeRevealButtons = () => {
330+
for (const data of unrevealedFeedbackBtns) {
331+
const feedbackPopover = bootstrap.Popover.getInstance(data[0]);
332+
feedbackPopover?.setContent({ '.popover-body': data[1] });
333+
}
334+
};
335+
336+
if (gradersOpen) removeRevealButtons();
337+
338+
// In addition to removing and putting back the feedback "Reveal" buttons as needed,
339+
// preserve the collapsed/expanded status of the problem graders in local storage.
340+
accordionCollapse.addEventListener('shown.bs.collapse', () => {
341+
localStorage.setItem(`${settingStoreID}.open`, 'true');
342+
gradersOpen = true;
343+
removeRevealButtons();
344+
});
345+
accordionCollapse.addEventListener('hidden.bs.collapse', () => {
346+
gradersOpen = false;
347+
localStorage.setItem(`${settingStoreID}.open`, 'false');
348+
for (const data of unrevealedFeedbackBtns) {
349+
const feedbackPopover = bootstrap.Popover.getInstance(data[0]);
350+
feedbackPopover?.setContent({ '.popover-body': data[0].dataset.bsContent });
351+
}
352+
});
353+
}
219354
})();

htdocs/js/System/system.scss

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1018,6 +1018,17 @@ td.alt-source {
10181018
}
10191019
}
10201020

1021+
.problem-grader.accordion {
1022+
.accordion-header {
1023+
.accordion-button {
1024+
--bs-accordion-btn-padding-x: 0.75rem;
1025+
--bs-accordion-btn-padding-y: 0.375rem;
1026+
--bs-accordion-btn-bg: var(--bs-primary, #038);
1027+
--bs-accordion-btn-color: var(--ww-primary-foreground-color, white);
1028+
}
1029+
}
1030+
}
1031+
10211032
.problem-grader-table {
10221033
.col-fixed {
10231034
width: 11rem;

lib/WeBWorK/ConfigValues.pm

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -820,7 +820,8 @@ sub getConfigValues ($ce) {
820820
doc2 => x(
821821
'A "Reveal" button must be clicked to make a correct answer visible any time that correct '
822822
. 'answers for a problem are shown. Note that this is always the case for instructors '
823-
. 'before answers are available to students, and in "Show Me Another" problems.'
823+
. 'before answers are available to students (except when the problem grader is open), and '
824+
. 'in "Show Me Another" problems.'
824825
),
825826
type => 'boolean'
826827
}

lib/WeBWorK/ContentGenerator/GatewayQuiz.pm

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -780,14 +780,11 @@ async sub pre_header_initialize ($c) {
780780
return;
781781
}
782782

783-
# Unset the showProblemGrader parameter if the "Hide Problem Grader" button was clicked.
784-
$c->param(showProblemGrader => undef) if $c->param('hideProblemGrader');
785-
786783
# What does the user want to do?
787784
my %want = (
788785
showOldAnswers => $user->showOldAnswers ne '' ? $user->showOldAnswers : $ce->{pg}{options}{showOldAnswers},
789786
showCorrectAnswers => 1,
790-
showProblemGrader => $c->param('showProblemGrader') || 0,
787+
showProblemGrader => $userID ne $effectiveUserID,
791788
showHints => 0, # Hints are not yet implemented in gateway quzzes.
792789
showSolutions => 1,
793790
recordAnswers => $c->{submitAnswers} && !$authz->hasPermissions($userID, 'avoid_recording_answers'),
@@ -1491,10 +1488,9 @@ async sub getProblemHTML ($c, $effectiveUser, $set, $formFields, $mergedProblem)
14911488
&& $c->can_showCorrectAnswersForAll($set, $c->{problem}, $c->{tmplSet})),
14921489
showMessages => !$showOnlyCorrectAnswers,
14931490
showCorrectAnswers => (
1494-
$c->{will}{showProblemGrader} ? 2
1495-
: !$c->{previewAnswers} && $c->can_showCorrectAnswersForAll($set, $c->{problem}, $c->{tmplSet})
1491+
!$c->{previewAnswers} && $c->can_showCorrectAnswersForAll($set, $c->{problem}, $c->{tmplSet})
14961492
? ($c->ce->{pg}{options}{correctRevealBtnAlways} ? 1 : 2)
1497-
: !$c->{previewAnswers} && $c->{will}{showCorrectAnswers} ? 1
1493+
: $c->{will}{showProblemGrader} || (!$c->{previewAnswers} && $c->{will}{showCorrectAnswers}) ? 1
14981494
: 0
14991495
),
15001496
debuggingOptions => getTranslatorDebuggingOptions($c->authz, $c->{userID}),

lib/WeBWorK/ContentGenerator/Problem.pm

Lines changed: 15 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -431,20 +431,17 @@ async sub pre_header_initialize ($c) {
431431
Count => $problem->{showMeAnotherCount},
432432
};
433433

434-
# Unset the showProblemGrader parameter if the "Hide Problem Grader" button was clicked.
435-
$c->param(showProblemGrader => undef) if $c->param('hideProblemGrader');
436-
437434
# Permissions
438435

439436
# What does the user want to do?
440437
my %want = (
441438
showOldAnswers => $user->showOldAnswers ne '' ? $user->showOldAnswers : $ce->{pg}{options}{showOldAnswers},
442439
showCorrectAnswers => 1,
443-
showProblemGrader => $c->param('showProblemGrader') || 0,
444-
showAnsGroupInfo => $c->param('showAnsGroupInfo') || $ce->{pg}{options}{showAnsGroupInfo},
445-
showAnsHashInfo => $c->param('showAnsHashInfo') || $ce->{pg}{options}{showAnsHashInfo},
446-
showPGInfo => $c->param('showPGInfo') || $ce->{pg}{options}{showPGInfo},
447-
showResourceInfo => $c->param('showResourceInfo') || $ce->{pg}{options}{showResourceInfo},
440+
showProblemGrader => $userID ne $effectiveUserID,
441+
showAnsGroupInfo => $c->param('showAnsGroupInfo') || $ce->{pg}{options}{showAnsGroupInfo},
442+
showAnsHashInfo => $c->param('showAnsHashInfo') || $ce->{pg}{options}{showAnsHashInfo},
443+
showPGInfo => $c->param('showPGInfo') || $ce->{pg}{options}{showPGInfo},
444+
showResourceInfo => $c->param('showResourceInfo') || $ce->{pg}{options}{showResourceInfo},
448445
showHints => 1,
449446
showSolutions => 1,
450447
useMathView => $user->useMathView ne '' ? $user->useMathView : $ce->{pg}{options}{useMathView},
@@ -581,10 +578,10 @@ async sub pre_header_initialize ($c) {
581578
&& after($c->{set}->answer_date, $c->submitTime)),
582579
showMessages => !$showOnlyCorrectAnswers,
583580
showCorrectAnswers => (
584-
$will{showProblemGrader} || ($c->{submitAnswers} && $c->{showCorrectOnRandomize}) ? 2
581+
$c->{submitAnswers} && $c->{showCorrectOnRandomize} ? 2
585582
: !$c->{previewAnswers} && after($c->{set}->answer_date, $c->submitTime)
586583
? ($ce->{pg}{options}{correctRevealBtnAlways} ? 1 : 2)
587-
: !$c->{previewAnswers} && $will{showCorrectAnswers} ? 1
584+
: $will{showProblemGrader} || (!$c->{previewAnswers} && $will{showCorrectAnswers}) ? 1
588585
: 0
589586
),
590587
debuggingOptions => getTranslatorDebuggingOptions($authz, $userID),
@@ -722,9 +719,6 @@ sub siblings ($c) {
722719

723720
my @items;
724721

725-
# Keep the grader open when linking to problems if it is already open.
726-
my %problemGraderLink = $c->{will}{showProblemGrader} ? (params => { showProblemGrader => 1 }) : ();
727-
728722
for my $problemID (@problemIDs) {
729723
if ($isJitarSet
730724
&& !$authz->hasPermissions($eUserID, 'view_unopened_sets')
@@ -795,7 +789,7 @@ sub siblings ($c) {
795789
@items,
796790
$c->tag(
797791
'a',
798-
$active ? () : (href => $c->systemLink($problemPage, %problemGraderLink)),
792+
$active ? () : (href => $c->systemLink($problemPage)),
799793
class => $class,
800794
$c->b($c->maketext('Problem [_1]', join('.', @seq)) . $status_symbol)
801795
)
@@ -806,7 +800,7 @@ sub siblings ($c) {
806800
@items,
807801
$c->tag(
808802
'a',
809-
$active ? () : (href => $c->systemLink($problemPage, %problemGraderLink)),
803+
$active ? () : (href => $c->systemLink($problemPage)),
810804
class => 'nav-link' . ($active ? ' active' : ''),
811805
$c->b($c->maketext('Problem [_1]', $problemID) . $status_symbol)
812806
)
@@ -970,10 +964,9 @@ sub nav ($c, $args) {
970964
}
971965

972966
my %tail;
973-
$tail{displayMode} = $c->{displayMode} if defined $c->{displayMode};
974-
$tail{showOldAnswers} = 1 if $c->{will}{showOldAnswers};
975-
$tail{showProblemGrader} = 1 if $c->{will}{showProblemGrader};
976-
$tail{studentNavFilter} = $c->param('studentNavFilter') if $c->param('studentNavFilter');
967+
$tail{displayMode} = $c->{displayMode} if defined $c->{displayMode};
968+
$tail{showOldAnswers} = 1 if $c->{will}{showOldAnswers};
969+
$tail{studentNavFilter} = $c->param('studentNavFilter') if $c->param('studentNavFilter');
977970

978971
return $c->tag(
979972
'div',
@@ -1133,10 +1126,8 @@ sub output_message ($c) {
11331126

11341127
# Output the problem grader if the user has permissions to grade problems
11351128
sub output_grader ($c) {
1136-
if ($c->{will}{showProblemGrader}) {
1137-
return WeBWorK::HTML::SingleProblemGrader->new($c, $c->{pg}, $c->{problem})->insertGrader;
1138-
}
1139-
1129+
return WeBWorK::HTML::SingleProblemGrader->new($c, $c->{pg}, $c->{problem})->insertGrader
1130+
if $c->{will}{showProblemGrader};
11401131
return '';
11411132
}
11421133

@@ -1444,7 +1435,7 @@ sub output_summary ($c) {
14441435
# Attempt summary
14451436
if ($c->{submitAnswers}) {
14461437
push(@$output, $c->attemptResults($pg));
1447-
} elsif ($will{checkAnswers} || $c->{will}{showProblemGrader}) {
1438+
} elsif ($will{checkAnswers}) {
14481439
push(
14491440
@$output,
14501441
$c->tag(

lib/WeBWorK/ContentGenerator/ShowMeAnother.pm

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -155,10 +155,9 @@ async sub pre_header_initialize ($c) {
155155
}
156156

157157
# Disable options that are not applicable for showMeAnother.
158-
$c->{can}{recordAnswers} = 0;
159-
$c->{can}{checkAnswers} = 0; # This is turned on if the showMeAnother conditions are met below.
160-
$c->{can}{getSubmitButton} = 0;
161-
$c->{can}{showProblemGrader} = 0;
158+
$c->{can}{recordAnswers} = 0;
159+
$c->{can}{checkAnswers} = 0; # This is turned on if the showMeAnother conditions are met below.
160+
$c->{can}{getSubmitButton} = 0;
162161

163162
if ($c->stash->{isPossible}) {
164163
$c->{can}{showCorrectAnswers} =

lib/WeBWorK/Utils.pm

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -357,7 +357,6 @@ sub generateURLs ($c, %params) {
357357
for my $name ('displayMode', 'showCorrectAnswers', 'showHints', 'showOldAnswers', 'showSolutions') {
358358
$args{$name} = [ $c->param($name) ] if defined $c->param($name) && $c->param($name) ne '';
359359
}
360-
$args{showProblemGrader} = 1;
361360
} else {
362361
$routePath = $c->url_for('problem_list', setID => $params{set_id});
363362
}

templates/ContentGenerator/GatewayQuiz.html.ep

Lines changed: 6 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -388,6 +388,7 @@
388388
%
389389
<%= form_for $action, name => 'gwquiz', method => 'POST', class => 'problem-main-form mt-0', begin =%>
390390
<%= $c->hidden_authen_fields =%>
391+
<%= hidden_field courseID => $ce->{courseName} =%>
391392
%
392393
% # Hacks to use a javascript link to trigger previews and jump to subsequent pages of a multipage test.
393394
<%= hidden_field pageChangeHack => '' =%>
@@ -533,7 +534,7 @@
533534
% $recordMessage = tag('div', class => 'alert alert-danger d-inline-block mb-2 p-1',
534535
% maketext('CORRECT ANSWERS SHOWN ONLY -- ANSWERS NOT RECORDED')
535536
% );
536-
% } elsif ($c->{will}{checkAnswers} || $c->{will}{showProblemGrader}) {
537+
% } elsif ($c->{will}{checkAnswers}) {
537538
% $recordMessage = tag('div', class => 'alert alert-danger d-inline-block mb-2 p-1',
538539
% maketext('ANSWERS ONLY CHECKED -- ANSWERS NOT RECORDED')
539540
% );
@@ -549,7 +550,7 @@
549550
% # Show the jump to anchor.
550551
<div id="<%= "prob$i" %>" tabindex="-1"><%= $recordMessage %></div>
551552
% # Output the problem header.
552-
<h2><%= maketext('Problem [_1].', $i + 1) %></h2>
553+
<h2 class="gw-problem-number"><%= maketext('Problem [_1].', $i + 1) %></h2>
553554
<span class="problem-sub-header">
554555
% if ($c->{can}{showTemplateIds}) {
555556
<%= '('
@@ -738,20 +739,10 @@
738739
<p><em><%= maketext('Note: grading the test grades all problems, not just those on this page.') %></em></p>
739740
% }
740741
%
741-
% if ($c->{can}{showProblemGrader}) {
742+
% if ($c->{can}{showCorrectAnswers}) {
742743
<div class="submit-buttons-container col-12 my-2">
743-
% if ($c->{can}{showCorrectAnswers}) {
744-
<%= submit_button maketext('Show Correct Answers'), name => 'showCorrectAnswers',
745-
class => 'btn btn-primary mb-1' =%>
746-
% }
747-
% if ($c->{will}{showProblemGrader}) {
748-
<%= submit_button maketext('Hide Problem Graders'), name => 'hideProblemGrader',
749-
class => 'btn btn-primary mb-1' =%>
750-
<%= hidden_field showProblemGrader => 1 =%>
751-
% } else {
752-
<%= submit_button maketext('Show Problem Graders'), name => 'showProblemGrader',
753-
class => 'btn btn-primary mb-1' =%>
754-
% }
744+
<%= submit_button maketext('Show Correct Answers'), name => 'showCorrectAnswers',
745+
class => 'btn btn-primary mb-1' =%>
755746
</div>
756747
% }
757748
%

0 commit comments

Comments
 (0)