Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add label reordering #1130

Merged
merged 26 commits into from
Dec 4, 2024
Merged

Add label reordering #1130

merged 26 commits into from
Dec 4, 2024

Conversation

caseyhans
Copy link
Collaborator

Add the ability to optionally reorder labels "after" other sibling labels.

  • Checks that 'parent' field matches the parent of the 'after' sibling field, if given
  • Returns and refreshes entire label list on successful updates, creates, and deletes, so the list is always accurate
  • Removes scroll HTMX animation--unnecessary in this case since the forms are so small, and it feels a bit annoying if you're trying to see the entire tree while creating a new label

image

Copy link
Collaborator

@rabstejnek rabstejnek left a comment

Choose a reason for hiding this comment

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

Looks good!

I wonder though if it would be a good idea to add both a "Before" and "After" field, with validation that you can only provide one or the other. Technically you could do everything using "After", but if you want a fresh tag to be at the top you'd have to move all the other tags to be after it, so a tag list with 10 tags would require 9 operations to move a fresh tag to the top. This may be something we can wait on user feedback about to see if its really necessary.

Base automatically changed from refactor-htmx-create to main December 3, 2024 23:25
shapiromatron and others added 6 commits December 3, 2024 18:29
# Conflicts:
#	hawc/apps/assessment/templates/assessment/fragments/label_edit_row.html
#	hawc/apps/assessment/templates/assessment/label_list.html
# Conflicts:
#	hawc/apps/assessment/templates/assessment/fragments/label_row.html
Copy link
Owner

@shapiromatron shapiromatron left a comment

Choose a reason for hiding this comment

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

Looks great. I really like the hx-retarget trick where we design the html templates for the happy case but fallback to form replacement on error, really nice work.

I learned something new today!

context = self.get_context_data(form=form, **kwargs)
response = render(request, template, context)
if retarget_form:
response["HX-Retarget"] = "#label-edit-row-"
Copy link
Owner

Choose a reason for hiding this comment

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

didn't know about this, thanks, so basically on success it uses the target in the template, but if there's a form validation error it retargets and replaces itself.

https://htmx.org/reference/

a CSS selector that updates the target of the content update to a different element on the page

@shapiromatron
Copy link
Owner

Looks good!

I wonder though if it would be a good idea to add both a "Before" and "After" field, with validation that you can only provide one or the other. Technically you could do everything using "After", but if you want a fresh tag to be at the top you'd have to move all the other tags to be after it, so a tag list with 10 tags would require 9 operations to move a fresh tag to the top. This may be something we can wait on user feedback about to see if its really necessary.

I tried switching to after and it worked, but then I had a hard time with the wording for the form, I felt like the current approach was clearer to the user, even it it make require more edits. That's ok, I'd imagine that sorting is done rarely.

@shapiromatron shapiromatron merged commit 82eab7e into main Dec 4, 2024
6 checks passed
@shapiromatron shapiromatron deleted the label-reordering branch December 4, 2024 16:30
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