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

Project 8 accessibility #387

Open
wants to merge 18 commits into
base: master
Choose a base branch
from

Conversation

erikamolsson
Copy link

Updated business site with accessibility.

@zoe-lindqvist
Copy link

Hi Erika, great job with the semantic HTML elements, like header, main, article and section. You also used clear labels and descriptive alt text for images. Your website is also responsive and is accessible by keyboard. You've used ARIA roles thoughtfully, like role="menu" and role="presentation". Well done again 👍

Copy link
Contributor

@HIPPIEKICK HIPPIEKICK left a comment

Choose a reason for hiding this comment

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

Well defined form, descriptive alt tags as well as sufficient contrasts 🥳

Nice that you’ve used ARIA labels and roles, but some elements have incorrect roles.img and a tags respectively since the img and a tags handle these roles inherently. However, adding ARIA labels directly to links and buttons could improve support for screen readers.

@HIPPIEKICK
Copy link
Contributor

PS. Noticed your hamburger is a bit buggy on small phones

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