search

buildo / react-cookie-banner

React Cookie banner which can be automatically dismissed with a scroll. Because fuck The Cookie Law, that's why.
http://react-components.buildo.io/#cookiebanner
MIT License
182 stars 19 forks source link

Accessibility improvements: Button elements & a rel attributes #43

Closed richardwestenra closed 6 years ago

richardwestenra commented 6 years ago

Closes #45

Description

  1. Change the default close button & icon elements to use <button> elements, either instead of a div, or as an added wrapper around the <i> icon element. This is mainly to make these elements focusable, tabbable and clickable with a keyboard. Among other things, this change will improve accessibility for visually-impaired screenreader users, as well as users with reduced motor skills who might prefer to use a keyboard instead of a mouse.
  2. Add 'rel' attribute param to the link option. Allowing developers to add the rel attribute will enable them to set rel="noopener noreferrer" on external links, especially those which open in a new tab. This helps mitigate a security vulnerability and potentially improves performance too.
  3. Improve contrast of button foreground/background colours, to help meet WCAG text contrast guidelines.

Notes

  1. I've tried to keep the styling as close as possible to the existing styling. I've retained the default focus outline (which is very important for keyboard a11y). I would've liked to add extra hover/focus/active styles for the buttons but I'm not sure how to do that with the existing inline style setup, plus that's probably best kept for another PR.
  2. I've made the rel attribute a string prop so developers can set whatever rel content they want, in case they want to set something else like rel="external" or rel="home" or whatever.
FrancescoCioria commented 6 years ago

Hi @richardwestenra and thanks for this great PR!

The code looks 👍 I'll try it locally asap

richardwestenra commented 6 years ago

Thanks @FrancescoCioria! Note that I just noticed and fixed a missing comma. Hopefully that'll fix the broken build?

FrancescoCioria commented 6 years ago

Hopefully that'll fix the broken build?

that was definitely a mistake, but the CI is failing for other reasons and I don't think it's because of your PR... I'll look into it and see if I can fix it

FrancescoCioria commented 6 years ago

I tested and it looks fine 🎉 💅

nemobot commented 6 years ago