fix(theming): Ensure all text colors have enough contrast for accessibility by susnux · Pull Request #40773 · nextcloud/server

susnux · 2023-10-04T21:41:02Z

Summary

Part of multiple a11y issues.

Text that has assigned the color --color-text-maxcontrast should always have a contrast of 4.5:1, even when the element is hovered where the background will be --color-background-hover.
So the maxcontrast color is slightly adjusted (I honestly do not see a difference 🙈 ) but it ensures a contrast of 4.5:1

Screenshots

before	after

Checklist

Code is properly formatted
Sign-off message is added to all commits
Tests (unit, integration, api and/or acceptance) are included
Screenshots before/after for front-end changes
Documentation (manuals or wiki) has been updated or is not required
Backports requested where applicable (ex: critical bugfixes)

skjnldsv · 2023-10-05T05:55:49Z

Could we have testing with various combinations of foreground/background colors so we're sure all of them pass WCAG AA ? :)

susnux · 2023-10-05T12:56:58Z

Could we have testing with various combinations of foreground/background colors so we're sure all of them pass WCAG AA ? :)

@skjnldsv Done but some combination are failing, how to handle those, ignore? Fix? 😅

susnux · 2023-10-05T13:01:47Z

cc @jancborchardt are those failing combinations expected to be used together or do we never want them to be used together? (those without a check mark but a number)

Accessibility of CSS color variables for Generic combinations
    ✓ color contrast of color-main-text on color-background-main (321ms)
    ✓ color contrast of color-main-text on color-background-hover (51ms)
    ✓ color contrast of color-main-text on color-background-dark (54ms)
    ✓ color contrast of color-main-text on color-background-darker (49ms)
    ✓ color contrast of color-text-maxcontrast on color-background-main (49ms)
    ✓ color contrast of color-text-maxcontrast on color-background-hover (58ms)
    1) color contrast of color-text-maxcontrast on color-background-dark
    2) color contrast of color-text-maxcontrast on color-background-darker
    ✓ color contrast of color-text-maxcontrast-default on color-background-main (65ms)
    ✓ color contrast of color-text-maxcontrast-default on color-background-hover (45ms)
    3) color contrast of color-text-maxcontrast-default on color-background-dark
    4) color contrast of color-text-maxcontrast-default on color-background-darker
    ✓ color contrast of color-error-text on color-background-main (83ms)
    ✓ color contrast of color-error-text on color-background-hover (59ms)
    ✓ color contrast of color-error-text on color-background-dark (56ms)
    5) color contrast of color-error-text on color-background-darker
    ✓ color contrast of color-warning-text on color-background-main (67ms)
    6) color contrast of color-warning-text on color-background-hover
    7) color contrast of color-warning-text on color-background-dark
    8) color contrast of color-warning-text on color-background-darker
    ✓ color contrast of color-success-text on color-background-main (169ms)
    ✓ color contrast of color-success-text on color-background-hover (65ms)
    ✓ color contrast of color-success-text on color-background-dark (69ms)
    ✓ color contrast of color-success-text on color-background-darker (62ms)
    ✓ color contrast of color-info-text on color-background-main (87ms)
    ✓ color contrast of color-info-text on color-background-hover (68ms)
    ✓ color contrast of color-info-text on color-background-dark (71ms)
    ✓ color contrast of color-info-text on color-background-darker (82ms)

  Accessibility of CSS color variables for Primary
    9) color contrast of color-primary-text on color-primary-default
    ✓ color contrast of color-primary-text on color-primary (177ms)
    10) color contrast of color-primary-text on color-primary-hover

  Accessibility of CSS color variables for Primary light
    ✓ color contrast of color-primary-light-text on color-primary-light (187ms)
    ✓ color contrast of color-primary-light-text on color-primary-light-hover (60ms)

  Accessibility of CSS color variables for Primary element
    ✓ color contrast of color-primary-element-text on color-primary-element (49ms)
    ✓ color contrast of color-primary-element-text on color-primary-element-hover (52ms)
    ✓ color contrast of color-primary-element-text-dark on color-primary-element (56ms)
    11) color contrast of color-primary-element-text-dark on color-primary-element-hover

  Accessibility of CSS color variables for Primary element light
    ✓ color contrast of color-primary-element-light-text on color-primary-element-light (82ms)
    ✓ color contrast of color-primary-element-light-text on color-primary-element-light-hover (105ms)

Previous with -light and -lighter:

Details

 Accessibility of CSS color variables for Generic combinations
    ✓ color contrast of color-main-text on color-background-main (357ms)
    ✓ color contrast of color-main-text on color-background-hover (59ms)
    ✓ color contrast of color-main-text on color-background-dark (46ms)
    ✓ color contrast of color-main-text on color-background-darker (48ms)
    ✓ color contrast of color-text-light on color-background-main (38ms)
    ✓ color contrast of color-text-light on color-background-hover (50ms)
    ✓ color contrast of color-text-light on color-background-dark (39ms)
    ✓ color contrast of color-text-light on color-background-darker (51ms)
    ✓ color contrast of color-text-lighter on color-background-main (40ms)
    ✓ color contrast of color-text-lighter on color-background-hover (53ms)
    1) color contrast of color-text-lighter on color-background-dark
    2) color contrast of color-text-lighter on color-background-darker
    ✓ color contrast of color-text-maxcontrast on color-background-main (46ms)
    ✓ color contrast of color-text-maxcontrast on color-background-hover (43ms)
    3) color contrast of color-text-maxcontrast on color-background-dark
    4) color contrast of color-text-maxcontrast on color-background-darker
    ✓ color contrast of color-text-maxcontrast-default on color-background-main (74ms)
    ✓ color contrast of color-text-maxcontrast-default on color-background-hover (44ms)
    5) color contrast of color-text-maxcontrast-default on color-background-dark
    6) color contrast of color-text-maxcontrast-default on color-background-darker
    ✓ color contrast of color-error-text on color-background-main (72ms)
    ✓ color contrast of color-error-text on color-background-hover (39ms)
    ✓ color contrast of color-error-text on color-background-dark (43ms)
    7) color contrast of color-error-text on color-background-darker
    ✓ color contrast of color-warning-text on color-background-main (83ms)
    8) color contrast of color-warning-text on color-background-hover
    9) color contrast of color-warning-text on color-background-dark
    10) color contrast of color-warning-text on color-background-darker
    ✓ color contrast of color-success-text on color-background-main (110ms)
    ✓ color contrast of color-success-text on color-background-hover (49ms)
    ✓ color contrast of color-success-text on color-background-dark (55ms)
    ✓ color contrast of color-success-text on color-background-darker (59ms)
    ✓ color contrast of color-info-text on color-background-main (58ms)
    ✓ color contrast of color-info-text on color-background-hover (68ms)
    ✓ color contrast of color-info-text on color-background-dark (53ms)
    ✓ color contrast of color-info-text on color-background-darker (55ms)

  Accessibility of CSS color variables for Primary
    11) color contrast of color-primary-text on color-primary-default
    ✓ color contrast of color-primary-text on color-primary (106ms)
    12) color contrast of color-primary-text on color-primary-hover

  Accessibility of CSS color variables for Primary light
    ✓ color contrast of color-primary-light-text on color-primary-light (89ms)
    ✓ color contrast of color-primary-light-text on color-primary-light-hover (52ms)

  Accessibility of CSS color variables for Primary element
    ✓ color contrast of color-primary-element-text on color-primary-element (52ms)
    ✓ color contrast of color-primary-element-text on color-primary-element-hover (51ms)
    ✓ color contrast of color-primary-element-text-dark on color-primary-element (51ms)
    13) color contrast of color-primary-element-text-dark on color-primary-element-hover

  Accessibility of CSS color variables for Primary element light
    ✓ color contrast of color-primary-element-light-text on color-primary-element-light (95ms)
    ✓ color contrast of color-primary-element-light-text on color-primary-element-light-hover (54ms)

jancborchardt · 2023-10-05T14:23:05Z

Just to clarify, since most of the issues appear with these values:

Both color-text-light and color-text-lighter are deprecated in favor of color-text-maxcontrast
Where is color-background-dark and -darker used? Cause dark theme is just color-main-background as well, or do you mean dark theme by this?

susnux · 2023-10-05T14:31:42Z

Just to clarify, since most of the issues appear with these values:
* Both color-text-light and color-text-lighter are deprecated in favor of color-text-maxcontrast

Ok then it does not make sense to test those ✅

* Where is color-background-dark and -darker used? Cause dark theme is just color-main-background as well, or do you mean dark theme by this?

They are unrelated to the dark color theme, but used for element backgrounds to stand out of the main background.

Those background colors are used e.g. for selected items, like if you select an app in the app settings the background color of that row it changed to -dark:

It (-dark) is also used as the background color for code blocks in markdown (pre) .
-darker is used on some backgrounds, like two-factor-provider

susnux · 2023-10-06T14:32:07Z

@jancborchardt Basically I see 3 problems:

The dark(er) background is not accessible with color maxcontrast. (For dark this might be a problem as we use it for normal element backgrounds, for darker we can migrate our code to only use it for presentation purpose without text on it)
Primary: We already added the element version for this problem?
The warning text color. Same problem as with that yellow / golden stars for favorites: Yellow / Orange simply has no contrast on bright background.

jancborchardt · 2023-10-24T09:52:01Z

The dark(er) background is not accessible with color maxcontrast. (For dark this might be a problem as we use it for normal element backgrounds, for darker we can migrate our code to only use it for presentation purpose without text on it)

Yeah sounds good. Maybe also good to look at what other dark modes like e.g. GitHub do?

Primary: We already added the element version for this problem?

Yep, exactly.

The warning text color. Same problem as with that yellow / golden stars for favorites: Yellow / Orange simply has no contrast on bright background.

Yes – we should probably just not use text in those colors, and if we have warning text always use NcNoteCard. Or if it needs to be smaller use the colored icon but normal colored text.

susnux · 2023-10-24T09:54:12Z

Yeah sounds good. Maybe also good to look at what other dark modes like e.g. GitHub do?

To not confuse @jancborchardt it is not about dark mode, it is about that slightly darker background for e.g. every second table row or something like that (similar to hover).

susnux · 2023-10-25T11:59:25Z

@nextcloud/designers Please review that you are ok with this changes.

I tried to stick as close as possible to the previous values, but for text accessibility at least some values had to be changed.
(See added accessibility test for combinations that shall pass).

jancborchardt · 2023-10-25T13:10:34Z

@susnux could you post before/after screenshots of the specific cases we should check for easier review? :)

susnux · 2023-10-25T14:28:01Z

And I pushed another commit which also fixes the accessibility issues, but might look better. What do you think?
With the last commit:
This is now the way to go:

https://jsfiddle.net/r5d8nwz4/

nimishavijay · 2023-10-26T10:24:49Z

@susnux The latest screenshots look great to me :) much closer color to what is existing compared to the previous screenshot 👍 if the accessibility issues are also solved then we can go ahead :)

skjnldsv

Amazing work as always!! Thank you so much for doing this!

skjnldsv · 2023-10-26T10:28:13Z

+  --color-text-light: var(--color-main-text);
+  /** @deprecated use `--color-text-maxcontrast` instead */
+  --color-text-lighter: var(--color-text-maxcontrast);


This needs to be in the documentation 👍

Learned this here:
#40773 (comment)

So I added that comments, is there a place to document that?

@jancborchardt since when does color-text-lighter, color-text-light, been removed/deprecated?
Was there an official statement somewhere?

@susnux you can document it in the migration guide https://github.com/nextcloud/documentation/blob/master/developer_manual/app_publishing_maintenance/app_upgrade_guide/upgrade_to_28.rst

Was there an official statement somewhere?

If yes, does the 3 years rules also apply to CSS variables? (Just because I am curious, like how long do we keep providing the deprecated SCSS variables).

No clue really, For frontend we followed more the rule of 3 versions 🙈

@skjnldsv I think I found it [1] so seems to be deprecated since Nextcloud 20
[1] 9aff535

nextcloud/documentation#11246

I'm ok removing them then 🤷

JuliaKirschenheuter

cool! Thank you

….5:1 even on hover Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>

…deprecated Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>

…bility Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>

susnux · 2023-10-27T11:33:27Z

Github 🤦

susnux added bug design Design, UI, UX, etc. 3. to review Waiting for reviews accessibility labels Oct 4, 2023

susnux requested review from JuliaKirschenheuter, Pytal, jancborchardt and skjnldsv October 4, 2023 21:41

susnux force-pushed the fix/contrast-maxcontrast-vs-hover branch from 17a051a to 4349b2b Compare October 4, 2023 21:42

susnux force-pushed the fix/contrast-maxcontrast-vs-hover branch from 4349b2b to 2b820eb Compare October 5, 2023 10:33

susnux force-pushed the fix/contrast-maxcontrast-vs-hover branch from 6b469bd to 499208e Compare October 5, 2023 12:57

susnux mentioned this pull request Oct 5, 2023

[BITV]: NcSelect improvements #40697

Closed

susnux force-pushed the fix/contrast-maxcontrast-vs-hover branch 2 times, most recently from 12d998a to 1641e4e Compare October 5, 2023 14:51

skjnldsv added this to the Nextcloud 28 milestone Oct 24, 2023

susnux force-pushed the fix/contrast-maxcontrast-vs-hover branch from 1641e4e to c008f3e Compare October 25, 2023 11:58

susnux changed the title ~~fix(theming): Ensure that maxcontrast text has always a contrast of 4.5:1 even on hover~~ fix(theming): Ensure all text colors have enough contrast for accessibility Oct 25, 2023

susnux mentioned this pull request Oct 25, 2023

[BITV]: Make sure that contrasts on interactive elements between normal state and focused state have to be 3:1 #40711

Closed

susnux force-pushed the fix/contrast-maxcontrast-vs-hover branch from c008f3e to 16bf124 Compare October 25, 2023 13:51

This comment was marked as outdated.

Sign in to view

skjnldsv approved these changes Oct 26, 2023

View reviewed changes

skjnldsv reviewed Oct 26, 2023

View reviewed changes

susnux requested a review from nimishavijay October 26, 2023 15:41

susnux mentioned this pull request Oct 26, 2023

chore(devmanual): Deprecate --color-text-light and --color-text-lighter CSS variables nextcloud/documentation#11246

Merged

nimishavijay approved these changes Oct 26, 2023

View reviewed changes

Pytal approved these changes Oct 26, 2023

View reviewed changes

JuliaKirschenheuter approved these changes Oct 27, 2023

View reviewed changes

susnux added 5 commits October 27, 2023 12:54

fix(theming): Ensure that maxcontrast text has always a contrast of 4…

1c4787a

….5:1 even on hover Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>

tests: Add accessibility tests for CSS color variables

ec1ae45

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>

fix(theming): Make it clearer that color-text-light and -lighter are …

be129a7

…deprecated Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>

fix(theming): Ensure all text colors have enough contrast for accessi…

3676295

…bility Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>

fix(theming): Make changes on primary color less invasiv

3378a73

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>

susnux force-pushed the fix/contrast-maxcontrast-vs-hover branch from 3528ee4 to 3378a73 Compare October 27, 2023 10:56

ChristophWurst closed this in nextcloud/documentation#11246 Oct 27, 2023

susnux reopened this Oct 27, 2023

susnux merged commit 32eaf57 into master Oct 27, 2023

susnux deleted the fix/contrast-maxcontrast-vs-hover branch October 27, 2023 14:12

Uh oh!

Conversation

susnux commented Oct 4, 2023

Summary

Screenshots

Checklist

Uh oh!

skjnldsv commented Oct 5, 2023

Uh oh!

susnux commented Oct 5, 2023

Uh oh!

susnux commented Oct 5, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jancborchardt commented Oct 5, 2023

Uh oh!

susnux commented Oct 5, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

susnux commented Oct 6, 2023

Uh oh!

jancborchardt commented Oct 24, 2023

Uh oh!

susnux commented Oct 24, 2023

Uh oh!

susnux commented Oct 25, 2023

Uh oh!

jancborchardt commented Oct 25, 2023

Uh oh!

This comment was marked as outdated.

susnux commented Oct 25, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nimishavijay commented Oct 26, 2023

Uh oh!

skjnldsv left a comment

Choose a reason for hiding this comment

Uh oh!

skjnldsv Oct 26, 2023

Choose a reason for hiding this comment

Uh oh!

susnux Oct 26, 2023

Choose a reason for hiding this comment

Uh oh!

skjnldsv Oct 26, 2023

Choose a reason for hiding this comment

Uh oh!

susnux Oct 26, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

skjnldsv Oct 26, 2023

Choose a reason for hiding this comment

Uh oh!

susnux Oct 26, 2023

Choose a reason for hiding this comment

Uh oh!

susnux Oct 26, 2023

Choose a reason for hiding this comment

Uh oh!

skjnldsv Oct 26, 2023

Choose a reason for hiding this comment

Uh oh!

JuliaKirschenheuter left a comment

Choose a reason for hiding this comment

Uh oh!

susnux commented Oct 27, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

susnux commented Oct 5, 2023 •

edited

Loading

susnux commented Oct 5, 2023 •

edited

Loading

susnux commented Oct 25, 2023 •

edited

Loading

susnux Oct 26, 2023 •

edited

Loading