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

feat(color-picker): enable responsive layout #10904

Merged
merged 25 commits into from
Jan 29, 2025
Merged
Show file tree
Hide file tree
Changes from 24 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
dea748d
feat(color-picker): make responsive
jcfranco Nov 27, 2024
1333e58
merge dev
jcfranco Nov 28, 2024
3618715
Merge branch 'dev' into jcfranco/9415-update-color-picker-to-be-respo…
jcfranco Dec 3, 2024
d5a0fbc
update token vars
jcfranco Dec 4, 2024
5edd140
update layout for large widths
jcfranco Dec 4, 2024
0e6989b
add tests and tweak styles
jcfranco Dec 18, 2024
cb51491
tidy up
jcfranco Dec 18, 2024
6b72dc0
fix tests
jcfranco Dec 18, 2024
02cceff
throttle resize-handling logic
jcfranco Dec 18, 2024
25a2d2d
tweak responsive story to not exceed screenshot max
jcfranco Dec 18, 2024
2656805
tweak slider width rendering logic
jcfranco Dec 18, 2024
4279eaa
Merge branch 'dev' into jcfranco/9415-update-color-picker-to-be-respo…
jcfranco Dec 18, 2024
33de899
update tests
jcfranco Dec 19, 2024
8c5dc47
Merge branch 'dev' into jcfranco/9415-update-color-picker-to-be-respo…
jcfranco Jan 2, 2025
980d1d5
Merge branch 'dev' into jcfranco/9415-update-color-picker-to-be-respo…
jcfranco Jan 2, 2025
80ac37c
Merge branch 'dev' into jcfranco/9415-update-color-picker-to-be-respo…
jcfranco Jan 8, 2025
0bc0e9d
Merge branch 'dev' into jcfranco/9415-update-color-picker-to-be-respo…
jcfranco Jan 10, 2025
ab25bba
Merge branch 'dev' into jcfranco/9415-update-color-picker-to-be-respo…
jcfranco Jan 22, 2025
c994eb1
throttle canvas resizing handler instead of only canvas size update l…
jcfranco Jan 25, 2025
53179ed
only run color-picker tests for debugging
jcfranco Jan 26, 2025
26ba476
Merge branch 'dev' into jcfranco/9415-update-color-picker-to-be-respo…
jcfranco Jan 26, 2025
48ca5fc
set up resize observer after first update
jcfranco Jan 27, 2025
1b026ae
restore internal border calculation
jcfranco Jan 27, 2025
49ba00e
restore running all tests
jcfranco Jan 27, 2025
296085d
tidy up
jcfranco Jan 28, 2025
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
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,17 @@ import {
getElementRect,
} from "../../tests/utils";
import { html } from "../../../support/formatting";
import { CSS, DEFAULT_COLOR, DEFAULT_STORAGE_KEY_PREFIX, DIMENSIONS, SCOPE_SIZE } from "./resources";
import { CSS, DEFAULT_COLOR, DEFAULT_STORAGE_KEY_PREFIX, STATIC_DIMENSIONS, SCOPE_SIZE } from "./resources";
import { ColorValue } from "./interfaces";
import { getSliderWidth } from "./utils";
import { getColorFieldDimensions, getSliderWidth } from "./utils";
import type { ColorPicker } from "./color-picker";

type SpyInstance = MockInstance;

describe("calcite-color-picker", () => {
let consoleSpy: SpyInstance;
const defaultMediumWidthInPx = 240;
const centerColorFieldColor = "#408047";

async function clickScope(page: E2EPage, scope: "hue" | "color-field"): Promise<void> {
// helps workaround puppeteer not being able to click on a 0x0 element
Expand Down Expand Up @@ -117,9 +119,8 @@ describe("calcite-color-picker", () => {
]);
});

// #408047 is a color in the middle of the color field
describe("disabled", () => {
disabled("<calcite-color-picker value='#408047'></calcite-color-picker>");
disabled(html`<calcite-color-picker value="${centerColorFieldColor}"></calcite-color-picker>`);
});

describe("translation support", () => {
Expand Down Expand Up @@ -580,29 +581,30 @@ describe("calcite-color-picker", () => {
const picker = await page.find(`calcite-color-picker`);
const spy = await picker.spyOnEvent("calciteColorPickerChange");
let changes = 0;
const mediumScaleDimensions = DIMENSIONS.m;
const mediumScaleDimensions = STATIC_DIMENSIONS.m;
const widthOffset = 0.5;
const [colorFieldX, colorFieldY] = await getElementXY(page, "calcite-color-picker", `.${CSS.colorField}`);
const mediumScaleColorFieldDimensions = await getColorFieldDimensions(defaultMediumWidthInPx);

// clicking color field colors to pick a color
await page.mouse.click(colorFieldX, colorFieldY);
await page.waitForChanges();
expect(await picker.getProperty("value")).toBe("#ffffff");
expect(spy).toHaveReceivedEventTimes(++changes);

await page.mouse.click(colorFieldX, colorFieldY + mediumScaleDimensions.colorField.height - 0.1);
await page.mouse.click(colorFieldX, colorFieldY + mediumScaleColorFieldDimensions.height - 0.1);
await page.waitForChanges();
expect(await picker.getProperty("value")).toBe("#000000");
expect(spy).toHaveReceivedEventTimes(++changes);

await page.mouse.click(colorFieldX + mediumScaleDimensions.colorField.width - widthOffset, colorFieldY);
await page.mouse.click(colorFieldX + mediumScaleColorFieldDimensions.width - widthOffset, colorFieldY);
await page.waitForChanges();
expect(await picker.getProperty("value")).toBe("#ff0000");
expect(spy).toHaveReceivedEventTimes(++changes);

await page.mouse.click(
colorFieldX + mediumScaleDimensions.colorField.width - widthOffset,
colorFieldY + mediumScaleDimensions.colorField.height - 0.1,
colorFieldX + mediumScaleColorFieldDimensions.width - widthOffset,
colorFieldY + mediumScaleColorFieldDimensions.height - 0.1,
);
await page.waitForChanges();
expect(await picker.getProperty("value")).toBe("#000000");
Expand All @@ -615,7 +617,8 @@ describe("calcite-color-picker", () => {

// clicking on color slider to set hue
const colorsToSample = 7;
const offsetX = (getSliderWidth(mediumScaleDimensions, false) - widthOffset) / colorsToSample;
const offsetX =
(getSliderWidth(defaultMediumWidthInPx, mediumScaleDimensions, false) - widthOffset) / colorsToSample;
const [hueSliderX, hueSliderY] = await getElementXY(page, "calcite-color-picker", `.${CSS.hueSlider}`);

let x = hueSliderX;
Expand Down Expand Up @@ -661,7 +664,7 @@ describe("calcite-color-picker", () => {
(window as TestWindow).internalColor = color.color;
});

const middleOfSlider = getSliderWidth(mediumScaleDimensions, false) / 2;
const middleOfSlider = getSliderWidth(defaultMediumWidthInPx, mediumScaleDimensions, false) / 2;
await page.mouse.click(x + middleOfSlider, sliderHeight);

const internalColorChanged = await page.evaluate(() => {
Expand Down Expand Up @@ -771,7 +774,7 @@ describe("calcite-color-picker", () => {
const page = await newE2EPage();
await page.setContent(`<calcite-color-picker></calcite-color-picker>`);
const [hueSliderX] = await getElementXY(page, "calcite-color-picker", `.${CSS.hueSlider}`);
const sliderWidth = getSliderWidth(DIMENSIONS.m, false);
const sliderWidth = getSliderWidth(defaultMediumWidthInPx, STATIC_DIMENSIONS.m, false);

let [hueScopeX, hueScopeY] = await getElementXY(page, "calcite-color-picker", `.${CSS.hueScope}`);
let [hueScopeCenterX, hueScopeCenterY] = getScopeCenter(hueScopeX, hueScopeY);
Expand All @@ -785,7 +788,7 @@ describe("calcite-color-picker", () => {
[hueScopeX, hueScopeY] = await getElementXY(page, "calcite-color-picker", `.${CSS.hueScope}`);
[hueScopeCenterX, hueScopeCenterY] = getScopeCenter(hueScopeX, hueScopeY);

expect(hueScopeCenterX).toBe(hueSliderX + DIMENSIONS.m.thumb.radius);
expect(hueScopeCenterX).toBe(hueSliderX + STATIC_DIMENSIONS.m.thumb.radius);

await page.mouse.move(hueScopeCenterX, hueScopeCenterY);
await page.mouse.down();
Expand All @@ -796,7 +799,7 @@ describe("calcite-color-picker", () => {
[hueScopeX] = await getElementXY(page, "calcite-color-picker", `.${CSS.hueScope}`);
[hueScopeCenterX] = getScopeCenter(hueScopeX, hueScopeY);

expect(hueScopeCenterX).toBe(hueSliderX + sliderWidth - DIMENSIONS.m.thumb.radius);
expect(hueScopeCenterX).toBe(hueSliderX + sliderWidth - STATIC_DIMENSIONS.m.thumb.radius);
});

it("does not allow text selection when color field/sliders are used", async () => {
Expand Down Expand Up @@ -2310,7 +2313,8 @@ describe("calcite-color-picker", () => {
await page.waitForChanges();

const finalStyle = await scope.getComputedStyle();
expect(finalStyle.left).toBe(`${DIMENSIONS.m.colorField.width - SCOPE_SIZE / 2}px`);
const mediumScaleColorFieldDimensions = await getColorFieldDimensions(defaultMediumWidthInPx);
expect(finalStyle.left).toBe(`${mediumScaleColorFieldDimensions.width - SCOPE_SIZE / 2}px`);
});

it("allows nudging color's hue even if it does not change RGB value", async () => {
Expand All @@ -2330,7 +2334,7 @@ describe("calcite-color-picker", () => {

const getScopeLeftOffset = async () => parseFloat((await scope.getComputedStyle()).left);

expect(await getScopeLeftOffset()).toBeCloseTo(DIMENSIONS.m.thumb.radius - 0.5, 0);
expect(await getScopeLeftOffset()).toBeCloseTo(STATIC_DIMENSIONS.m.thumb.radius - 0.5, 0);

await nudgeAThirdOfSlider();
expect(await getScopeLeftOffset()).toBeCloseTo(58.9, 0);
Expand All @@ -2345,7 +2349,7 @@ describe("calcite-color-picker", () => {

// nudge it to wrap around
await scope.press("ArrowRight");
expect(await getScopeLeftOffset()).toBeCloseTo(DIMENSIONS.m.thumb.radius - 0.5, 0);
expect(await getScopeLeftOffset()).toBeCloseTo(STATIC_DIMENSIONS.m.thumb.radius - 0.5, 0);
});

it("allows editing hue slider via keyboard", async () => {
Expand Down Expand Up @@ -2384,15 +2388,16 @@ describe("calcite-color-picker", () => {

expect(await hueSliderScope.getComputedStyle()).toMatchObject({
top: "6.5px",
left: `${DIMENSIONS.m.thumb.radius - 0.5}px`,
left: `${STATIC_DIMENSIONS.m.thumb.radius - 0.5}px`,
});
});

describe("mouse", () => {
const scopeSizeOffset = 0.8;
const moveByInPx = 2;

it("should update value when color field scope is moved", async () => {
const page = await newE2EPage();
await page.setContent(`<calcite-color-picker ></calcite-color-picker>`);
await page.setContent(html`<calcite-color-picker value="${centerColorFieldColor}"></calcite-color-picker>`);
const colorPicker = await page.find("calcite-color-picker");

const [colorFieldScopeX, colorFieldScopeY] = await getElementXY(
Expand All @@ -2402,31 +2407,29 @@ describe("calcite-color-picker", () => {
);
const value = await colorPicker.getProperty("value");

await page.mouse.move(colorFieldScopeX, colorFieldScopeY + scopeSizeOffset);
await page.mouse.down();
await page.mouse.up();
await page.mouse.click(colorFieldScopeX - moveByInPx, colorFieldScopeY);
await page.waitForChanges();
expect(await colorPicker.getProperty("value")).not.toBe(value);
});

it("should update value when hue scope is moved", async () => {
const page = await newE2EPage();
await page.setContent(`<calcite-color-picker></calcite-color-picker>`);
await page.setContent(html`<calcite-color-picker value="${centerColorFieldColor}"></calcite-color-picker>`);
const colorPicker = await page.find("calcite-color-picker");

const [hueScopeX, hueScopeY] = await getElementXY(page, "calcite-color-picker", `.${CSS.hueScope}`);
const value = await colorPicker.getProperty("value");

await page.mouse.move(hueScopeX + scopeSizeOffset, hueScopeY);
await page.mouse.down();
await page.mouse.up();
await page.mouse.click(hueScopeX - moveByInPx, hueScopeY);
await page.waitForChanges();
expect(await colorPicker.getProperty("value")).not.toBe(value);
});

it("should update value when opacity scope is moved", async () => {
const page = await newE2EPage();
await page.setContent(`<calcite-color-picker alpha-channel></calcite-color-picker>`);
await page.setContent(
html`<calcite-color-picker alpha-channel value="${centerColorFieldColor}"></calcite-color-picker>`,
);
const [opacityScopeX, opacityScopeY] = await getElementXY(
page,
"calcite-color-picker",
Expand All @@ -2435,9 +2438,7 @@ describe("calcite-color-picker", () => {
const colorPicker = await page.find("calcite-color-picker");
const value = await colorPicker.getProperty("value");

await page.mouse.move(opacityScopeX - 2, opacityScopeY);
await page.mouse.down();
await page.mouse.up();
await page.mouse.click(opacityScopeX - moveByInPx, opacityScopeY);
await page.waitForChanges();
expect(await colorPicker.getProperty("value")).not.toBe(value);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,8 @@
:host([scale="s"]) {
--calcite-color-picker-spacing: 8px;

.container {
inline-size: 198px;
}
inline-size: 200px;
min-inline-size: 200px;
Copy link
Member

Choose a reason for hiding this comment

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

Any future potential for a scale prop here for sizing?

Copy link
Member Author

Choose a reason for hiding this comment

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

If you mean a custom CSS prop, it might be tricky since I also need static dimensions for canvas drawing.

Copy link
Member Author

Choose a reason for hiding this comment

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

I should DRY this up though.


.saved-colors {
@apply gap-1;
Expand All @@ -20,19 +19,17 @@
:host([scale="m"]) {
--calcite-color-picker-spacing: 12px;

.container {
inline-size: 238px;
}
inline-size: 240px;
min-inline-size: 240px;
}

:host([scale="l"]) {
--calcite-color-picker-spacing: 16px;

@apply text-n1h;
inline-size: 304px;
min-inline-size: 304px;

.container {
inline-size: 302px;
}
@apply text-n1h;

.section {
&:first-of-type {
Expand All @@ -45,14 +42,12 @@
}

.control-section {
@apply flex-nowrap items-baseline;
}

.control-section {
@apply flex-wrap;
@apply flex flex-col flex-wrap items-baseline;
}

.color-hex-options {
inline-size: 100%;

@apply flex
flex-shrink
flex-col
Expand All @@ -66,7 +61,9 @@

.container {
@apply bg-foreground-1;
display: inline-block;
display: flex;
flex-direction: column;
block-size: min-content;
border: 1px solid var(--calcite-color-border-1);
}

Expand Down Expand Up @@ -96,12 +93,7 @@
}

.hex-and-channels-group {
@apply w-full;
}

.hex-and-channels-group,
.control-section {
@apply flex flex-row flex-wrap;
@apply flex flex-col flex-wrap w-full;
}

.section {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { boolean, modesDarkDefault } from "../../../.storybook/utils";
import { boolean, createBreakpointStories, modesDarkDefault } from "../../../.storybook/utils";
import { html } from "../../../support/formatting";
import { ATTRIBUTES } from "../../../.storybook/resources";
import { ColorPicker } from "./color-picker";
Expand Down Expand Up @@ -83,3 +83,19 @@ export const Focus_TestOnly = (): string =>
Focus_TestOnly.parameters = {
chromatic: { delay: 2000 },
};

export const responsive = (): string =>
createBreakpointStories(html`
<style>
.breakpoint-story-container {
flex-wrap: wrap;
gap: 10px;
}
.breakpoint-story-container > * {
// we avoid full width to stay within Chromatic’s screenshot size limit
width: 25%;
}
</style>
<calcite-color-picker scale="{scale}"></calcite-color-picker>
<calcite-color-picker alpha-channel scale="{scale}"></calcite-color-picker>
`);
Loading
Loading