-
Notifications
You must be signed in to change notification settings - Fork 134
Add focus trap functionality to modal component #367
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
Conversation
WalkthroughThe changes introduce a "Focus Trap" feature to the modal component, enhancing accessibility by keeping keyboard focus within the modal when enabled. This is achieved by adding a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ModalComponent
participant useFocusTrap
User->>ModalComponent: Open modal (with focusTrap=true)
ModalComponent->>useFocusTrap: Initialize and activate focus trap
useFocusTrap-->>ModalComponent: Focus is restricted within modal
User->>ModalComponent: Interacts with modal (keyboard navigation)
ModalComponent->>useFocusTrap: Deactivate focus trap on close
useFocusTrap-->>ModalComponent: Focus trap removed
Poem
Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
✅ Deploy Preview for sensational-seahorse-8635f8 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed:
Length of output: 1109 🏁 Script executed:
Length of output: 11676 @wazeerc Hi there! I see you've pinged me in your PR about adding focus trap functionality to the modal component. Based on what I can see, you've implemented:
This is a great accessibility improvement that helps with WCAG 2.2 compliance for focus order (Level A). How can I help you with this PR? I can:
Let me know what you need! |
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
docs/components/modal.md (1)
180-180
: Remove trailing colon from heading for markdown style compliance.The heading "### Props:" has a trailing colon, which is flagged by markdownlint (MD026). Remove the colon for consistency with markdown style guidelines.
-### Props: +### Props🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
180-180: Trailing punctuation in heading
Punctuation: ':'(MD026, no-trailing-punctuation)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.json
is excluded by!**/package-lock.json
📒 Files selected for processing (5)
docs/components/modal.md
(2 hunks)docs/components/modal/examples/FwbModalExample.vue
(3 hunks)docs/components/modal/examples/FwbModalExampleFocusTrap.vue
(1 hunks)package.json
(1 hunks)src/components/FwbModal/FwbModal.vue
(3 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
docs/components/modal.md
180-180: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
🔇 Additional comments (16)
package.json (1)
72-72
: Appropriate dependency added for focus trap functionality.The addition of
@vueuse/integrations
at version^13.1.0
is necessary to support the focus trap functionality through theuseFocusTrap
composable.docs/components/modal/examples/FwbModalExample.vue (3)
12-12
: LGTM: Modal example properly integrates the focus trap prop.The
focus-trap
prop is correctly passed to theFwbModal
component to enable accessibility functionality.
60-60
: LGTM: Interface properly updated with focusTrap prop.The
ModalProps
interface is correctly extended with the optional boolean property for focus trap functionality.
69-69
: LGTM: Default value correctly configured.Setting the default value of
focusTrap
tofalse
makes the property opt-in, which is appropriate since it's a new feature that changes behavior.docs/components/modal/examples/FwbModalExampleFocusTrap.vue (1)
1-15
: LGTM: Excellent example demonstrating focus trap functionality.This example component effectively demonstrates the difference between modals with and without focus trap by placing them side-by-side with appropriate trigger text labels. This makes it clear to users how the feature works and when to use it.
src/components/FwbModal/FwbModal.vue (8)
65-66
: LGTM: Imports properly updated for focus trap functionality.The component correctly imports the
useFocusTrap
composable from@vueuse/integrations
and addsnextTick
and lifecycle hooks needed for the implementation.
75-75
: LGTM: Interface properly updated with focusTrap prop.The optional boolean property for focus trap functionality is correctly added to the component's props interface.
83-83
: LGTM: Default value correctly configured.Setting the default value of
focusTrap
tofalse
makes the property opt-in, which is appropriate for a new accessibility feature that changes keyboard behavior.
127-131
: LGTM: Focus trap implementation looks good.The focus trap is properly initialized with appropriate options:
- Using the modal reference as the trapped element
- Setting
immediate: false
to allow manual activation- Smart initial focus targeting the close button if available, otherwise the modal itself
This implementation aligns with accessibility best practices.
133-140
: LGTM: Focus trap activation implemented correctly.The focus trap is properly activated after the next tick (allowing the DOM to fully render) when the
focusTrap
prop is enabled. This conditional activation ensures the feature is only applied when requested.
142-144
: LGTM: Cleanup handled properly.The focus trap is correctly deactivated in the
onBeforeUnmount
lifecycle hook, ensuring that keyboard focus is properly released when the modal is closed.
101-111
: Additional modal positioning options enhance flexibility.The modal positioning options have been expanded with a comprehensive set of placement variations (top/center/bottom combined with start/center/end), which enhances the component's flexibility.
87-99
: Modal size options have been expanded.The modal size classes now include additional options like
xs
andsm
, providing more granular control over the modal's width.docs/components/modal.md (3)
7-7
: Import for focus trap example is correct.The new import for
FwbModalExampleFocusTrap
is appropriate and matches the new documentation section.
162-177
: Focus Trap documentation is clear and accurate.The new section effectively explains the feature, its accessibility benefits, and provides a clear usage example. No improvements needed.
182-188
: API table updates are accurate and comprehensive.The new and updated props are clearly documented, matching the implementation and PR objectives.
Why?
According to Focus Order (Level A) from WCAG 2.2 - A web page implements modal dialogs via scripting. When the trigger button is activated, a dialog opens and focus is set within the dialog. As long as the dialog is open, focus is limited to the elements of the dialog. When the dialog is dismissed, focus returns to the button or the element following the button.
Summary by CodeRabbit