Skip to content
This repository has been archived by the owner on Jan 1, 2024. It is now read-only.

Mailgo accessibility #51

Open
katekalcevich opened this issue Jul 24, 2020 · 28 comments
Open

Mailgo accessibility #51

katekalcevich opened this issue Jul 24, 2020 · 28 comments
Labels
enhancement New feature or request

Comments

@katekalcevich
Copy link
Contributor

Hi, would you be interested in making mailgo accessible to people with disabilities? This would involve code changes to support users who only use a keyboard (no mouse) or a screen reader.

The expected keyboard interaction for modals is:

  • when the modal opens, focus moves to an element inside the modal
  • you can tab through interactive elements inside the modal, but you can't tab to anything outside the modal
  • you can close the modal using the escape key
  • you can close the modal by focusing on a close button and pressing the space bar or enter key
  • when you close the modal, focus returns to the element that invoked the modal

The expected screen reader interaction for modals is:

  • the modal container has a role of dialog (which screen readers will announce)
  • the modal container has aria-modal set to true
  • the modal has either an aria-labelledby property or an aria-label
@manzinello
Copy link
Owner

Hi Kate,

Hi, would you be interested in making mailgo accessible to people with disabilities? This would involve code changes to support users who only use a keyboard (no mouse) or a screen reader.

Absolutely yes!

I've already (just a couple of hours ago!) merged a PR about accessibility (this one:
#49) that I think that solves a lot of the points you have explained.
You can check directly the code merged in this PR to check your points, I really want to include all these things in the next release!

you can close the modal by focusing on a close button and pressing the space bar or enter key

I am not an expert of accessibility: I have a question about spacebar/enter key, because is something I have implemented and was not in the PR. Using spacebar/enter key the behaviour I have implemented is the "default action", so in this case open the mailto following the classic behaviour of the browser. Is it wrong? Have I to close the modal on this action? I really don't know accessibility rules and I think that it is a very interesting/important point, I really want to follow the correct rules in mailgo but I am not an expert. Can you help me?

Oh, obviously you can submit directly a PR with changes about accessibility!

Thank you so much for your contribution!

@manzinello manzinello added the enhancement New feature or request label Jul 24, 2020
@katekalcevich
Copy link
Contributor Author

katekalcevich commented Jul 24, 2020 via email

@manzinello
Copy link
Owner

The modal should close if a user presses the escape key. The spacebar/enter trigger is for the close button. If it's a real button (e.g. or that is the default behaviour. You can click a button or focus on it and press enter/space to click it without a mouse). So it's not the modal that needs to close when you press enter or space - that's for when the focus is on the close button and only if it's not a true button. Lots of people make buttons with

or tags.

Very clear, thank you for the explanation!

Hi Matteo, I will have a look at PR #49 this weekend.

I think that accessibility is very important for mailgo, I have released the new mailgo version 0.9.10 yesterday with the changes submitted in that PR. If you want to check the behaviour of mailgo modal from an accessibility point of view you can directly see this and this example pages, CodePen or also this folder in the repo, all of them run the latest version of mailgo. Feel free to give feedback about them or maybe submit directly a PR with your changes!

Thank you again!

@katekalcevich
Copy link
Contributor Author

Hi Matteo, I had a look and it's pretty good now! I have a few suggestions. I looked at the code, but when it comes to making any changes myself, well I just am not that advanced of a developer! I'll leave it to you to implement.

  1. Modal bug? On the Tel links example page, the modal is mostly working fine, but I can slip out of it if I tab backwards. It's not a huge deal, but you don't have the same problem on the mailto modals example page. However, the same problem does not exist at all in the CodePen examples, so maybe it is just a small bug on the example pages implementation.

  2. Contrast / font size. This is the one big barrier I found. Someone with low vision would have trouble reading the text. Perhaps in addition to dark mode you could make an accessible light mode?
    To get enough contrast for easy reading, the Gmail links should be #C0372A, Outlook links #0967AA, Telegram links #086DA0, WhatsApp links #067466, and Skype links #076D92.
    I'd change the mailgo.dev link to the colour you use on hover (in both regular and dark mode).
    The dark mode links are fine as they are except the mailgo.dev link. They contrast well.
    I'd increase the modal font size to 110% (about 16.5px) and the small mailgo.dev link from 9px to 12px.

  3. Focus support. On hover, you change the background colour of the links - a very nice effect. If you added a :focus style then the background change would work on keyboard too. It's not critical - doesn't create a barrier for anyone, but changing it makes the experience more equivalent. It's up to you to prioritize this or not.

I didn't find anything else that's an obvious issue. I checked it with Axe, the WCAG Color contrast checker, keyboard only and VoiceOver screen reader. Not a full accessibility audit, but a pretty thorough check. Great work!

@manzinello
Copy link
Owner

Thank you so much Kate!

Modal bug? On the Tel links example page, the modal is mostly working fine, but I can slip out of it if I tab backwards. It's not a huge deal, but you don't have the same problem on the mailto modals example page. However, the same problem does not exist at all in the CodePen examples, so maybe it is just a small bug on the example pages implementation.

I will check this, I think that could be a bug in example pages implementation.

Contrast / font size. This is the one big barrier I found. Someone with low vision would have trouble reading the text. Perhaps in addition to dark mode you could make an accessible light mode?
To get enough contrast for easy reading, the Gmail links should be #C0372A, Outlook links #0967AA, Telegram links #086DA0, WhatsApp links #067466, and Skype links #076D92.
I'd change the mailgo.dev link to the colour you use on hover (in both regular and dark mode).
The dark mode links are fine as they are except the mailgo.dev link. They contrast well.
I'd increase the modal font size to 110% (about 16.5px) and the small mailgo.dev link from 9px to 12px.

Awesome suggestion, thank you so much. I will work on this, the colors I have used are the original of the brands, but your suggestions about these colors are very precious.

Focus support. On hover, you change the background colour of the links - a very nice effect. If you added a :focus style then the background change would work on keyboard too. It's not critical - doesn't create a barrier for anyone, but changing it makes the experience more equivalent. It's up to you to prioritize this or not.

I will add the focus too!

Thank you again!

manzinello added a commit that referenced this issue Jul 29, 2020
@manzinello
starting with changes for accessibility :active (#51)
0905ff7
manzinello added a commit that referenced this issue Jul 29, 2020
@manzinello
changing font-size for accessibility (#51)
5269c76
manzinello added a commit that referenced this issue Jul 29, 2020
@manzinello
working on colors for accessibility (#51)
68e8b61
@geoffreycrofte
Copy link

Thanks @katekalcevich for the feedback on Accessibility :) (I've worked on it ha ha :D)

For the contrast / button style / size / focus I was about to suggestion modifications too. For small devices for instance the tappable area is not that big, but it's also a good usability practice.
I saw @manzinello was working on it. I'll wait a little bit then :)

@manzinello
Copy link
Owner

Hi @geoffreycrofte! Yes, I've just started working on the suggestions of @katekalcevich, in particular about colors, font-size, focus.

For small devices for instance the tappable area is not that big, but it's also a good usability practice.

Well, if for a better usability is required a different behaviour depending on screen size we can obviously use media queries for example!

If you want to explain your considerations and suggestions: feel free to speak, I can directly work also on them (or maybe you can submit also a PR).

@katekalcevich, sorry: I've directly started to work on your suggestions because this morning I had some time to work on mailgo, maybe do you want to contribute directly with a PR?

@katekalcevich
Copy link
Contributor Author

@manzinello I could find which file has the CSS styles, but I'm happy to edit if you can point me in the right direction!

@manzinello
Copy link
Owner

Sure! Mailgo uses Sass/SCSS, the file that manages all the styles of the library is this one: https://github.com/manzinello/mailgo/blob/master/src/mailgo.scss, that is built with webpack in CSS.

If you don't know Sass or you don't have time to work on it, please let me know so I can continue with changes. Otherwise submit directly you PR!

@katekalcevich
Copy link
Contributor Author

Well, I'll probably have time on the weekend, so it depends on how fast you want it. I do know Sass.

@manzinello
Copy link
Owner

Perfect for the weekend, thank you!

@geoffreycrofte if you want to work and submit a PR with your accessibility changes, you can work on it! Just, please, if you can: explain them here before!

Thank you again!

@geoffreycrofte
Copy link

I'll wait a little bit as you might already worked on it. I'll pull your work, work on it to test it and my PR will explain all of that and why :)

@manzinello
Copy link
Owner

Perfect! Thank you!

@manzinello
Copy link
Owner

Today I will release mailgo 0.9.14 because of some bugs with new css-loader (and with a new language and improvements in i18n).

Then I will not work on mailgo until next week so you are completely free to clone/pull the updated repo and do your PR! Thank you again!

@manzinello
Copy link
Owner

Hi! I've added Yahoo Mail as a new possibility for mailto links. @katekalcevich or @geoffreycrofte can you make a check of the colour I have used? I have used the original Yahoo purple color but maybe there is a more correct purple color near the one I have added. You can directly submit a PR with the change, I'd like to use this issue for a11y questions, is it ok for you?

@katekalcevich
Copy link
Contributor Author

Hi Matteo, I'll do a PR and brighten the Yahoo colour in dark mode. It's fine in the regular mode.

P.S. I used your code on a website I just launched: http://teakboutique.ca/contact-us.html

@manzinello
Copy link
Owner

manzinello commented Aug 27, 2020 •
edited
Loading

Hi Matteo, I'll do a PR and brighten the Yahoo colour in dark mode. It's fine in the regular mode.

Perfect, thank you so much!

P.S. I used your code on a website I just launched: http://teakboutique.ca/contact-us.html

Thank you! For any type of problem or suggestion using it open an ISSUE or write me to matteo@manzinello.dev (well, I think that my email address is like everywhere in mailgo demos 😂)

@manzinello
Copy link
Owner

@katekalcevich I am waiting you PR for the Yahoo color! Just to know: when you can submit it? Because in the meantime I am releasing a new version (with a lot of changes), I would like to have the new color in it! Thank you so much! 👍

@katekalcevich
Copy link
Contributor Author

Sorry @manzinello I thought I already submitted it! I'll try again right now.

@manzinello
Copy link
Owner

@katekalcevich no problem!

@manzinello
Copy link
Owner

Hi again @katekalcevich, sorry: is there maybe some problem in submitting PR? If so, tell me I will investigate the matter. Thank you!

@katekalcevich
Copy link
Contributor Author

Only problem @manzinello is I'm not very good with Git! I lost the updates I had done before so I had to update my fork to match your master and recheck all the colours. You should have the PR now.

@manzinello
Copy link
Owner

@katekalcevich PR received! Thank you so much!

@RaymondFallon
Copy link

This thread is a really awesome example of collaboration on the accessibility front. Kudos to all of y'all!

@manzinello
Copy link
Owner

Hi everyone, I've just pushed this commit (1033b95) with some changes in the structure of colors in SCSS files. The problem is that every background-color repeats the color so I have changed it to use the variables defined on top of the file.

For example this

background-color: rgba(0, 175, 240, 0.08);

has been changed in this

background-color: rgba($skype-dark-color, 0.08);

Well, I've noticed that not every color of the background was respecting the active color. For example the background of the dark mode was the color of the non-darkmode color with opacity 0.08. Now I have uniformed this with

color and as background the same color with opacity 0.08

Is it correct for you? Is it works from the accessibility point of view?

@katekalcevich
Copy link
Contributor Author

Only the Yahoo dark color needed to change to have enough contrast. I've submitted a pull request. I also added a separate commit with a static file for color contrast testing (up to you if you want to pull that in or just take the first commit). It makes it easier to run a browser extension to test all the colours at once, but would need to be manually updated every time colours change.

@manzinello
Copy link
Owner

As always, thanks @katekalcevich! Good for html file, I will just move it in another folder (maybe a test folder or in examples)! Maybe we will think about an html with just <div>s and the import of mailgo.css from dist folder, I will think about it, at the moment it's ok to change the colour manually in html file.

manzinello added a commit that referenced this issue Sep 13, 2020
@manzinello
moved color test in test folder #51
9977866
@manzinello
Copy link
Owner

Accepted PR, I've created a test folder for these types of test! Thank you again.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@geoffreycrofte @katekalcevich @manzinello @RaymondFallon