-
Notifications
You must be signed in to change notification settings - Fork 1
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
fix: show all listings button labels #1568
Conversation
✅ Deploy Preview for detroit-partners-dev ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for detroit-storybook-dev ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for detroit-public-dev ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
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.
LGTM apart from the two missing Spanish translations for Reset
and Filter
which might be known / not available?
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 is working well! Just one question. I'm noticing that the issue references the language of "restart" rather than "reset". I personally prefer your wording but still wanted to call that out in case that is a strict request. Any thoughts here @sarahlazarich or @eajensenwa?
@ColinBuyck I don't think that's a strict requirement, we should we ok! Thanks for checking in. |
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.
LGTM once Emily's translation question is addressed 🏟️
I'm sorry I missed that question! I google translated for those, and the responses are here. Reset:
Filter:
|
* feat: add meaningful labels to filter buttons * feat: change reset button label * fix: add missing filter translations for spanish
* fix: reset focus for NVDA (#1556) * fix: first attempt to reset focus for nvda * fix: attemmpt with asPath approach * fix: reset focus in application file * fix: missing tabIndex value * fix: hidden new page approach * fix: new page with state * fix: functional navigation announcement * fix: cleaned up css approach * fix: remove unused css * fix: remove space typo * fix: nvda testing without alert * fix: re-instate alert * fix: simpler solution for cross browser support * fix: remove alert since focused * fix: announcement clean up * fix: variable naming * fix: announcement updates pr LH feedback * fix: heading level typo * fix: clean up string handling * fix: clean up rebase * feat: change labels for no min/max rent (#1569) * fix: release ux issues (#1571) * refactor: uptake uic siteheader (#1565) * fix: show all listings button labels (#1568) * feat: add meaningful labels to filter buttons * feat: change reset button label * fix: add missing filter translations for spanish * fix: cherry-picked forgot-password from core (#1566) * fix: cherry-picked commit from core * fix: update testing approach to mirror core * fix: undo core test mirroring * fix: resolve testing error * fix: revert unrelated changes * fix: space cleanup * fix: space cleanup pt 2 * fix: translations added * fix: custom focus management (#1572) * fix: custom focus management * fix: clean up + commenting * fix: remove auto fix from linter * fix: add missing screen reader only labels to fields (#1573) * fix: update to community type draggability (#1578) * fix: description grid misalignment (#1581) * fix: description grid misalignment * fix: description misalignment --------- Co-authored-by: ColinBuyck <53269332+ColinBuyck@users.noreply.github.com> Co-authored-by: Krzysztof Zięcina <kriss.kontakt@gmail.com> Co-authored-by: Emily Jablonski <65367387+emilyjablonski@users.noreply.github.com> Co-authored-by: Yazeed Loonat <YazeedLoonat@gmail.com>
Pull Request Template
Issue Overview
This PR addresses #1543
Description
Changed label of reset button to 'reset'. Added aria-label to filter button.
Removed obsolete translations from used keys.
How Can This Be Tested/Reviewed?
go to listings page, turn on screen reader. Check filter button without filters + after picking some filters, and reset button.
Checklist:
yarn generate:client
and/or created a migration if I made backend changes that require themReviewer Notes:
Steps to review a PR:
On Merge:
If you have one commit and message, squash. If you need each message to be applied, rebase and merge.