Skip to content

Conversation

wazeerc
Copy link
Contributor

@wazeerc wazeerc commented Sep 27, 2025

Fixes issue #404

Summary by CodeRabbit

  • New Features
    • Improved modal focus handling: pressing Escape closes the modal without deactivating the focus trap prematurely, reducing unexpected focus escapes.
  • Documentation
    • Clarified Focus Trap behavior, noting that pressing Escape will still close the modal.

Copy link

netlify bot commented Sep 27, 2025

Deploy Preview for sensational-seahorse-8635f8 ready!

Name Link
🔨 Latest commit 7ffec75
🔍 Latest deploy log https://app.netlify.com/projects/sensational-seahorse-8635f8/deploys/68d7d1b00ad8820008e32d23
😎 Deploy Preview https://deploy-preview-408--sensational-seahorse-8635f8.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

Copy link
Contributor

coderabbitai bot commented Sep 27, 2025

Walkthrough

Adds escapeDeactivates: false to the modal’s focus-trap configuration to change Escape-key handling, and updates modal documentation to clarify that Escape still closes the modal.

Changes

Cohort / File(s) Summary of Changes
Documentation
docs/components/modal.md
Appended a clarifying sentence in the Focus Trap section stating that pressing Escape still closes the modal.
Modal Focus Trap Config
src/components/FwbModal/FwbModal.vue
Set useFocusTrap option escapeDeactivates: false to adjust how Escape interacts with the focus trap; no public API changes.

Sequence Diagram(s)

sequenceDiagram
    actor U as User
    participant M as Modal
    participant FT as Focus Trap

    rect rgba(230, 245, 255, 0.5)
    note over U,M: Open modal
    U->>M: Open
    M->>FT: Initialize useFocusTrap({ escapeDeactivates: false })
    end

    rect rgba(240, 255, 230, 0.5)
    note over U,FT: Keypress: Escape
    U->>FT: Press Escape
    FT-->>M: Focus trap remains active (no deactivation)
    M->>M: Close modal logic handles Escape
    M-->>U: Modal closes
    end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related issues

Possibly related PRs

Suggested labels

🔧 enhancement

Poem

A hop, a tap—Escape I press,
The trap stays put, avoids the mess.
The modal bows, then slips from sight,
Focus steady, UX just right.
Thump-thump joy from ears that flap—
I ship the fix with one small tap. 🐇✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title succinctly describes the main change of allowing the modal to close on Escape even when the focus trap is active, which aligns directly with the code and documentation updates in this pull request. It is clear, specific, and focused on the primary functionality adjustment without extraneous detail.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 16fb867 and 7ffec75.

📒 Files selected for processing (2)
  • docs/components/modal.md (1 hunks)
  • src/components/FwbModal/FwbModal.vue (1 hunks)

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@Sqrcz Sqrcz self-assigned this Sep 28, 2025
@Sqrcz Sqrcz added the 🔧 enhancement New feature or request label Sep 28, 2025
Copy link
Collaborator

@Sqrcz Sqrcz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice addition! Thank you 🙏

@Sqrcz Sqrcz merged commit 103e3a5 into themesberg:main Sep 28, 2025
14 checks passed
@wazeerc wazeerc deleted the fix/a11y/modal-esc-focustrap branch September 28, 2025 18:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔧 enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants