From 0a3b2b9636e6f33b0f6eea227b60dd08b97c73ac Mon Sep 17 00:00:00 2001 From: Alex Jordan Date: Sun, 26 Jul 2026 11:33:15 -0700 Subject: [PATCH] buttons for reordering problems in a set details page Co-Authored-By: Claude Sonnet 5 --- .../js/ProblemSetDetail/problemsetdetail.js | 198 ++++++++++++- .../Instructor/ProblemSetDetail.html.ep | 261 ++++++++++-------- 2 files changed, 336 insertions(+), 123 deletions(-) diff --git a/htdocs/js/ProblemSetDetail/problemsetdetail.js b/htdocs/js/ProblemSetDetail/problemsetdetail.js index 1b9be297ee..61f471fd2c 100644 --- a/htdocs/js/ProblemSetDetail/problemsetdetail.js +++ b/htdocs/js/ProblemSetDetail/problemsetdetail.js @@ -42,11 +42,192 @@ } }; + const getSourceFilePath = (id) => + document.getElementById(`problem.${id}.source_file_id`)?.value || + document.getElementById(`problem_${id}_default_source_file`)?.value || + ''; + + // Compute the "same source file" alerts for every problem based on the current order of the list. + const updateRepeatFileAlerts = () => { + if (!container) return; + const repeatFileText = container.dataset.repeatFileText; + if (!repeatFileText) return; + + const shownYet = new Map(); + for (const item of container.querySelectorAll('.psd_list_item')) { + const row = item.querySelector(':scope > .problem_detail_row'); + const alertContainer = row?.querySelector('.pdr_repeat_file_alert'); + if (!alertContainer) return; + + alertContainer.innerHTML = ''; + + const problemID = item.id.replace('psd_list_item_', ''); + const sourceFile = getSourceFilePath(problemID).replace(/^\//, '').replace(/\.\./g, ''); + if (!sourceFile || sourceFile.startsWith('group:')) return; + + if (shownYet.has(sourceFile)) { + const alert = document.createElement('div'); + alert.className = 'alert alert-danger p-1 mb-2 fw-bold'; + alert.textContent = repeatFileText.replace('[_1]', shownYet.get(sourceFile)); + alertContainer.append(alert); + } else { + shownYet.set(sourceFile, row.querySelector('.pdr_problem_number')?.textContent ?? ''); + } + } + }; + const setProblemNumberFields = () => { container.querySelectorAll('.psd_list_item .pdr_problem_number').forEach((num) => (num.textContent = '')); recursiveRenumber(Sortable.get(container).toArray()); + updateRepeatFileAlerts(); + }; + + // Buttons to rearrange problems. Either up/down for rearranging order, or buttons for nestin/de-nesting JITAR + // problems. Buttons are disabled when the corresponding action isn't possible. + const updateReorderButtonStates = () => { + if (!container) return; + for (const list of [container, ...container.querySelectorAll('.sortable-branch')]) { + const items = list.querySelectorAll(':scope > .psd_list_item'); + for (const [index, item] of items.entries()) { + const upButton = item.querySelector(':scope > .problem_detail_row .psd_move_up'); + const downButton = item.querySelector(':scope > .problem_detail_row .psd_move_down'); + if (upButton) upButton.disabled = index === 0; + if (downButton) downButton.disabled = index === items.length - 1; + + // Nesting nests the item under its previous sibling, so it needs a previous sibling to nest under. + // Denesting moves the item up to its parent's list, so it isn't possible at the top level. + const nestButton = item.querySelector(':scope > .problem_detail_row .psd_nest'); + const denestButton = item.querySelector(':scope > .problem_detail_row .psd_denest'); + if (nestButton) nestButton.disabled = index === 0; + if (denestButton) denestButton.disabled = list === container; + } + } + }; + + // Recompute the nestDepth property (used by the drag-and-drop "put" rule) for a sub-list and all of its + // descendant sub-lists after the sub-list has moved to a new position in the tree. + const recomputeNestDepth = (list) => { + if (!list) return; + list.nestDepth = list.parentNode.closest('.sortable-branch').nestDepth + 1; + list.querySelectorAll(':scope > .psd_list_item > .sortable-branch').forEach(recomputeNestDepth); + }; + + // Show or hide each row's expand/collapse button depending on whether its sub-list currently has any children. + const updateCollapseButtonVisibility = () => { + if (!container) return; + for (const list of container.querySelectorAll('.sortable-branch')) { + if (list.firstElementChild) list.parentNode.collapseButton?.classList.remove('d-none'); + else list.parentNode.collapseButton?.classList.add('d-none'); + } + }; + + // Bookkeeping after any keyboard-driven reorder/nest/denest: update the hidden problem number/parent fields, + // re-enable/disable fields whose relevance depends on tree position, update this row's button states, and + // manage focus. + const finishReorder = (item, button, fallbackSelectors) => { + setProblemNumberFields(); + disableFields(); + updateReorderButtonStates(); + updateCollapseButtonVisibility(); + + if (!button.disabled) { + button.focus(); + return; + } + for (const selector of fallbackSelectors) { + const fallback = item.querySelector(selector); + if (fallback && !fallback.disabled) { + fallback.focus(); + return; + } + } + }; + + const prefersReducedMotion = window.matchMedia('(prefers-reduced-motion: reduce)').matches; + + // Animate elements that just moved from where they used to be to where they ended up (the "FLIP" technique), + // so a move up/down reads as the two rows swapping places instead of an unexplained instant jump. Only the + // vertical position can change here, since these are stacked list items. + const animateReorder = (elementsAndFirstRects) => { + if (prefersReducedMotion) return; + for (const [el, firstRect] of elementsAndFirstRects) { + const deltaY = firstRect.top - el.getBoundingClientRect().top; + if (!deltaY) continue; + el.style.transition = 'none'; + el.style.transform = `translateY(${deltaY}px)`; + el.getBoundingClientRect(); // Force a reflow so the starting position above is rendered before animating. + requestAnimationFrame(() => { + el.style.transition = 'transform 150ms ease-in-out'; + el.style.transform = ''; + }); + el.addEventListener('transitionend', () => (el.style.transition = ''), { once: true }); + } }; + const moveItem = (button, direction) => { + const item = button.closest('.psd_list_item'); + const sibling = direction === 'up' ? item.previousElementSibling : item.nextElementSibling; + if (!sibling?.classList.contains('psd_list_item')) return; + + const firstRects = [item, sibling].map((el) => [el, el.getBoundingClientRect()]); + + if (direction === 'up') item.parentNode.insertBefore(item, sibling); + else item.parentNode.insertBefore(sibling, item); + + animateReorder(firstRects); + + finishReorder(item, button, [ + direction === 'up' ? '.psd_move_down' : '.psd_move_up', + '.psd_denest', + '.psd_nest' + ]); + }; + + const nestItem = (button) => { + const item = button.closest('.psd_list_item'); + const prevSibling = item.previousElementSibling; + if (!prevSibling?.classList.contains('psd_list_item')) return; + + const targetList = prevSibling.querySelector(':scope > .sortable-branch'); + if (!targetList) return; + + targetList.append(item); + recomputeNestDepth(item.querySelector(':scope > .sortable-branch')); + + // Make sure the item's new position is actually visible, in case its new parent's children were collapsed. + bootstrap.Collapse.getInstance(targetList)?.show(); + + finishReorder(item, button, ['.psd_denest', '.psd_move_up', '.psd_move_down']); + }; + + const denestItem = (button) => { + const item = button.closest('.psd_list_item'); + const currentList = item.parentNode; + if (currentList === container) return; + + const parentItem = currentList.closest('.psd_list_item'); + const grandParentList = parentItem.parentNode; + + grandParentList.insertBefore(item, parentItem.nextSibling); + recomputeNestDepth(item.querySelector(':scope > .sortable-branch')); + + finishReorder(item, button, ['.psd_nest', '.psd_move_up', '.psd_move_down']); + }; + + container?.addEventListener('click', (e) => { + const button = e.target.closest('.psd_move_up, .psd_move_down, .psd_nest, .psd_denest'); + if (!button || button.disabled) return; + + // Since focus stays on (or moves to another) reorder button after the click is handled below, the + // tooltip's own focus/hover triggers won't naturally hide it. Hide it explicitly so it doesn't get stuck. + bootstrap.Tooltip.getInstance(button)?.hide(); + + if (button.classList.contains('psd_move_up')) moveItem(button, 'up'); + else if (button.classList.contains('psd_move_down')) moveItem(button, 'down'); + else if (button.classList.contains('psd_nest')) nestItem(button); + else denestItem(button); + }); + const setSortable = (list) => { // Set up the bootstrap collapses. Note that if a list is empty, then the collapse is shown. Since it is empty // you still see nothing, but dragging into the list is smoother because it doesn't need to be expanded first. @@ -112,6 +293,7 @@ onSort() { setProblemNumberFields(); disableFields(); + updateReorderButtonStates(); }, onRemove() { if (hiddenCollapse?._isTransitioning) @@ -121,10 +303,7 @@ else hiddenCollapse?.show(); }, onChange(evt) { - container.querySelectorAll('.sortable-branch').forEach((list) => { - if (list.firstElementChild) list.parentNode.collapseButton?.classList.remove('d-none'); - else list.parentNode.collapseButton?.classList.add('d-none'); - }); + updateCollapseButtonVisibility(); if (evt.from.querySelectorAll('.psd_list_item').length < 2) evt.from.parentNode.collapseButton?.classList.add('d-none'); } @@ -182,6 +361,8 @@ if (container) setSortable(container); container?.querySelectorAll('.sortable-branch').forEach(setSortable); + updateReorderButtonStates(); + updateRepeatFileAlerts(); // Recursively convert the sortable list to a tree. Each entry of the tree has the problem id, the parent id if it // has a parent, and a list of the child problems (each of which is again an entry of the tree). @@ -211,7 +392,10 @@ // Initialize tooltips. document - .querySelectorAll('.psd_view,.psd_edit,.pdr_render,.pdr_grader,.pdr_handle > i') + .querySelectorAll( + '.psd_view,.psd_edit,.pdr_render,.pdr_grader,.pdr_handle > i,' + + '.psd_move_up,.psd_move_down,.psd_nest,.psd_denest' + ) .forEach((el) => new bootstrap.Tooltip(el)); // If editing for user(s), then disable drag and drop re-ordering of problems. @@ -345,9 +529,7 @@ const ro = { problemSeed: document.getElementById(`problem.${id}.problem_seed_id`)?.value ?? 1, - sourceFilePath: - document.getElementById(`problem.${id}.source_file_id`)?.value || - document.getElementById(`problem_${id}_default_source_file`)?.value + sourceFilePath: getSourceFilePath(id) }; if (ro.sourceFilePath.startsWith('group')) { diff --git a/templates/ContentGenerator/Instructor/ProblemSetDetail.html.ep b/templates/ContentGenerator/Instructor/ProblemSetDetail.html.ep index c291ffbe12..a9a80b8e34 100644 --- a/templates/ContentGenerator/Instructor/ProblemSetDetail.html.ep +++ b/templates/ContentGenerator/Instructor/ProblemSetDetail.html.ep @@ -16,8 +16,8 @@ <%= javascript getAssetURL( $ce, 'node_modules/flatpickr/dist/l10n/' - . ($ce->{language} =~ /^el/ ? 'gr' : ($ce->{language} =~ s/^(..).*/$1/gr)) - . '.js' + . ($ce->{language} =~ /^el/ ? 'gr' : ($ce->{language} =~ s/^(..).*/$1/gr)) + . '.js' ), defer => undef =%> % } <%= javascript getAssetURL($ce, 'node_modules/flatpickr/dist/plugins/confirmDate/confirmDate.js'), @@ -204,7 +204,7 @@
<%= maketext('Set Parameters') %>
<%= $c->fieldTable($userToShow, $setID, undef, $setRecord, - $editingSetVersion + $editingSetVersion ? $db->getSetVersion($userToShow, $setID, $editingSetVersion) : $db->getUserSet($userToShow, $setID)) =%>
@@ -393,8 +393,6 @@ %
- % my %shownYet; - % my $repeatFile; % my @problemRows; % % for my $problemID (@$problemIDList) { @@ -414,16 +412,6 @@ % my $problemFile = % ((param("problem.$problemID.source_file") || $sourceFile) =~ s|^/||r) =~ s|\.\.||gr; % - % # Warn of repeat problems - % if (defined $shownYet{$problemFile}) { - % my $prettyID = $shownYet{$problemFile}; - % $prettyID = join('.', jitar_id_to_seq($prettyID)) if $isJitarSet; - % $repeatFile = maketext('This problem uses the same source file as number [_1].', $prettyID); - % } else { - % $shownYet{$problemFile} = $problemID unless $problemFile =~ /^group:/; - % $repeatFile = ''; - % } - % % my $error = $c->checkFile($problemFile, undef); % % my $problemNumber = $problemID; @@ -458,7 +446,7 @@ % my $problemRow = begin
-
+
<%= $problemNumber %> % if (!@editForUser) { @@ -467,108 +455,152 @@ % }
- <%= $collapseButton =%> - <%= hidden_field "prob_num_$problemID" => $lastProblemNumber, - id => "prob_num_$problemID" =%> - <%= hidden_field "prob_parent_id_$problemID" => $parentID, - id => "prob_parent_id_$problemID" =%> - % - % # Show the "Render Problem", "Edit Problem", and "Open in New Window" links if there - % # is a well defined problem to view. This is when editing a homework set, editing a - % # gateway set version, or editing a gateway set and the problem is not drawing from - % # grouping set. Really the only case needed here is that of a group problem. - % if (!$isGatewaySet || $editingSetVersion || $problemFile !~ /^group/) { - - <%= link_to $c->systemLink(url_for( - 'instructor_problem_editor_withset_withproblem', - setID => $setID, problemID => $problemID - )), - class => 'psd_edit btn btn-secondary btn-sm', - target => 'WW_Editor', - data => { - bs_toggle => 'tooltip', - bs_placement => 'top', - bs_title => maketext('Edit Problem') - }, - begin =%> - - - <% end =%> - % my $problemLink; - % if ($isGatewaySet) { - % # If editing a set version, then open the actual set version. - % # Otherwise, the best that can be done is to use an undefined set. - % if ($editingSetVersion) { - % my ($problemNumber, $pageNumber) = - % get_test_problem_position($db, $problemToShow); - % $problemLink = $c->systemLink(url_for('gateway_quiz', - % setID => $problemToShow->set_id - % . ',v' . $problemToShow->version_id, - % problemID => $problemToShow->problem_id - %), - % params => { - % effectiveUser => $editForUser[0], - % currentPage => $pageNumber - % } - % )->fragment("prob$problemNumber"); + % if (!@editForUser) { +
+
+ + +
+ % if ($isJitarSet) { +
+ + +
+ % } + <%= $collapseButton =%> +
+ % } +
+ <%= hidden_field "prob_num_$problemID" => $lastProblemNumber, + id => "prob_num_$problemID" =%> + <%= hidden_field "prob_parent_id_$problemID" => $parentID, + id => "prob_parent_id_$problemID" =%> + % + % # Show the "Render Problem", "Edit Problem", and "Open in New Window" links if + % # there is a well defined problem to view. This is when editing a homework set, + % # editing a gateway set version, or editing a gateway set and the problem is not + % # drawing from a grouping set. Really the only case needed here is that of a + % # group problem. + % if (!$isGatewaySet || $editingSetVersion || $problemFile !~ /^group/) { + + <%= link_to $c->systemLink(url_for( + 'instructor_problem_editor_withset_withproblem', + setID => $setID, problemID => $problemID + )), + class => 'psd_edit btn btn-secondary btn-sm', + target => 'WW_Editor', + data => { + bs_toggle => 'tooltip', + bs_placement => 'top', + bs_title => maketext('Edit Problem') + }, + begin =%> + + + <% end =%> + % my $problemLink; + % if ($isGatewaySet) { + % # If editing a set version, then open the actual set version. + % # Otherwise, the best that can be done is to use an undefined set. + % if ($editingSetVersion) { + % my ($problemNumber, $pageNumber) = + % get_test_problem_position($db, $problemToShow); + % $problemLink = $c->systemLink(url_for('gateway_quiz', + % setID => $problemToShow->set_id + % . ',v' . $problemToShow->version_id, + % problemID => $problemToShow->problem_id + %), + % params => { + % effectiveUser => $editForUser[0], + % currentPage => $pageNumber + % } + % )->fragment("prob$problemNumber"); + % } else { + % $problemLink = $c->systemLink( + % url_for('gateway_quiz', setID => 'Undefined_Set', problemID => '1'), + % params => { + % effectiveUser => + % @editForUser == 1 ? $editForUser[0] : param('user'), + % problemSeed => + % $problemToShow ? $problemToShow->problem_seed : '', + % sourceFilePath => $problemToShow && $problemToShow->source_file + % ? $problemToShow->source_file + % : $globalProblems->{$problemID}->source_file + % } + % ) + % } % } else { % $problemLink = $c->systemLink( - % url_for('gateway_quiz', setID => 'Undefined_Set', problemID => '1'), + % url_for('problem_detail', setID => $setID, problemID => $problemID), % params => { - % effectiveUser => - % @editForUser == 1 ? $editForUser[0] : param('user'), - % problemSeed => $problemToShow ? $problemToShow->problem_seed : '', - % sourceFilePath => $problemToShow && $problemToShow->source_file - % ? $problemToShow->source_file - % : $globalProblems->{$problemID}->source_file + % effectiveUser => @editForUser == 1 ? $editForUser[0] : param('user') % } % ) % } - % } else { - % $problemLink = $c->systemLink( - % url_for('problem_detail', setID => $setID, problemID => $problemID), - % params => { - % effectiveUser => @editForUser == 1 ? $editForUser[0] : param('user') - % } - % ) + <%= link_to $problemLink, + class => 'psd_view btn btn-secondary btn-sm', + target => 'WW_View', + data => { + bs_toggle => 'tooltip', + bs_placement => 'top', + bs_title => maketext('Open in New Window') + }, + begin =%> + + + <% end =%> % } - <%= link_to $problemLink, - class => 'psd_view btn btn-secondary btn-sm', - target => 'WW_View', - data => { - bs_toggle => 'tooltip', - bs_placement => 'top', - bs_title => maketext('Open in New Window') - }, - begin =%> - - - <% end =%> - % } - % if ($authz->hasPermissions(param('user'), 'problem_grader')) { - <%= link_to $c->systemLink(url_for( - 'instructor_problem_grader', - setID => $setID, - problemID => $problemID - )), - class => "pdr_grader btn btn-secondary btn-sm", - data => { - bs_toggle => "tooltip", - bs_placement => "top", - bs_title => maketext("Grade Problem") - }, - begin =%> - - - <% end =%> - % } + % if ($authz->hasPermissions(param('user'), 'problem_grader')) { + <%= link_to $c->systemLink(url_for( + 'instructor_problem_grader', + setID => $setID, + problemID => $problemID + )), + class => "pdr_grader btn btn-secondary btn-sm", + data => { + bs_toggle => "tooltip", + bs_placement => "top", + bs_title => maketext("Grade Problem") + }, + begin =%> + + + <% end =%> + % } +
<%= $source_file_parts[0] =%> @@ -631,9 +663,7 @@ ) =%>
- % if ($repeatFile) { -
<%= $repeatFile %>
- % } +
\ <% if ($error) { =%>\
<%= $error %>
@@ -647,7 +677,8 @@ % push @problemRows, $problemRow->(); % } % -
    +
      % if ($isJitarSet) { % # If this is a jitar set then print nested lists. % my $nestedIDHash = {};