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

Style/tweak stylesheet colours #47

Merged
merged 5 commits into from
Oct 30, 2020
Merged

Style/tweak stylesheet colours #47

merged 5 commits into from
Oct 30, 2020

Conversation

mst101
Copy link
Contributor

@mst101 mst101 commented Oct 24, 2020

Grey text on a coloured background does not look good, so I've chosen a dark blue instead (#104756).

In terms of accessibility, this is WCAG AAA compliant on the .highlighted background of #44bbdd and WCAG AA compliant on the .selected background of #cae5ed for 'normal text'. See contrast checker

N.B. As things stand, the .disabled styles completely override any .highlighted styles. Are you happy with this behaviour, or do you think that disabled cells that are highlighted should be distinguishable from disabled cells that are not highlighted?

Also, I noticed that the .disabled text colour #dddddd is not accessibility compliant on a white background. However, if you decrease the brightness enough to make it at least WCAG AA compliant for normal text i.e. #757575 - that just seems far too dark to my eyes for something that is supposed to be disabled. The muted text colour of #a3a3a3 is also not compliant. What are your thoughts on this?

@MrWook
Copy link
Collaborator

MrWook commented Oct 29, 2020

Looks good to me.
The colors of disabled and previous/next month are the same but this applies to the default html datepicker too. So we can stick with you changes 👍.

I never really worked with the highlight dates part before but i guess is would expect that a disabled date can not be highlighted. So we can keep it that way.
If someone needs it we can make a prop to toggle the behaviour.

Disabled states don't need to be accessible compliant, i guess https://www.w3.org/TR/WCAG21/#contrast-minimum

If you merge master into your branch i will merge this PR 👍

@mst101
Copy link
Contributor Author

mst101 commented Oct 29, 2020

Ok great, thanks.

Thanks for the W3 reference. Yes, I think it's fair to consider the disabled dates to be 'inactive'; they therefore can be considered 'incidental' and do not need to be accessible-compliant:

"Incidental: Text or images of text that are part of an inactive user interface component... have no contrast requirement."

The .muted (prev/next) dates, however, can't be considered inactive and are therefore not compliant with the current colour of #a3a3a3. Given that our text is the standard font size of 16pt, it is not considered to be 'large text' (which is either 14pt or above and bold, or 18pt or above).

I played around with the idea of perhaps using a light grey background for these 'edge dates', but it didn't look right. So, if you'd like us to be fully compliant, I think we should darken the text colour of the edge dates slightly to be the lightest acceptable grey for WCAG AA compliance i.e. #757575. I'll create a new commit with this value. To my eyes, it looks a little darker than I'd like, but perhaps that's just me?

@mst101
Copy link
Contributor Author

mst101 commented Oct 29, 2020

Just had another thought - we could perhaps make the edge dates a little smaller? What do you think?
We're 'sweating the small stuff', I know...

@MrWook
Copy link
Collaborator

MrWook commented Oct 30, 2020

I don't think playing with the font size is a good thing, this is oriented on the body font size.
Like i said earlier on the default html input the disabled days and previous/next month days have the same colour as well.

I think the way it is now with your changes it is perfectly sufficient.

@mst101
Copy link
Contributor Author

mst101 commented Oct 30, 2020

Ok, great. Let me know if there's anything else you need to me do in order for you to merge this in. Thanks.

@MrWook MrWook merged commit 78f331e into sumcumo:master Oct 30, 2020
@mst101 mst101 deleted the style/tweak-stylesheet-colours branch October 30, 2020 09:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants