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

feat: better screen reader text #790

Merged
merged 9 commits into from
Jan 29, 2024
Merged

feat: better screen reader text #790

merged 9 commits into from
Jan 29, 2024

Conversation

virus-rpi
Copy link
Collaborator

Changes:

Added a better way for screen readers to interpret the customer logos on the landing page

Screen reader text before:

image
image
image
image
image
image
image
image

Screen reader text after:

list Our Customers: 8 item
Element 1 of 8: Allianz
Element 2 of 8: Rosenbauer
Element 3 of 8: Media Markt
Element 4 of 8: Saturn
Element 5 of 8: Green City
Element 6 of 8: FC Bayern
Element 7 of 8: Zeiss
Element 8 of 8: ADAC

How to verify:

On Mac:

  • Activate VoiceOver with ⌘ Command + fn + F5
  • Navigate with ⌃ Control + ⌥ Option + Arrow Key

On Windows:

  • Activate Windows Narrator with Windows + Control + Enter
  • Navigate with the arrow keys

@virus-rpi virus-rpi requested a review from a team as a code owner January 2, 2024 15:30
Copy link

netlify bot commented Jan 2, 2024

Deploy Preview for satellytes ready!

Name Link
🔨 Latest commit a2fe8dc
🔍 Latest deploy log https://app.netlify.com/sites/satellytes/deploys/65b7cf263397f900083d912c
😎 Deploy Preview https://deploy-preview-790--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: 78 (🔴 down 1 from production)
Accessibility: 94 (no change from production)
Best Practices: 100 (no change from production)
SEO: 86 (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 self-assigned this Jan 2, 2024
@georgiee georgiee mentioned this pull request Jan 9, 2024
@virus-rpi
Copy link
Collaborator Author

❌ Deploy Preview for satellytes failed.

Name Link
🔨 Latest commit bf40197
🔍 Latest deploy log https://app.netlify.com/sites/satellytes/deploys/65942c04fbaab80008793c12

The error:

error Building static HTML failed for path "/blog/post/writing-and-enforcing-conventional-commit-messages-and-pull-request-titles/"
const shareImagePath = heroImage.shareImage.resize.src;
WebpackError: TypeError: Cannot read properties of null (reading "resize")

But because i didn't modify anything in the file that causes the error I don't think this was caused by my changes

@virus-rpi virus-rpi removed their assignment Jan 15, 2024
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.

An ideal solution would use the actual customer logos, have it in a list structure and then use css to spread them into the visual grid layout.

Your solution is much simpler but creates kind of a duplicated content: one part for the visuals and one part for the screen reads. I think that's good enough for now and better than the previous state anyway.

I approve this PR already but there are two comments you could deliver: a comment and something about the DOM

src/components/pages/landingpage/customers.tsx Outdated Show resolved Hide resolved
@virus-rpi
Copy link
Collaborator Author

the failing test is:

Run yarn test:format
yarn test:format
shell: /usr/bin/bash -e {0}
env:
CI: true
yarn run v1.22.21
$ prettier "**/*.{tsx,ts,md}" --check
Checking formatting...
[warn] src/components/layout/seo.tsx
[warn] Code style issues found in the above file. Run Prettier to fix.
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
Error: Process completed with exit code 1.

but I ran prettier before pushing

@maxmarkus
Copy link
Contributor

maxmarkus commented Jan 29, 2024

Please run yarn format once and check in the changed file. Not sure why it wasn't picked up by the pre-commit hook, but that fixes your issue.
Feel free to merge your PR once its green.

@virus-rpi virus-rpi merged commit d940b46 into main Jan 29, 2024
6 checks passed
@virus-rpi virus-rpi deleted the chore/a11y-custumer-logo branch January 29, 2024 16:17
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