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

Update taxonomy forms #1000

Merged
merged 7 commits into from
Aug 22, 2022
Merged

Update taxonomy forms #1000

merged 7 commits into from
Aug 22, 2022

Conversation

allisonking
Copy link
Contributor

@allisonking allisonking commented Aug 19, 2022

Closes #998

Code Changes

  • Extends the forms for DataSubjects and DataUses based on the updated requirements
    • Adds a new prop extraFormFields and renames the original form to TaxonomyFormBase so other forms can build off of it
    • Adds more props to the hooks now that there are more unique fields between taxonomy types
      • Does this via typescript generics... this is my first time somewhat successfully using them, so very open to suggestions if I'm doing it weirdly 😅
  • Updates a few of the form inputs to take data-testid the same way as everyone else
  • Adds cypress tests

Steps to Confirm

  • Go to the /taxonomy page and try editing the forms for DataSubjects and DataUses

Pre-Merge Checklist

  • All CI Pipelines Succeeded
  • Documentation Updated:
    • documentation complete, or draft/outline provided (tag docs-team to complete/review on this branch)
    • documentation issue created (tag docs-team to complete issue separately)
  • Issue Requirements are Met
  • Relevant Follow-Up Issues Created
  • Update CHANGELOG.md

Description Of Changes

#977 was clever about reusing the same component and isolating unique attributes to hooks. This worked well back when the edit form was the same across every taxonomy type. Now that they're no longer the same, we are still able to use the same pattern, but it might make things a little harder to grok, in exchange for cleaner components.

image

image

DataCategory and DataQualifier forms still look the same.

@allisonking allisonking marked this pull request as ready for review August 19, 2022 22:03
@allisonking allisonking requested a review from a team August 19, 2022 22:04
Copy link
Contributor

@ThomasLaPiana ThomasLaPiana left a comment

Choose a reason for hiding this comment

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

LGTM! Played around with it and all worked as expected

@ThomasLaPiana
Copy link
Contributor

@allisonking this is a random thought and unrelated to this ticket, but I didn't want it to get lost and potentially we could open another issue for it.

The UX here feels a bit weird to me. When I click on an item from the list nothing happens. No feedback at all, instead I have to use the edit button to get the item to pop up in the side drawer (I think that's what it's called?) to see more info.

I think my preferred behavior here would be to click on the item in the list, and have it immediately open to the side. There is no need for an edit button if it automatically opens, and we could add a red delete button next to the update button.

Thoughts? The current method felt immediately unintuitive to me

@allisonking
Copy link
Contributor Author

@allisonking this is a random thought and unrelated to this ticket, but I didn't want it to get lost and potentially we could open another issue for it.

The UX here feels a bit weird to me. When I click on an item from the list nothing happens. No feedback at all, instead I have to use the edit button to get the item to pop up in the side drawer (I think that's what it's called?) to see more info.

I think my preferred behavior here would be to click on the item in the list, and have it immediately open to the side. There is no need for an edit button if it automatically opens, and we could add a red delete button next to the update button.

Thoughts? The current method felt immediately unintuitive to me

Yeah I definitely see where you're coming from. I have two concerns with this approach but maybe we can work around them:

  1. If users don't click on the item at all, they won't know the edit/delete features are available
  2. The parent fields, when clicked on, open up to reveal their children fields. This feels intuitive to me. If we wanted "edit on click" like you're describing, we'd have two things that happen when clicking on a parent field: opening up the children, and also opening up the side drawer. The children elements would only open up the side drawer as you're describing which works well, but the parents have a bit more going on. It seems like it'd be annoying if you're just trying to open up parents, for the side edit drawer to appear for each of them on each click.

Perhaps @mfbrown can weigh in, or we can ask the design team? Adding a video for easy reference

Screen.Recording.2022-08-22.at.10.45.36.AM.mov

@allisonking
Copy link
Contributor Author

Made a ticket for Thomas' suggestion here: #1003

Going to merge this in the mean time to keep work flowing

@allisonking allisonking merged commit c936c76 into main Aug 22, 2022
@allisonking allisonking deleted the aking-998-update-taxonomy-forms branch August 22, 2022 18:10
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.

Update edit taxonomy forms with updated scope
2 participants