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

[BITV]: adapt checkboxes colors in normal state on dark theme #42121

Closed
JuliaKirschenheuter opened this issue Dec 8, 2023 · 10 comments
Closed
Assignees
Labels
1. to develop Accepted and waiting to be taken care of a11y28checked needed for a11y accessibility

Comments

@JuliaKirschenheuter
Copy link
Contributor

JuliaKirschenheuter commented Dec 8, 2023

Waiting for #42287 to be merged and backported

Screenshot from 2023-12-08 10-58-04

This have to be done for dark theme as well.

Problems:

  • Checkboxes
  • Radiobuttons
  • Checkbox radio switch

Screenshot from 2023-12-08 13-36-03
Screenshot from 2023-12-08 13-35-41
Screenshot from 2023-12-08 13-47-30
Screenshot from 2023-12-08 13-48-27

Suggestion:

Use --color-primary-element-light-text instead of --color-primary, like:

Screenshot from 2023-12-08 11-58-46

@JuliaKirschenheuter JuliaKirschenheuter added 1. to develop Accepted and waiting to be taken care of accessibility labels Dec 8, 2023
@JuliaKirschenheuter JuliaKirschenheuter self-assigned this Dec 8, 2023
@JuliaKirschenheuter JuliaKirschenheuter changed the title [BITV]: adapt colors of checkboxes in normal state on dark theme [BITV]: adapt checkboxes colors in normal state on dark theme Dec 8, 2023
@szaimen
Copy link
Contributor

szaimen commented Dec 8, 2023

I guess the calculation of the colors indeed needs to be different on dark mode compared to light mode in order to fullfill the contrast requirements. Calculation is done in

protected function generatePrimaryVariables(string $colorMainBackground, string $colorMainText): array {
$isBrightColor = $this->util->isBrightColor($colorMainBackground);
$colorPrimaryElement = $this->util->elementColor($this->primaryColor, $isBrightColor);
$colorPrimaryLight = $this->util->mix($colorPrimaryElement, $colorMainBackground, -80);
$colorPrimaryElementLight = $this->util->mix($colorPrimaryElement, $colorMainBackground, -80);
// primary related colours
return [
// invert filter if primary is too bright
// to be used for legacy reasons only. Use inline
// svg with proper css variable instead or material
// design icons.
// ⚠️ Using 'no' as a value to make sure we specify an
// invalid one with no fallback. 'unset' could here fallback to some
// other theme with media queries
'--primary-invert-if-bright' => $this->util->invertTextColor($this->primaryColor) ? 'invert(100%)' : 'no',
'--primary-invert-if-dark' => $this->util->invertTextColor($this->primaryColor) ? 'no' : 'invert(100%)',
'--color-primary' => $this->primaryColor,
'--color-primary-default' => $this->defaultPrimaryColor,
'--color-primary-text' => $this->util->invertTextColor($this->primaryColor) ? '#000000' : '#ffffff',
'--color-primary-hover' => $this->util->mix($this->primaryColor, $colorMainBackground, 60),
'--color-primary-light' => $colorPrimaryLight,
'--color-primary-light-text' => $this->util->mix($this->primaryColor, $this->util->invertTextColor($colorPrimaryLight) ? '#000000' : '#ffffff', -20),
'--color-primary-light-hover' => $this->util->mix($colorPrimaryLight, $colorMainText, 90),
// used for buttons, inputs...
'--color-primary-element' => $colorPrimaryElement,
'--color-primary-element-hover' => $this->util->mix($colorPrimaryElement, $colorMainBackground, 82),
'--color-primary-element-text' => $this->util->invertTextColor($colorPrimaryElement) ? '#000000' : '#ffffff',
// mostly used for disabled states
'--color-primary-element-text-dark' => $this->util->darken($this->util->invertTextColor($colorPrimaryElement) ? '#000000' : '#ffffff', 6),
// used for hover/focus states
'--color-primary-element-light' => $colorPrimaryElementLight,
'--color-primary-element-light-hover' => $this->util->mix($colorPrimaryElementLight, $colorMainText, 90),
'--color-primary-element-light-text' => $this->util->mix($colorPrimaryElement, $this->util->invertTextColor($colorPrimaryElementLight) ? '#000000' : '#ffffff', -20),
// to use like this: background-image: var(--gradient-primary-background);
'--gradient-primary-background' => 'linear-gradient(40deg, var(--color-primary) 0%, var(--color-primary-hover) 100%)',
];
}
IIRC...

@JuliaKirschenheuter
Copy link
Contributor Author

JuliaKirschenheuter commented Dec 8, 2023

Thank you @szaimen!

Do we really need to adjust this colors? It looks to me that i can just use an existing one --color-primary-element-light-text like if is used for text inside of the button:
Screenshot from 2023-12-08 15-52-16

and then use a dark mode mixin (but not sure if we have it). What do you think?

@szaimen
Copy link
Contributor

szaimen commented Dec 8, 2023

Do we really need to adjust this colors? It looks to me that i can just use an existing one --color-primary-element-light-text like if is used for text inside of the button: Screenshot from 2023-12-08 15-52-16

and then use a dark mode mixin. What do you think?

I wouldn't say we should do it like this. Better would be to adjust the calculation to work correctly in all, dark and bright mode.

@susnux
Copy link
Contributor

susnux commented Dec 8, 2023

I agree with @szaimen as that is the purpose of the --color-primary-element. To be used for graphical elements and thus should have a high enough contrast

@JuliaKirschenheuter
Copy link
Contributor Author

checked several components on a dark theme, sometimes there are 3.02:1 but still passing criteria. Seems not to be an issue anymore.

@JuliaKirschenheuter
Copy link
Contributor Author

JuliaKirschenheuter commented Dec 13, 2023

seems to be not resolved on activity settings on the table: 2.98:1

@JuliaKirschenheuter
Copy link
Contributor Author

Dear @nimishavijay

we have 2 problems in a dark theme in combination with --color-primary-element (#00679e):

--color-background-hover -> #212121

Screenshot from 2023-12-15 15-56-16

--color-main-background -> #171717

Screenshot from 2023-12-15 15-57-09

You can check it on hovered and normal state:

Screenshot from 2023-12-15 17-14-41
Screenshot from 2023-12-15 17-14-23

Could you please provide another colors for us?
Probably related:

https://play.phpsandbox.io/mexitek/phpcolors
#39317
#39391

@susnux
Copy link
Contributor

susnux commented Dec 15, 2023

Might be fixed with #42287

@JuliaKirschenheuter
Copy link
Contributor Author

fixed by #42287

@JuliaKirschenheuter
Copy link
Contributor Author

closed #42287

@szaimen szaimen added the a11y28checked needed for a11y label Jan 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1. to develop Accepted and waiting to be taken care of a11y28checked needed for a11y accessibility
Projects
None yet
Development

No branches or pull requests

3 participants