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

Accessibility #7

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Accessibility #7

wants to merge 4 commits into from

Conversation

nkverse
Copy link
Owner

@nkverse nkverse commented Oct 4, 2022

Changes

A couple of changes were made to make this project more accessible.

Page titles

the page title was update with a more meaningful title, to let know the visitor what the page is about.

Image text alternatives

All img tags now have an alt attribute with a descriptive description that explains what is the image.

this helps with two things:

  • on image load failure, the alt will give a descriptive idea of what the image is
  • screen reader can read what the image is about

Text headings

no changes were made text headings are good.

Color contrast

no changes, the color design is very easy on the eye.

Resize

The design was tested with 300% upscale/zoom, nothing broke and everything looks solid.

Interaction

updated labels were added to each input inside the form, and they are hidden using CSS, but screen reader can read them and give a meaningful idea of what the form is about.

Moving content

no changes all interactions are in check. even the transition effects are set to a reasonable and comfortable speed.

Multimedia

no changes besides images the design doesn't contain any videos or audios ...

The basic structure of the page

no changes All CSS was disabled in order to check how the structure of the page will hold. the structure is meaningful even if CSS is absent

Copy link

@sumairq sumairq left a comment

Choose a reason for hiding this comment

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

Hi @nkverse ,

Your project is complete! There is nothing else to say other than... it's time to merge it :shipit:
Congratulations! 🎉

Highlights

✔️ Good commit history
✔️Linters are passing
✔️Very professional README
✔️Correct Github flow
✔️Descriptive PR

Optional suggestions

Every comment with the [OPTIONAL] prefix won't stop the approval of this PR. However, I strongly recommend you to take them into account as they can make your code better. Some of them were simply missed by the previous reviewer and addressing them will really improve your application.

Cheers and Happy coding!👏👏👏

Feel free to leave any questions or comments in the PR thread if something is not 100% clear.
Please, remember to tag me in your question so I can receive the notification.


As described in the Code reviews limits policy you have a limited number of reviews per project (check the exact number in your Dashboard). If you think that the code review was not fair, you can request a second opinion using this form.

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