buttons for reordering problems in a set details page

[Alex-Jordan]

When in the Set Details page, there will now be buttons on each problem to move a problem up/down in sequence. The mouse click-and-drag tool is still there, but these buttons give a keyboard accessible way to rearrange the problems.

If the set is a JITAR set, there are also buttons for indenting/outdenting problems.

Buttons are disabled when appropriate (like you can't move problem #1 up in the list).

[drgrice1]

Conceptually I think that this is okay. However, I have been thinking about working on this, and this is not how I would implement this. There are several problems with this approach.

One issue is that it involves numerous buttons, which there really isn't room for. This is what I see on a narrow window. I took it to the extreme to really emphasize the point in this screenshot, but it happens already even when the window is not nearly this narrow.

Another issue is that when you click on one of the up/down buttons for a problem the switch of problems is almost imperceivable. In most sets the settings are usually all the same except for the source file. So since the numbering is automatically updated (and that is correct to do), the only way you can see the change is by watching the source file.

Another issue that I am seeing has to do with the tooltips. I think that this can be fixed with this pull request and its design though. The issue is that if you click on one of the up/down buttons the tooltip is revealed and the problem is switched with the one above or below. The keyboard focus goes with the switch to where the original problem now is, but the tooltip stays where it was. This seems to confuse Bootstrap's javascript, and the tooltip never seems to get closed. In the extreme you can end up with numerous tooltips open at a time as in the following picture.

I have also noticed that one of those tooltips gets completely stuck, and there seems to be no way to actually get it to go away. It seems to be the one on the first button used. I believe this can be fixed by taking care to close the tooltips in the javascript when the button is clicked.

What I envisioned, and had planned to implement when I got around to it, was an approach similar to what is now implemented for the PG drag and drop in the dragndrop.js file for the DragNDrop.pm module. The existing move buttons would serve as the focus point. The mouse could still be used to drag those around as it currently is, but the arrow keys could also be used to move the problems around when the move button is focused. The up and down arrows clearly would move a problem up or down, and in a JITAR set the left and right arrows increase or decrease the nesting level. This would all be aria announce as it is for the PG drag and drop. One advantage of this approach over the implementation in this pull request is that if you use the down arrow repeatedly, you would be able to quickly move a single problem down several positions. If you want to move a problem all the way from the top of the list to bottom, you could just hold the down arrow down, and keyboard repeat would take the problem down quickly. Also, with the css transitions similar to those used in the PG drag and drop it would make the movement of the problem more clear.

[Alex-Jordan]

I'm happy to defer to your recommendations here. I could withdraw this, or feed what you have described here to Claude. I probably won't directly intervene with coding changes myself on this one though.

Whatever things here (and with the other PRs I've made today) that we can complete, it will affect the Accessibility Guide I'm trying to complete before the release. It's getting down to a pretty short list of known issues (as far as webwork2 goes, that is).

[drgrice1]

We can perhaps go with this for now, and then change to what I was thinking of later when I have time to work on that.

[Alex-Jordan]

OK. I'll see what Claude can do for the smaller tweaks you identified.

[Alex-Jordan]

The issues you found are at least mostly addressed now. The method I directed for the first issue (about overcrowding) is maybe not the best though.

[drgrice1]

I find the "indent"/"outdent" terminology a bit ugly. Also, in a JITAR set it isn't really indentation. It is nesting, and in fact the wording everywhere else calls it nesting.

[Alex-Jordan]

There is another problem with this. Say problem 1 and 2 use the same file. Problem 2 will have an alert "This problem uses the same source file as number 1.". Then if I use the buttons to swap #1 with #2, now the new #1 still says "This problem uses the same source file as number 1.". I'm working on that.

[Alex-Jordan]

OK, the language (both user-facing and internal) is now about nest/denest instead of indent/outdent.

Also I moved the alerts for "This problem uses the same source file as number 1." from the template to the javascript, so it can be recalculated as things move. This is probably already an issue with the current mouse-driven rearrangement tool.

[drgrice1]

This is going to take some time to review and test properly. But here are some things I see at this point.

[Alex-Jordan]

OK, those code changes are in.

[drgrice1]

I think this looks good at this point.

[pstaabp]

I think the responsiveness is a bit odd for JITAR sets. Here's the large size:

and for a medium size:

and then for a narrow screen:

The 2nd four-pack of buttons (old ones) start on the top row, then in the medium size, go to the 2nd row and then on the narrow screen, back to the top row.

It may be less jarring to bring the source input box to the 2nd row on the medium screen.

[somiaj]

Looks good.

One thing I noticed (no fix needed) is when you copy/paste the name of a pg file into another problem, the 'duplicate warning' message doesn't appear until the order of the problems is changed. Further if you edit the filename of a duplicate file to a unique thing, the warning doesn't get removed until the problems are reordered. Would it be worth also updating the duplicate messages when the filename of a problem is edited?

[Alex-Jordan]

It may be less jarring to bring the source input box to the 2nd row on the medium screen.

This was fairly nontrivial. The top two lines of each problem's box use bootstrap grid, and then below that is a table with limited options for setting widths with bootstrap. But I like what I came up with better. See what you think.

@somiaj For what you noticed, I did not want to add even listeners for edits to the text field. But it could be done in the future.

[Alex-Jordan]

I think that this should get another pass by @drgrice1, since some things changed structurally to respond to @pstaabp's observations.

[drgrice1]

I think that the number of buttons that this adds, particularly for JITAR sets, is a challenge that I am not sure can be fully remedied.

This is what I see on a mobile device in a JITAR set:

Compare that to what is seen with the current release candidate branch:

Both screenshots were taken after collapsing all details and expanding all nesting.

The buttons don't fit on the first line, and moving the details collapse button down makes it so that the source file label and input get meshed as well.

This was not great before as certain screen widths were already a problem, but now things are worse. This is one of the reasons that I had mentioned the approach of just using the existing move button with arrow key controls. It also seems odd to me to have a move button that is for mouse cursor use only, and then to have completely separate buttons for keyboard controls.

We can go with this for now, but I think this needs a complete redesign in the future.

[drgrice1]

I just noticed this. It does not make sense that this click handler is on the container instead of directly on the buttons. That is not a good way to deal with this. This should be

Suggested change

	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);
	});
	for (const button of document.querySelectorAll('.psd_move_up, .psd_move_down, .psd_nest, .psd_denest')) {
	button.addEventListener('click', () => {
	// 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);
	});
	}

It is really a hack to use a click event listener on the container and then find the button that was clicked in the listener, when really you want a click on the button. Note that you don't even need to check that the button is disabled with this because disabled buttons do not receive the click event at all.

[Alex-Jordan]

OK, this change is made (not yet pushed). I'm going to see if there's a quick way to address the spacing issues you saw. (Yes, the whole thing should be refactored. In my opinion, also replacing the table in the lower part.)

[Alex-Jordan]

Pushed, with that click event listener change.

Also with changes that keep the collapse button at the top row. However, at the tiny screen size (smaller than the sm breakpoint), there is not enough room for all the things you mentioned for a nested JITAR. So only below the sm breakpoint, I made the buttons narrower. Then it all fits. Even if it's JITAR with a wide number like 8.20.8.

[drgrice1]

I think that is better, and good enough for now.