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.

Sign up for GitHub

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

Jump to bottom

chore: WDS elevation colors refinement #29618

Merged
merged 5 commits into from
Dec 14, 2023
Merged

chore: WDS elevation colors refinement #29618

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
312a18c
Base numbers
ichik Dec 12, 2023
c81f3fe
Typo
ichik Dec 14, 2023
cebdcdc
Adjust shadows (tmp for preview)
ichik Dec 14, 2023
c825bbf
Adjustments for very light seeds
ichik Dec 14, 2023
6378ab8
Fix tests
ichik Dec 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions app/client/packages/design-system/theming/src/color/src/DarkModeTheme.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -592,23 +592,23 @@ export class DarkModeTheme implements ColorModeTheme {
private get bgElevation1() {
const color = this.bg.clone();

color.oklch.l += 0.05;
color.oklch.l += 0.07;

return color;
}

private get bgElevation2() {
const color = this.bgElevation1.clone();

color.oklch.l += 0.05;
color.oklch.l += 0.04;

return color;
}

private get bgElevation3() {
const color = this.bgElevation2.clone();

color.oklch.l += 0.05;
color.oklch.l += 0.02;

return color;
}
Expand Down
17 changes: 14 additions & 3 deletions app/client/packages/design-system/theming/src/color/src/LightModeTheme.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ export class LightModeTheme implements ColorModeTheme {
}

if (!this.seedIsVeryLight) {
color.oklch.l = 0.985;
color.oklch.l = 0.96;
}

// Cold colors can have a bit more chroma while staying perceptually neutral
Expand Down Expand Up @@ -619,16 +619,27 @@ export class LightModeTheme implements ColorModeTheme {
private get bgElevation1() {
const color = this.bg.clone();

color.oklch.l += 0.01;
if (this.seedIsVeryLight) {
color.oklch.l += 0.015;
}

if (!this.seedIsVeryLight) {
color.oklch.l += 0.02;
}

return color;
}

private get bgElevation2() {
const color = this.bgElevation1.clone();

color.oklch.l += 0.01;
if (this.seedIsVeryLight) {
color.oklch.l += 0.012;
}

if (!this.seedIsVeryLight) {
color.oklch.l += 0.015;
}
return color;
}

Expand Down
10 changes: 5 additions & 5 deletions app/client/packages/design-system/theming/src/color/tests/LightModeTheme.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ describe("bg color", () => {

it("should return correct color when lightness < 0.93", () => {
const { bg } = new LightModeTheme("oklch(0.92 0.09 231)").getColors();
expect(bg).toBe("rgb(95.828% 98.573% 100%)");
expect(bg).toBe("rgb(92.567% 95.296% 96.777%)");
});

it("should return correct color when hue > 120 && hue < 300", () => {
Expand All @@ -18,12 +18,12 @@ describe("bg color", () => {

it("should return correct color when hue < 120 or hue > 300", () => {
const { bg } = new LightModeTheme("oklch(0.92 0.07 110)").getColors();
expect(bg).toBe("rgb(98.101% 98.258% 96.176%)");
expect(bg).toBe("rgb(94.827% 94.982% 92.913%)");
});

it("should return correct color when chroma < 0.04", () => {
const { bg } = new LightModeTheme("oklch(0.92 0.02 110)").getColors();
expect(bg).toBe("rgb(98.026% 98.026% 98.026%)");
expect(bg).toBe("rgb(94.752% 94.752% 94.752%)");
});
});

Expand Down Expand Up @@ -794,12 +794,12 @@ describe("bdFocus color", () => {

it("should return correct color when hue is between 0 and 55", () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@KelvinOm @jsartisan we're doing tests here for different hues, because we used to rotate focus hue based on accent, but we have removed that in favor of simpler logic. Should we just remove or simplify these tests?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We will need the tests for refactoring this code. Now, when we change the code, we need to make changes to the tests. This is expected.

const { bdFocus } = new LightModeTheme("oklch(0.85 0.1 30)").getColors();
expect(bdFocus).toEqual("rgb(100% 70.125% 64.059%)");
expect(bdFocus).toEqual("rgb(72.468% 27.962% 22.197%)");
});

it("should return correct color when hue > 340", () => {
const { bdFocus } = new LightModeTheme("oklch(0.85 0.1 350)").getColors();
expect(bdFocus).toEqual("rgb(100% 67.07% 84.709%)");
expect(bdFocus).toEqual("rgb(68.494% 27.322% 49.304%)");
});
});

Expand Down
6 changes: 3 additions & 3 deletions app/client/packages/design-system/theming/src/token/src/defaultTokens.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,9 @@
"1": "0px"
},
"boxShadow": {
"1": "0 2px 2px 0 rgba(0, 0, 0, 0.2)",
"2": "0 2px 2px 0 rgba(0, 0, 0, 0.2)",
"3": "0 2px 2px 0 rgba(0, 0, 0, 0.2)"
"1": "0 8px 8px 0 rgba(0, 0, 0, 0.05)",
"2": "0 4px 4px 0 rgba(0, 0, 0, 0.1)",
"3": "0 2px 2px 0 rgba(0, 0, 0, 0.15)"
},
"borderWidth": {
"1": "1px",
Expand Down
2 changes: 1 addition & 1 deletion app/client/packages/design-system/widgets/src/testing/Elevation.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export const Elevation = () => {
padding="spacing-6"
style={{
background: "var(--color-bg-elevation-3)",
boxShadow: "var(--box-shadow-1)",
boxShadow: "var(--box-shadow-3)",
}}
/>
</Flex>
Expand Down
Loading