-
Notifications
You must be signed in to change notification settings - Fork 2
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
27. App info modal #62
Conversation
Visit the preview URL for this PR (updated for commit a5beeb4): https://tcl-66-smart-shopping-list--pr62-an-issue-27-8sui0olu.web.app (expires Thu, 11 Apr 2024 03:45:09 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: 2dc16cb2a79cbd6723fdc511b73e0743df1d18d0 |
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.
It looks good and works well but 2 things:
-
Modals should be closeable using the ESC key for accessibility:
https://www.w3.org/WAI/ARIA/apg/patterns/dialog-modal/ -
This isn't required to fix and more of an FYI, but it looks slightly wonky in Chrome on Windows:
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.
Nice work, looks good to me!
Description
Related Issue
closes #61
Acceptance Criteria
Type of Changes
accessibility, enhancement
Updates
Before
After
Screen.Recording.2024-04-03.at.7.05.56.PM.mov
Testing Steps / QA Criteria
Notes
I am not set in stone with the exact verbiage used. So if you want it to be said in a different way, feel free to let me know! I just used AI to help me come up with the wording.