Skip to content

Improve the admin course location management page. - #3224

Open
drgrice1 wants to merge 2 commits into
openwebwork:developfrom
drgrice1:location-delete-confirm
Open

drgrice1 wants to merge 2 commits into
openwebwork:developfrom
drgrice1:location-delete-confirm

Conversation

@drgrice1

Copy link
Copy Markdown
Member

Remove the "Confirm" checkbox and instead show a dialog when the "Take Action!" button is pressed in the case that the "Delete location:" radio is checked and a location is selected to be deleted. The dialog lists the locations that are going to be deleted.

The "No locations" option in the select is also gone. I don't see the point of that option.

Add some basic form validation to the location management form.

Server side don't throw an exception if "Edit location" is used when no location is selected (usually because none exist), and give a better message in this case. Although, that should now never happen since the "Edit location" radio and the select that goes with it is now disabled when no locations exist.

@drgrice1
drgrice1 force-pushed the location-delete-confirm branch from 8fad0ef to cf72972 Compare September 29, 2026 11:33
@somiaj

somiaj commented Sep 29, 2026

Copy link
Copy Markdown
Contributor

Any reason to keep the dropdown menu for edit/delete, and not just state, "Edit first selected location" and "Delete selected locations", and use the table to select them (similar to what is done elsewhere)? Also the delete warning, most places put that in an alert div, should that be done here?

A bigger change, but most places would use action tabs for the three options, so this interface look like most other ones.

I am okay with these changes as is, just some thoughts when testing things.

@drgrice1

Copy link
Copy Markdown
Member Author

I think the real question is, is there any reason to remove the dropdown for the edit/delete. To be quite frank the current dropdown is a better interface than what you are proposing. Particularly for the case of the "edit".

Reworking this into a tabbed interface is beyond what I care to do here. So I will leave it. This page (and the entire location feature) is one that I don't think many use at all.

@somiaj somiaj left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

Another thing I noticed, the edit location page has no button to get back (outside of the original location manager link) and there is no confirmation when you delete ip addresses.

@pstaabp

pstaabp commented Oct 6, 2026

Copy link
Copy Markdown
Member

Overall, this looks good. I generally don't use this, but made the location name: "math lab" and got an error for having a space. Can we replace that with an underscore like we do with problem sets?

@drgrice1

drgrice1 commented Oct 6, 2026

Copy link
Copy Markdown
Member Author

I am not sure that it is worth the effort to rework locations to show underscores as spaces and translate spaces into underscores on creation. Particularly since, as already mentioned, this is not feature that is used much, and even you just said you don't use it. If that is done, that will certainly be in a separate pull request.

Remove the "Confirm" checkbox and instead show a dialog when the "Take
Action!" button is pressed in the case that the "Delete location:" radio
is checked and a location is selected to be deleted. The dialog lists
the locations that are going to be deleted.

The "No locations" option in the select is also gone.  I don't see the
point of that option.

Add some basic form validation to the location management form.

Server side don't throw an exception if "Edit location" is used when no
location is selected (usually because none exist), and give a better
message in this case. Although, that should now never happen since the
"Edit location" radio and the select that goes with it is now disabled
when no locations exist.
@drgrice1
drgrice1 force-pushed the location-delete-confirm branch from 5ff9731 to 92925f2 Compare October 6, 2026 21:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants