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 accessibility issues shown in chrome lighthouse audit #118

Merged
merged 1 commit into from
Oct 12, 2019

Conversation

lawrence-dass
Copy link

@lawrence-dass lawrence-dass commented Oct 12, 2019

Before:
before accessibility fix
After:
after accessibility fix

Wrapping div:
wrapping div
Note: We are using react below React v16.2.0, or else I would prefer to use React Fragment. Still div does not cause any side effect in the application layout.
Reference:
https://reactjs.org/docs/fragments.html
https://getstream.io/blog/react-fragments/
https://stackoverflow.com/questions/33766085/how-to-avoid-extra-wrapping-div-in-react

Button discernible text:
button discernible text
References:
https://dequeuniversity.com/rules/axe/3.1/button-name?application=lighthouse
https://www.w3schools.com/tags/att_global_title.asp

A minor change in the colors:
Before:
before minor change
After:
after minor change

Let me know if find you any concern, I'll update the PR accordingly.

Copy link
Owner

@nakulrathore nakulrathore left a comment

Choose a reason for hiding this comment

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

Perfect. and great explanation

@nakulrathore nakulrathore merged commit 34b428b into nakulrathore:master Oct 12, 2019
@nakulrathore
Copy link
Owner

@all-contributors please add @Lawrence4code for code

@allcontributors
Copy link
Contributor

@nakulrathore

I've put up a pull request to add @Lawrence4code! 🎉

@nakulrathore
Copy link
Owner

@Lawrence4code , let me know if you want to work on the performance of the app

@lawrence-dass
Copy link
Author

lawrence-dass commented Oct 12, 2019

@nakulrathore Thank you for the opportunity.

lighthouse screenshot

I dug into site perf, as of this moment there's not much scope for improvement as the site scores 100% on performance in chrome lighthouse. The site load on 1.213s (which is great, anything below 2s is awesome) as per https://www.webpagetest.org, https://www.webpagetest.org/result/191012_V9_909f40f66916dad95626bafa69eb0132/

At the moment, focus on code splitting and prefetch would be an overkill, please refer https://dassur.ma/things/less-snakeoil/

Just one recommendation would be to load the render-blocking CSS before the script file in the app js file. ( Refer: https://gtmetrix.com/reports/boxshadows.nakulrathore.com/6pkBfbeA )

By the way, I love the way you maintain your codebase.

Few recommendations:

  • Some kind of CSS naming conventions like BEM (my favorite)(if upgraded to sass)
  • Using rem instead of px unit to make the site more responsive and CSS variables.
  • Typescript with React.

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