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): adjust thumb and color preview sizes to follow updated spec #9523

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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 @@ -11,6 +11,7 @@ import {
import { html } from "../../../support/formatting";
import { CSS, DEFAULT_COLOR, DEFAULT_STORAGE_KEY_PREFIX, DIMENSIONS, SCOPE_SIZE } from "./resources";
import { ColorValue } from "./interfaces";
import { getSliderWidth } from "./utils";

type SpyInstance = jest.SpyInstance;

Expand Down Expand Up @@ -521,7 +522,7 @@ describe("calcite-color-picker", () => {

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

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

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

const internalColorChanged = await page.evaluate(() => {
Expand Down Expand Up @@ -677,6 +678,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);

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

await page.mouse.move(hueScopeCenterX, hueScopeCenterY);
await page.mouse.down();
await page.mouse.move(hueScopeCenterX + DIMENSIONS.m.slider.width, hueScopeCenterY);
await page.mouse.move(hueScopeCenterX + sliderWidth, hueScopeCenterY);
await page.mouse.up();
await page.waitForChanges();

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

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

describe("unsupported value handling", () => {
Expand Down Expand Up @@ -2213,16 +2215,15 @@ describe("calcite-color-picker", () => {
expect(await getScopeLeftOffset()).toBeCloseTo(DIMENSIONS.m.thumb.radius - 0.5, 0);

await nudgeAQuarterOfSlider();
expect(await getScopeLeftOffset()).toBeCloseTo(67.5, 0);
expect(await getScopeLeftOffset()).toBeCloseTo(70, 0);

await nudgeAQuarterOfSlider();
expect(await getScopeLeftOffset()).toBeCloseTo(135.5, 0);
expect(await getScopeLeftOffset()).toBeCloseTo(141, 0);

await nudgeAQuarterOfSlider();
// hue wraps around, so we nudge it back to assert position at the edge
await scope.press("ArrowLeft");
expect(await getScopeLeftOffset()).toBeLessThanOrEqual(193.5);
expect(await getScopeLeftOffset()).toBeGreaterThan(189.5);
expect(await getScopeLeftOffset()).toBeCloseTo(204.5, 0);

// nudge it to wrap around
await scope.press("ArrowRight");
Expand Down Expand Up @@ -2257,7 +2258,7 @@ describe("calcite-color-picker", () => {
const hueSliderScope = await page.find(`calcite-color-picker >>> .${CSS.hueScope}`);

expect(await hueSliderScope.getComputedStyle()).toMatchObject({
top: "9.5px",
top: "6.5px",
left: `${DIMENSIONS.m.thumb.radius - 0.5}px`,
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@
pointer-events-none;
&:focus {
@apply focus-outset;
outline-offset: 11px;
outline-offset: 6px;
}
}

Expand All @@ -111,6 +111,7 @@
.sliders {
@apply flex flex-col justify-between;
margin-inline-start: var(--calcite-color-picker-spacing);
gap: var(--calcite-spacing-xxs);
}

.preview-and-sliders {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,10 @@ export const simple = (args: ColorPickerArgs): string => html`
></calcite-color-picker>
`;

export const alphaChannel = (): string => html`
export const alphaChannelAllScales = (): string => html`
<calcite-color-picker scale="s" alpha-channel value="#b33f3333"></calcite-color-picker>
<calcite-color-picker scale="m" alpha-channel value="#b33f3333"></calcite-color-picker>
<calcite-color-picker scale="l" alpha-channel value="#b33f3333"></calcite-color-picker>
`;

export const disabled_TestOnly = (): string => html`<calcite-color-picker disabled></calcite-color-picker>`;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ import {
colorEqual,
CSSColorMode,
Format,
getSliderWidth,
hexify,
normalizeAlpha,
normalizeColor,
Expand Down Expand Up @@ -742,7 +743,6 @@ export class ColorPicker
colorFieldScopeLeft,
colorFieldScopeTop,
dimensions: {
slider: { width: sliderWidth },
thumb: { radius: thumbRadius },
},
hexDisabled,
Expand All @@ -758,6 +758,8 @@ export class ColorPicker
scale,
scopeOrientation,
} = this;
const sliderWidth = this.getSliderWidth();

const selectedColorInHex = color ? hexify(color, alphaChannel) : null;
const hueTop = thumbRadius;
const hueLeft = hueScopeLeft ?? (sliderWidth * DEFAULT_COLOR.hue()) / HSV_LIMITS.h;
Expand Down Expand Up @@ -809,7 +811,11 @@ export class ColorPicker
/>
</div>
<div class={CSS.previewAndSliders}>
<calcite-color-picker-swatch class={CSS.preview} color={selectedColorInHex} scale="l" />
<calcite-color-picker-swatch
class={CSS.preview}
color={selectedColorInHex}
scale={this.alphaChannel ? "l" : this.scale}
/>
<div class={CSS.sliders}>
<div class={CSS.controlAndScope}>
<canvas
Expand Down Expand Up @@ -1104,22 +1110,14 @@ export class ColorPicker
}

private captureHueSliderColor(x: number): void {
const {
dimensions: {
slider: { width },
},
} = this;
const width = this.getSliderWidth();
const hue = (HUE_LIMIT_CONSTRAINED / width) * x;

this.internalColorSet(this.baseColorFieldColor.hue(hue), false);
}

private captureOpacitySliderValue(x: number): void {
const {
dimensions: {
slider: { width },
},
} = this;
const width = this.getSliderWidth();
const alpha = opacityToAlpha((OPACITY_LIMITS.max / width) * x);

this.internalColorSet(this.baseColorFieldColor.alpha(alpha), false);
Expand Down Expand Up @@ -1349,7 +1347,7 @@ export class ColorPicker
}

const adjustedSliderDimensions = {
width: dimensions.slider.width,
width: this.getSliderWidth(),
height:
dimensions.slider.height + (dimensions.thumb.radius - dimensions.slider.height / 2) * 2,
};
Expand Down Expand Up @@ -1444,11 +1442,12 @@ export class ColorPicker

const {
dimensions: {
slider: { width },
thumb: { radius },
},
} = this;

const width = this.getSliderWidth();

const x = hsvColor.hue() / (HUE_LIMIT_CONSTRAINED / width);
const y = radius;
const sliderBoundX = this.getSliderBoundX(x, width, radius);
Expand All @@ -1460,17 +1459,22 @@ export class ColorPicker
this.drawThumb(this.hueSliderRenderingContext, radius, sliderBoundX, y, hsvColor, false);
}

private getSliderWidth(): number {
return getSliderWidth(this.dimensions, this.alphaChannel);
}

private drawHueSlider(): void {
const context = this.hueSliderRenderingContext;
const {
dimensions: {
slider: { height, width },
slider: { height },
thumb: { radius: thumbRadius },
},
} = this;

const x = 0;
const y = thumbRadius - height / 2;
const width = this.getSliderWidth();

const gradient = context.createLinearGradient(0, 0, width, 0);

Expand Down Expand Up @@ -1511,13 +1515,14 @@ export class ColorPicker
const {
baseColorFieldColor: previousColor,
dimensions: {
slider: { height, width },
slider: { height },
thumb: { radius: thumbRadius },
},
} = this;

const x = 0;
const y = thumbRadius - height / 2;
const width = this.getSliderWidth();

context.clearRect(0, 0, width, height + this.getSliderCapSpacing() * 2);

Expand Down Expand Up @@ -1599,11 +1604,11 @@ export class ColorPicker

const {
dimensions: {
slider: { width },
thumb: { radius },
},
} = this;

const width = this.getSliderWidth();
const x = alphaToOpacity(hsvColor.alpha()) / (OPACITY_LIMITS.max / width);
const y = radius;
const sliderBoundX = this.getSliderBoundX(x, width, radius);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,10 @@ export const DIMENSIONS = {
width: 160,
},
thumb: {
radius: 10,
radius: 7,
},
preview: {
size: 20,
},
},
m: {
Expand All @@ -80,7 +83,10 @@ export const DIMENSIONS = {
width: 272,
},
thumb: {
radius: 10,
radius: 7,
},
preview: {
size: 24,
},
},
l: {
Expand All @@ -93,7 +99,10 @@ export const DIMENSIONS = {
width: 464,
},
thumb: {
radius: 10,
radius: 7,
},
preview: {
size: 32,
},
},
};
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import Color from "color";
import { Scale } from "../interfaces";
import { ColorValue, HSLA, HSVA, RGB, RGBA } from "./interfaces";
import { DIMENSIONS } from "./resources";

export const hexChar = /^[0-9A-F]$/i;
const shorthandHex = /^#[0-9A-F]{3}$/i;
Expand Down Expand Up @@ -257,3 +259,18 @@ export function toNonAlphaMode(mode: SupportedMode): SupportedMode {

return nonAlphaMode;
}

export function getSliderWidth(activeDimensions: (typeof DIMENSIONS)[Scale], hasAlpha: boolean): number {
const {
slider: { width },
preview,
} = activeDimensions;

if (hasAlpha) {
return width;
}

const previewWidthOffset = DIMENSIONS["l"].preview.size - preview.size;

return width + previewWidthOffset;
}
Loading