Skip to content

Commit

Permalink
fix: WDC color APCA checks (appsmithorg#23660)
Browse files Browse the repository at this point in the history
## Description

tl;dr Fixed APCA contrast checks for both light and dark modes (detailed
info in the ticket). Refined how we pick shade or tint for `fgOnAccent`.

Fixes appsmithorg#23218    

## Type of change

- Bug fix (non-breaking change which fixes an issue)
- Chore (housekeeping or task changes that don't impact user perception)


## How Has This Been Tested?

- Manual

### Test Plan
Initial POC refinement, no testing necessary

## Checklist:
### Dev activity
- [x] My code follows the style guidelines of this project
- [x] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation
- [x] My changes generate no new warnings
- [ ] I have added tests that prove my fix is effective or that my
feature works
- [ ] New and existing unit tests pass locally with my changes
- [ ] PR is being merged under a feature flag


### QA activity:
- [ ] Test plan has been approved by relevant developers
- [ ] Test plan has been peer reviewed by QA
- [ ] Cypress test cases have been added and approved by either SDET or
manual QA
- [ ] Organized project review call with relevant stakeholders after
Round 1/2 of QA
- [ ] Added Test Plan Approved label after reveiwing all Cypress test
  • Loading branch information
ichik authored May 24, 2023
1 parent 03c42b8 commit 2ce4e23
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 41 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ export class DarkModeTheme implements ColorModeTheme {
private get fgAccent() {
const color = this.seedColor.clone();

if (this.seedColor.contrastAPCA(this.bg) <= 60) {
if (this.bg.contrastAPCA(this.seedColor) >= -60) {
if (this.seedIsAchromatic) {
color.oklch.l = 0.79;
color.oklch.c = 0;
Expand All @@ -151,34 +151,26 @@ export class DarkModeTheme implements ColorModeTheme {
color.oklch.c = 0.136;
return color;
}

return color;
}

private get fgOnAccent() {
const color = this.seedColor.clone();

if (this.seedColor.contrastAPCA(this.bg) <= 40) {
if (this.seedIsAchromatic) {
color.oklch.l = 0.985;
color.oklch.c = 0;
return color;
}
const tint = this.seedColor.clone();
const shade = this.seedColor.clone();

color.oklch.l = 0.985;
color.oklch.c = 0.016;
return color;
if (this.seedIsAchromatic) {
tint.oklch.c = 0;
shade.oklch.c = 0;
}

if (this.seedIsAchromatic) {
color.oklch.l = 0.15;
color.oklch.c = 0;
return color;
tint.oklch.l = 0.94;
shade.oklch.l = 0.27;

if (-this.bgAccent.contrastAPCA(tint) < this.bgAccent.contrastAPCA(shade)) {
return shade;
}

color.oklch.l = 0.15;
color.oklch.c = 0.064;
return color;
return tint;
}

private get fgNegative() {
Expand All @@ -192,7 +184,7 @@ export class DarkModeTheme implements ColorModeTheme {
private get bdAccent() {
const color = this.seedColor.clone();

if (this.bg.contrastAPCA(this.seedColor) > -15) {
if (this.bg.contrastAPCA(this.seedColor) >= -25) {
if (this.seedIsAchromatic) {
color.oklch.l = 0.985;
color.oklch.c = 0;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ export class LightModeTheme implements ColorModeTheme {
private get fgAccent() {
const color = this.seedColor.clone();

if (this.seedColor.contrastAPCA(this.bg) >= -60) {
if (this.bg.contrastAPCA(this.seedColor) <= 60) {
if (this.seedIsAchromatic) {
color.oklch.l = 0.25;
color.oklch.c = 0;
Expand All @@ -251,29 +251,24 @@ export class LightModeTheme implements ColorModeTheme {
}

private get fgOnAccent() {
const color = this.seedColor.clone();
const tint = this.seedColor.clone();
const shade = this.seedColor.clone();

if (this.seedColor.contrastAPCA(this.bg) <= -60) {
if (this.seedIsAchromatic) {
color.oklch.l = 0.985;
color.oklch.c = 0;
return color;
}

color.oklch.l = 0.985;
color.oklch.c = 0.016;
return color;
if (this.seedIsAchromatic) {
tint.oklch.c = 0;
shade.oklch.c = 0;
}

if (this.seedIsAchromatic) {
color.oklch.l = 0.15;
color.oklch.c = 0;
return color;
tint.oklch.l = 0.96;
shade.oklch.l = 0.23;

if (
-this.bgAccent.contrastAPCA(tint) >= this.bgAccent.contrastAPCA(shade)
) {
return tint;
}

color.oklch.l = 0.15;
color.oklch.c = 0.064;
return color;
return shade;
}

private get fgNegative() {
Expand All @@ -290,7 +285,7 @@ export class LightModeTheme implements ColorModeTheme {
private get bdAccent() {
const color = this.seedColor.clone();

if (this.seedColor.contrastAPCA(this.bg) >= -25) {
if (this.bg.contrastAPCA(this.seedColor) <= 25) {
if (this.seedIsAchromatic) {
color.oklch.l = 0.15;
color.oklch.c = 0;
Expand Down

0 comments on commit 2ce4e23

Please sign in to comment.