fix: make the exit confirmation an accessible dialog#974
Open
Anexus5919 wants to merge 1 commit into
Open
Conversation
|
@Anexus5919 is attempting to deploy a commit to the somiljain2024-4175's projects Team on Vercel. A member of the Team first needs to authorize it. |
Contributor
Author
|
@Somil450 @diksha78dev Kindly have a review on this pr. Thanks! |
The exit-confirm modal (duplicated in App and WorkoutScreen) was plain divs: no dialog role, no focus management or trap, no Escape to close, and App's "Stay" button had no styling so it rendered with low contrast. Extract a reusable ExitConfirmModal with role="dialog", aria-modal, and aria-labelledby; move focus to the safe "Stay" action on open, trap Tab within the dialog, close on Escape, and restore focus to the trigger on close. Use the app's btn-outline/btn-neon classes so both buttons have adequate contrast. Render it from both App and WorkoutScreen.
26d6cfb to
de859e5
Compare
Contributor
Author
|
@Somil450 @diksha78dev Kindly have a review on this pr. Thanks! |
Contributor
Author
|
This failing check is from |
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
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.
📌 Related Issue
Fixes #891
📝 Description
The "Confirm Exit" modal, duplicated in
App.tsxandWorkoutScreen.tsx, was built from plain<div>s: no dialog role, no focus management or trap, no Escape to close, and (in App) a "Stay" button with no styling that rendered low-contrast.🔹 What has been changed?
ExitConfirmModalwithrole="dialog",aria-modal="true", andaria-labelledby(id fromuseId); focus moved to the non-destructive Stay button on open; Tab focus trapped within the dialog (wraps both ends); Escape closes (invokes Stay); focus restored to the previously focused element on close; and the app'sbtn-outline(Stay) /btn-neon+ red (Exit) classes for adequate, theme-aware contrast.App.tsxandWorkoutScreen.tsxwith<ExitConfirmModal>(preserving each one's message and exit action).🔹 Why are these changes needed?
🛠️ Type of Change
🧪 Testing
✅ Tests Performed
npx tsc --noEmitclean;npx eslintclean for the change (the pre-existinguser?.uidexhaustive-deps warning on an unrelatedWorkoutScreeneffect is unchanged).🔹 Note on automated tests
No committed test: the focus-trap/Escape behavior needs
@testing-library/react, which can't currently run (@testing-library/domevicted fromnode_modules).🌐 Browsers Tested
Not applicable here (focus/dialog behavior; verified via reasoning and the Accessibility pane).
📷 Screenshots / Demo (if applicable)
A capture comparing the Stay/Exit button contrast and showing Tab staying within the modal would help.
📋 Checklist
💬 Additional Notes
Branched from
upstream/main; merges cleanly with the current base. Fixes both duplicated copies of the modal (App and WorkoutScreen) via one shared component.