-
Notifications
You must be signed in to change notification settings - Fork 39
fix: a11y improvements for finder #5221
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
base: main
Are you sure you want to change the base?
Conversation
ColinBuyck
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm noticing that the focus stays on the next button when the user changes from one page to another (within the same section). My guess is that we'll have to use the ref approach for both rather than the getElementById. You can see in the recording that the focus reset works with the section change but not the step change. Lmk if you have any questions
Screen.Recording.2025-08-13.at.7.56.28.AM.mov
|
Oof just saw the wip label! Sorry for the premature comment 🦤 ping me when this is ready! |
✅ Deploy Preview for bloom-flagly ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for partners-bloom-dev ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for bloom-exygy-dev ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for bloom-public-seeds ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for bloom-lakeview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
bedd3d2 to
1f595ee
Compare
1f595ee to
2dca682
Compare
✅ Deploy Preview for bloom-angelopolis canceled.
|
✅ Deploy Preview for partners-bloom-dev ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for bloom-public-seeds ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for bloom-angelopolis ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for bloom-exygy-dev ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for bloom-lakeview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
| validateRentValues(getValues, clearErrors, setError) | ||
| }, | ||
| }} | ||
| ariaLabel={t(`finder.rent.minRentReader`)} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue: This label and the one below are unnecessary here and they should generally be reserved for when there is no visible label - we have a label element and assign it to the input with for, so we should be good to go
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This makes sense, but i remember adding that in below issue, as a11y improvement
CityOfDetroit#1544
Just double check if i should remove it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh this is my bad! i thought this string was identical to the t("finder.rent.minRent") label, if it's different can def stay!
emilyjablonski
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you so much for these updates! Just need to remove the aria label and then good to merge
This PR addresses #4913
Description
Adds finder a11y improvements from detroit page. It will be:
How Can This Be Tested/Reviewed?
turn on screen reader and walk through
/finder.Author Checklist:
yarn generate:clientand/or created a migration when requiredReview Process: