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: contact page accessibility problems #792

Merged
merged 15 commits into from
Feb 6, 2024
Merged

Conversation

virus-rpi
Copy link
Collaborator

@virus-rpi virus-rpi commented Jan 9, 2024

Changes:

  • Error message color changed from #FF0D35 to #EB0027 to make the color contrast WCAG AA compliant
  • Changes error messages so that they suggest a fix (e.g. "Your name is missing" to "The field name is not filled in. Please fill in the field name to send the message.")
  • Made the honeypot fields aria-invisible so that they don't get read by screen readers
  • Added a aria-label to the google maps link
  • Add aria-hidden to mandatory note at the bottom of the contact form

@virus-rpi virus-rpi requested a review from a team as a code owner January 9, 2024 16:50
Copy link

netlify bot commented Jan 9, 2024

Deploy Preview for satellytes ready!

Name Link
🔨 Latest commit d9f359d
🔍 Latest deploy log https://app.netlify.com/sites/satellytes/deploys/65c24dc30844840008fc3581
😎 Deploy Preview https://deploy-preview-792--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: 85 (🔴 down 1 from production)
Accessibility: 100 (no change from production)
Best Practices: 100 (no change from production)
SEO: 93 (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.

Copy link
Member

@georgiee georgiee 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 the good catches! Especially the honeypot thing is embarasing 😬 I've found some issues to fix, thanks!

src/assets/locales/en/translations.json Outdated Show resolved Hide resolved
src/components/layout/theme.tsx Show resolved Hide resolved
src/components/pages/contact/address.tsx Show resolved Hide resolved
src/components/pages/contact/honeypot.tsx Outdated Show resolved Hide resolved
src/components/pages/contact/honeypot.tsx Outdated Show resolved Hide resolved
src/templates/blog-post.tsx Outdated Show resolved Hide resolved
@maxmarkus
Copy link
Contributor

maxmarkus commented Jan 29, 2024

Idea: How about setting the whole map as aria-hidden? For visually impaired people it seems to bring no benefits, it has 5-6 links that can be tabbed and the readout is weird (screenshot below). Since we have the google maps link below, that might be enough.

screenshot of open streetmap zoom-in voice-over

Edit: After writing this, I realized that leafletjs is accessible. But the idea can still be valid.

@virus-rpi
Copy link
Collaborator Author

Idea: How about setting the whole map as aria-hidden? For visually impaired people it seems to bring no benefits, it has 5-6 links that can be tabbed and the readout is weird (screenshot below). Since we have the google maps link below, that might be enough.

screenshot of open streetmap zoom-in voice-over Edit: After writing this, I realized that [leafletjs is accessible](https://leafletjs.com/examples/accessibility/). But the idea can still be valid.

i tried aria hiding the map but VoiceOver still reads it despite the aria-hidden=true.

@virus-rpi
Copy link
Collaborator Author

I found a way to make the map aria-hidden but it is still tab focusable and i could not figure out how to remove it from the tab order (tabIndex = {-1} gets ignored)

Copy link
Contributor

@maxmarkus maxmarkus left a comment

Choose a reason for hiding this comment

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

Despite the fact that the general description "see no evil monkey emoji, because it's embarrassing" feels a bit weird I am approving it. We might think about rewording anyway.

@virus-rpi virus-rpi dismissed georgiee’s stale review February 6, 2024 15:01

Requested changes were done

@virus-rpi virus-rpi merged commit 5f49f4c into main Feb 6, 2024
6 checks passed
@virus-rpi virus-rpi deleted the chore/a11y/contact-fixes branch February 6, 2024 15:20
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.

3 participants