Skip to content
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 (a11y): trap focus when navigation flyout is active #829

Merged
merged 4 commits into from
Nov 6, 2024

Conversation

virus-rpi
Copy link
Collaborator

Changes:

  • trap focus when navigation flyout is active

@virus-rpi virus-rpi self-assigned this Oct 17, 2024
@virus-rpi virus-rpi requested a review from a team as a code owner October 17, 2024 17:08
Copy link

netlify bot commented Oct 17, 2024

Deploy Preview for satellytes ready!

Name Link
🔨 Latest commit 2b7912e
🔍 Latest deploy log https://app.netlify.com/sites/satellytes/deploys/672b84f69e129300086e4e16
😎 Deploy Preview https://deploy-preview-829--satellytes.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
1 paths audited
Performance: 87 (🟢 up 3 from production)
Accessibility: 100 (no change from production)
Best Practices: 100 (no change from production)
SEO: 100 (no change from production)
PWA: 80 (no change from production)
View the detailed breakdown and full score reports

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

@virus-rpi virus-rpi mentioned this pull request Oct 17, 2024
20 tasks
Copy link
Member

@hofmann-felix hofmann-felix left a comment

Choose a reason for hiding this comment

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

What a brilliant solution to this problem! Nice work 🚀 I tested the behavior on different pages and it works great 👍

I left you two more suggestions to improve readability. Can you please take a look and see if you have the same opinion?

src/components/layout/header/menu-flyout.tsx Outdated Show resolved Hide resolved
src/components/layout/header/menu-flyout.tsx Outdated Show resolved Hide resolved
Copy link
Member

@hofmann-felix hofmann-felix left a comment

Choose a reason for hiding this comment

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

Thanks for applying my suggestions. There are still two places left where you can add the new constants. Please take a look at my comment before you merge

src/components/layout/header/menu-flyout.tsx Outdated Show resolved Hide resolved
@virus-rpi virus-rpi merged commit b634e86 into main Nov 6, 2024
6 checks passed
@virus-rpi virus-rpi deleted the chore/a11y/navigation/flyout branch November 6, 2024 15:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants