refactor feedback form category/service selection #2497
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
thecristen
commented
Apr 8, 2025
| Keyword.new() | ||
| |> Keyword.put(:key, value) | ||
| |> Keyword.put(:value, value) | ||
| |> Keyword.put(:"aria-label", value) |
There was a problem hiding this comment.
Adding aria-labels here per the advice here to improve screen reader handling,
anthonyshull
approved these changes
Apr 11, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary of changes
Asana Ticket: Feedback form | Form refresh breaks subject selection
After consultation with some design-savvy folk, I've decided to adjust the form this way.
Instead of a dynamically-populated
<select>, which was subject to data loss on refresh and probably wasn't particularly accessible anyway, we use a simpler static<select>with<optgroup>to separate the groups.It's a bit long, but this was agreed upon to be the least bad quick option.
One side effect is that we can remove the "Category" radio buttons, since we can now infer the selected category from this big dropdown.
And this all needed some downstream changes, to ensure validation still works and that the right data is sent.
Caution
The Cypress test only mostly works: the mechanism for retrieving the "sent" data is flawed when many tests are happening together, I think. But I think fixing this is out of scope 👀