From 5f1c3b5d2ecc0c44683517e3148819b6c93b1805 Mon Sep 17 00:00:00 2001 From: Michael Jordan Date: Tue, 13 Jun 2023 17:22:03 -0400 Subject: [PATCH 1/2] fix(color-area,color-slider): color-area labeling, RTL support, vertical slider orientation #3313 --- packages/color-area/src/ColorArea.ts | 177 +++++++++++++----- packages/color-area/src/color-area.css | 18 ++ packages/color-area/test/color-area.test.ts | 76 +++++--- packages/color-slider/src/ColorSlider.ts | 24 ++- packages/color-slider/src/color-slider.css | 4 + .../color-slider/test/color-slider.test.ts | 16 +- packages/color-wheel/src/ColorWheel.ts | 3 + 7 files changed, 233 insertions(+), 85 deletions(-) diff --git a/packages/color-area/src/ColorArea.ts b/packages/color-area/src/ColorArea.ts index a1b3baaadc5..5e975708c0a 100644 --- a/packages/color-area/src/ColorArea.ts +++ b/packages/color-area/src/ColorArea.ts @@ -30,6 +30,11 @@ import { ColorController, ColorValue, } from '@spectrum-web-components/reactive-controllers/src/Color.js'; +import { LanguageResolutionController } from '@spectrum-web-components/reactive-controllers/src/LanguageResolution.js'; +import { + isAndroid, + isIOS, +} from '@spectrum-web-components/shared/src/platform.js'; import styles from './color-area.css.js'; @@ -44,6 +49,9 @@ export class ColorArea extends SpectrumElement { return [styles]; } + @property({ type: String, reflect: true }) + public override dir!: 'ltr' | 'rtl'; + @property({ type: Boolean, reflect: true }) public disabled = false; @@ -62,15 +70,17 @@ export class ColorArea extends SpectrumElement { @query('.handle') private handle!: ColorHandle; + private languageResolver = new LanguageResolutionController(this); + private colorController = new ColorController(this, { extractColorFromState: () => ({ h: this.hue, s: this.x, - v: 1 - this.y, + v: this.y, }), applyColorToState: ({ s, v }) => { this.x = s; - this.y = 1 - v; + this.y = v; }, }); @@ -142,7 +152,7 @@ export class ColorArea extends SpectrumElement { this.requestUpdate('y', oldValue); } - private _y = 0; + private _y = 1; @property({ type: Number }) public step = 0.01; @@ -157,6 +167,8 @@ export class ColorArea extends SpectrumElement { private activeKeys = new Set(); + private _valueChanged = false; + public override focus(focusOptions: FocusOptions = {}): void { super.focus(focusOptions); this.forwardFocus(); @@ -171,15 +183,18 @@ export class ColorArea extends SpectrumElement { } } - private handleFocusin(): void { + private handleFocus(): void { this.focused = true; + this._valueChanged = false; } - private handleFocusout(): void { + private handleBlur(): void { if (this._pointerDown) { return; } + this.altered = 0; this.focused = false; + this._valueChanged = false; } private handleKeydown(event: KeyboardEvent): void { @@ -188,7 +203,11 @@ export class ColorArea extends SpectrumElement { this.altered = [event.shiftKey, event.ctrlKey, event.altKey].filter( (key) => !!key ).length; - const isArrowKey = code.search('Arrow') === 0; + const isArrowKey = + code.search('Arrow') === 0 || + code.search('Page') === 0 || + code.search('Home') === 0 || + code.search('End') === 0; if (isArrowKey) { event.preventDefault(); this.activeKeys.add(code); @@ -203,16 +222,28 @@ export class ColorArea extends SpectrumElement { this.activeKeys.forEach((code) => { switch (code) { case 'ArrowUp': - deltaY = step * -1; + deltaY = step; break; case 'ArrowDown': - deltaY = step * 1; + deltaY = step * -1; break; case 'ArrowLeft': - deltaX = step * -1; + deltaX = this.step * (this.isLTR ? -1 : 1); break; case 'ArrowRight': - deltaX = step * 1; + deltaX = this.step * (this.isLTR ? 1 : -1); + break; + case 'PageUp': + deltaY = step * 10; + break; + case 'PageDown': + deltaY = step * -10; + break; + case 'Home': + deltaX = step * (this.isLTR ? -10 : 10); + break; + case 'End': + deltaX = step * (this.isLTR ? 10 : -10); break; /* c8 ignore next 2 */ default: @@ -233,6 +264,7 @@ export class ColorArea extends SpectrumElement { this.colorController.applyColorFromState(); if (deltaX != 0 || deltaY != 0) { + this._valueChanged = true; this.dispatchEvent( new Event('input', { bubbles: true, @@ -296,8 +328,10 @@ export class ColorArea extends SpectrumElement { private handlePointermove(event: PointerEvent): void { const [x, y] = this.calculateHandlePosition(event); + this._valueChanged = false; + this.x = x; - this.y = y; + this.y = 1 - y; this.colorController.applyColorFromState(); this.dispatchEvent( new Event('input', { @@ -355,7 +389,7 @@ export class ColorArea extends SpectrumElement { Math.min(1, (offsetY - minOffsetY) / height) ); - return [percentX, percentY]; + return [this.isLTR ? percentX : 1 - percentX, percentY]; } private handleAreaPointerdown(event: PointerEvent): void { @@ -382,6 +416,32 @@ export class ColorArea extends SpectrumElement { } } + const isMobile = isAndroid() || isIOS(); + const defaultAriaLabel = 'Color Picker'; + const ariaLabel = this.label + ? `${this.label} ${defaultAriaLabel}` + : defaultAriaLabel; + const ariaRoleDescription = ifDefined( + isMobile ? undefined : '2d slider' + ); + + const ariaLabelX = this.labelX; + const ariaLabelY = this.labelY; + const ariaValueX = new Intl.NumberFormat( + this.languageResolver.language, + { + style: 'percent', + unitDisplay: 'narrow', + } + ).format(this.x); + const ariaValueY = new Intl.NumberFormat( + this.languageResolver.language, + { + style: 'percent', + unitDisplay: 'narrow', + } + ).format(this.y); + return html`
-
- -
-
- -
+
+
+ +
+
+ +
+
`; } @@ -451,8 +536,8 @@ export class ColorArea extends SpectrumElement { super.firstUpdated(changed); this.boundingClientRect = this.getBoundingClientRect(); - this.addEventListener('focusin', this.handleFocusin); - this.addEventListener('focusout', this.handleFocusout); + this.addEventListener('focus', this.handleFocus); + this.addEventListener('blur', this.handleBlur); this.addEventListener('keyup', this.handleKeyup); this.addEventListener('keydown', this.handleKeydown); } diff --git a/packages/color-area/src/color-area.css b/packages/color-area/src/color-area.css index 2636f409566..6cbba891044 100644 --- a/packages/color-area/src/color-area.css +++ b/packages/color-area/src/color-area.css @@ -32,3 +32,21 @@ governing permissions and limitations under the License. width: 100%; height: 100%; } + +:host([dir='rtl']) .gradient { + transform: scaleX(-1); +} + +.slider[orient='vertical'] { + appearance: slider-vertical; +} + +.slider:focus { + z-index: 1; +} + +.fieldset { + border: 0; + margin: 0; + padding: 0; +} diff --git a/packages/color-area/test/color-area.test.ts b/packages/color-area/test/color-area.test.ts index 2b83e4b9f77..2d002fc53ef 100644 --- a/packages/color-area/test/color-area.test.ts +++ b/packages/color-area/test/color-area.test.ts @@ -116,8 +116,20 @@ describe('ColorArea', () => { const inputX = el.shadowRoot.querySelector('input[name="x"]'); const inputY = el.shadowRoot.querySelector('input[name="y"]'); - expect(inputX?.getAttribute('aria-label')).to.equal('saturation'); - expect(inputY?.getAttribute('aria-label')).to.equal('luminosity'); + expect(inputX?.getAttribute('aria-label')).to.equal('Color Picker'); + expect(inputY?.getAttribute('aria-label')).to.equal('Color Picker'); + expect(inputX?.getAttribute('aria-roledescription')).to.equal( + '2d slider' + ); + expect(inputY?.getAttribute('aria-roledescription')).to.equal( + '2d slider' + ); + expect(inputX?.getAttribute('aria-valuetext')).to.equal( + '67%, saturation, 75%, luminosity' + ); + expect(inputY?.getAttribute('aria-valuetext')).to.equal( + '75%, luminosity, 67%, saturation' + ); }); it('overrides both X and Y labels with a provided "label" attribute', async () => { const el = await fixture( @@ -131,8 +143,12 @@ describe('ColorArea', () => { const inputX = el.shadowRoot.querySelector('input[name="x"]'); const inputY = el.shadowRoot.querySelector('input[name="y"]'); - expect(inputX?.getAttribute('aria-label')).to.equal('something custom'); - expect(inputY?.getAttribute('aria-label')).to.equal('something custom'); + expect(inputX?.getAttribute('aria-label')).to.equal( + 'something custom Color Picker' + ); + expect(inputY?.getAttribute('aria-label')).to.equal( + 'something custom Color Picker' + ); }); it('accepts `hue` values', async () => { const el = await fixture( @@ -164,7 +180,7 @@ describe('ColorArea', () => { expect(el.hue, 'hue').to.equal(100); expect(el.x, 'x').to.equal(0.67); - expect(el.y, 'y').to.equal(0.25); + expect(el.y, 'y').to.equal(0.75); }); it('accepts "color" values as hsla', async () => { const el = await fixture( @@ -177,14 +193,14 @@ describe('ColorArea', () => { expect(el.hue, 'hugh').to.equal(100); expect(el.x, 'ex').to.equal(0.67); - expect(el.y, 'why').to.equal(0.25); + expect(el.y, 'why').to.equal(0.75); el.color = 'hsla(120, 100%, 0, 1)'; await elementUpdated(el); expect(el.hue, 'hue 2').to.equal(120); expect(el.x, 'x 2').to.equal(0); - expect(el.y, 'y 2').to.equal(1); + expect(el.y, 'y 2').to.equal(0); }); it('accepts "color" values as rgb', async () => { const el = await fixture( @@ -197,7 +213,7 @@ describe('ColorArea', () => { expect(el.hue).to.equal(120); expect(el.x).to.equal(1); - expect(el.y).to.equal(0); + expect(el.y).to.equal(1); }); it('accepts "color" values as hex', async () => { const el = await fixture( @@ -210,7 +226,7 @@ describe('ColorArea', () => { expect(el.hue).to.equal(120); expect(el.x).to.equal(1); - expect(el.y).to.equal(0); + expect(el.y).to.equal(1); }); it('accepts "Arrow*" keypresses', async () => { const el = await fixture( @@ -221,7 +237,7 @@ describe('ColorArea', () => { expect(el.hue, 'hue').to.equal(100); expect(el.x, 'x').to.equal(0.67); - expect(el.y, 'y').to.equal(0.25); + expect(el.y, 'y').to.equal(0.75); el.inputX.focus(); await nextFrame(); @@ -238,7 +254,7 @@ describe('ColorArea', () => { await changeEvent; expect(el.x).to.equal(0.67); - expect(el.y).to.equal(0.23); + expect(el.y).to.equal(0.77); changeEvent = oneEvent(el, 'change'); await sendKeys({ @@ -252,7 +268,7 @@ describe('ColorArea', () => { await changeEvent; expect(el.x).to.equal(0.69); - expect(el.y).to.equal(0.23); + expect(el.y).to.equal(0.77); changeEvent = oneEvent(el, 'change'); await sendKeys({ @@ -266,7 +282,7 @@ describe('ColorArea', () => { await changeEvent; expect(el.x).to.equal(0.69); - expect(el.y).to.equal(0.25); + expect(el.y).to.equal(0.75); changeEvent = oneEvent(el, 'change'); await sendKeys({ @@ -280,7 +296,7 @@ describe('ColorArea', () => { await changeEvent; expect(el.x).to.equal(0.67); - expect(el.y).to.equal(0.25); + expect(el.y).to.equal(0.75); }); it('accepts "Arrow*" keypresses with alteration', async () => { const el = await fixture( @@ -293,7 +309,7 @@ describe('ColorArea', () => { el.focus(); expect(el.hue, 'hue').to.equal(100); expect(el.x, 'x').to.equal(0.67); - expect(el.y, 'y').to.equal(0.25); + expect(el.y, 'y').to.equal(0.75); await sendKeys({ down: 'Shift', @@ -312,7 +328,7 @@ describe('ColorArea', () => { expect(el.color).to.equal('hsl(100, 65%, 57%)'); expect(el.x, 'first').to.equal(0.67); - expect(el.y).to.equal(0.15); + expect(el.y).to.equal(0.85); await sendKeys({ press: 'ArrowRight', @@ -323,9 +339,9 @@ describe('ColorArea', () => { }); await elementUpdated(el); - expect(el.color).to.equal('hsl(100, 69%, 52%)'); - expect(el.x).to.equal(0.77); - expect(el.y).to.equal(0.15); + expect(el.color).to.equal('hsl(100, 66%, 56%)'); + expect(el.x).to.equal(0.69); + expect(el.y).to.equal(0.85); await sendKeys({ press: 'ArrowDown', @@ -337,9 +353,9 @@ describe('ColorArea', () => { await elementUpdated(el); - expect(el.color).to.equal('hsl(100, 63%, 46%)'); - expect(el.x).to.equal(0.77); - expect(el.y).to.equal(0.25); + expect(el.color).to.equal('hsl(100, 53%, 49%)'); + expect(el.x).to.equal(0.69); + expect(el.y).to.equal(0.75); await sendKeys({ press: 'ArrowLeft', @@ -357,7 +373,7 @@ describe('ColorArea', () => { expect(el.color).to.equal('hsl(100, 50%, 50%)'); expect(el.x, 'last').to.equal(0.67); - expect(el.y).to.equal(0.25); + expect(el.y).to.equal(0.75); }); it('accepts pointer events', async () => { const el = await fixture( @@ -382,7 +398,7 @@ describe('ColorArea', () => { expect(el.hue).to.equal(0); expect(el.x).to.equal(1); - expect(el.y).to.equal(0); + expect(el.y).to.equal(1); handle.dispatchEvent( new PointerEvent('pointerdown', { @@ -400,7 +416,7 @@ describe('ColorArea', () => { expect(el.hue).to.equal(0); expect(el.x).to.equal(1); - expect(el.y).to.equal(0); + expect(el.y).to.equal(1); const root = el.shadowRoot ? el.shadowRoot : el; const gradient = root.querySelector('.gradient') as HTMLElement; @@ -420,7 +436,7 @@ describe('ColorArea', () => { expect(el.hue).to.equal(0); expect(el.x).to.equal(1); - expect(el.y).to.equal(0); + expect(el.y).to.equal(1); gradient.dispatchEvent( new PointerEvent('pointerdown', { @@ -437,7 +453,7 @@ describe('ColorArea', () => { expect(el.hue).to.equal(0); expect(el.x, 'pointerdown x').to.equal(0.48); - expect(el.y, 'pointerdown y').to.equal(0.48); + expect(el.y, 'pointerdown y').to.equal(0.52); handle.dispatchEvent( new PointerEvent('pointermove', { @@ -464,7 +480,7 @@ describe('ColorArea', () => { expect(el.hue).to.equal(0); expect(el.x).to.equal(0.53); - expect(el.y).to.equal(0.53); + expect(el.y).to.equal(0.47); }); it('responds to events on the internal input element', async () => { const inputSpy = spy(); @@ -586,7 +602,7 @@ describe('ColorArea', () => { expect(el.hue, 'hue').to.equal(100); expect(el.x, 'x').to.equal(0.67); - expect(el.y, 'y').to.equal(0.25); + expect(el.y, 'y').to.equal(0.75); expect(el.color).to.equal('hsl(100, 50%, 50%)'); el.color = 'hsl(100, 0%, 50%)'; @@ -613,7 +629,7 @@ describe('ColorArea', () => { expect(el.hue).to.equal(100); expect(el.x, 'x').to.equal(0.67); - expect(el.y, 'y').to.equal(0.25); + expect(el.y, 'y').to.equal(0.75); expect(Math.abs(outputColor.h - inputColor.h)).to.be.lessThan(variance); expect(Math.abs(outputColor.s - inputColor.s)).to.be.lessThan(variance); diff --git a/packages/color-slider/src/ColorSlider.ts b/packages/color-slider/src/ColorSlider.ts index 2fdad8414d5..f860a0a29a2 100644 --- a/packages/color-slider/src/ColorSlider.ts +++ b/packages/color-slider/src/ColorSlider.ts @@ -30,6 +30,7 @@ import { ColorValue, HSL, } from '@spectrum-web-components/reactive-controllers/src/Color.js'; +import { LanguageResolutionController } from '@spectrum-web-components/reactive-controllers/src/LanguageResolution.js'; import styles from './color-slider.css.js'; @@ -44,6 +45,9 @@ export class ColorSlider extends Focusable { return [styles]; } + @property({ type: String, reflect: true }) + public override dir!: 'ltr' | 'rtl'; + @property({ type: Boolean, reflect: true }) public override disabled = false; @@ -59,6 +63,8 @@ export class ColorSlider extends Focusable { @property({ type: Boolean, reflect: true }) public vertical = false; + private languageResolver = new LanguageResolutionController(this); + private colorController = new ColorController(this, { /* c8 ignore next 3 */ applyColorToState: () => { @@ -138,9 +144,11 @@ export class ColorSlider extends Focusable { } event.preventDefault(); + const range = 360; + const mult = 100 / range; this.sliderHandlePosition = Math.min( 100, - Math.max(0, this.sliderHandlePosition + delta) + Math.max(0, this.sliderHandlePosition + delta * mult) ); this.value = 360 * (this.sliderHandlePosition / 100); this.colorController.applyColorFromState(); @@ -331,9 +339,23 @@ export class ColorSlider extends Focusable { class="slider" min="0" max="360" + aria-orientation=${ifDefined( + this.vertical ? 'vertical' : undefined + )} + orient=${ifDefined(this.vertical ? 'vertical' : undefined)} step=${this.step} aria-label=${this.label} .value=${String(this.value)} + aria-valuetext=${`${new Intl.NumberFormat( + this.languageResolver.language, + { + maximumFractionDigits: 0, + minimumIntegerDigits: 1, + style: 'unit', + unit: 'degree', + unitDisplay: 'narrow', + } + ).format(this.value)}`} @input=${this.handleInput} @change=${this.handleChange} @keydown=${this.handleKeydown} diff --git a/packages/color-slider/src/color-slider.css b/packages/color-slider/src/color-slider.css index 5fb33138dd6..ce99360c84f 100644 --- a/packages/color-slider/src/color-slider.css +++ b/packages/color-slider/src/color-slider.css @@ -25,6 +25,10 @@ governing permissions and limitations under the License. inset-block-end: 0; } +:host([vertical]) .slider { + appearance: slider-vertical; +} + :host(:focus) { outline: none; } diff --git a/packages/color-slider/test/color-slider.test.ts b/packages/color-slider/test/color-slider.test.ts index 06144f985bb..352d0f24f0b 100644 --- a/packages/color-slider/test/color-slider.test.ts +++ b/packages/color-slider/test/color-slider.test.ts @@ -227,7 +227,7 @@ describe('ColorSlider', () => { await elementUpdated(el); - expect(el.sliderHandlePosition).to.equal(2); + expect(el.sliderHandlePosition).to.equal(0.5555555555555556); input.dispatchEvent(arrowRightEvent()); input.dispatchEvent(arrowRightKeyupEvent()); @@ -236,7 +236,7 @@ describe('ColorSlider', () => { await elementUpdated(el); - expect(el.sliderHandlePosition).to.equal(4); + expect(el.sliderHandlePosition).to.equal(1.1111111111111112); input.dispatchEvent(arrowDownEvent()); input.dispatchEvent(arrowDownKeyupEvent()); @@ -245,7 +245,7 @@ describe('ColorSlider', () => { await elementUpdated(el); - expect(el.sliderHandlePosition).to.equal(2); + expect(el.sliderHandlePosition).to.equal(0.5555555555555556); input.dispatchEvent(arrowLeftEvent()); input.dispatchEvent(arrowLeftKeyupEvent()); @@ -276,7 +276,7 @@ describe('ColorSlider', () => { await elementUpdated(el); - expect(el.sliderHandlePosition).to.equal(2); + expect(el.sliderHandlePosition).to.equal(0.5555555555555556); input.dispatchEvent(arrowRightEvent()); input.dispatchEvent(arrowRightKeyupEvent()); @@ -294,7 +294,7 @@ describe('ColorSlider', () => { await elementUpdated(el); - expect(el.sliderHandlePosition).to.equal(2); + expect(el.sliderHandlePosition).to.equal(0.5555555555555556); input.dispatchEvent(arrowDownEvent()); input.dispatchEvent(arrowDownKeyupEvent()); @@ -328,7 +328,7 @@ describe('ColorSlider', () => { await elementUpdated(el); - expect(el.sliderHandlePosition).to.equal(20); + expect(el.sliderHandlePosition).to.equal(5.555555555555555); await sendKeys({ press: 'ArrowRight', @@ -339,7 +339,7 @@ describe('ColorSlider', () => { await elementUpdated(el); - expect(el.sliderHandlePosition).to.equal(40); + expect(el.sliderHandlePosition).to.equal(11.11111111111111); await sendKeys({ press: 'ArrowDown', @@ -350,7 +350,7 @@ describe('ColorSlider', () => { await elementUpdated(el); - expect(el.sliderHandlePosition).to.equal(20); + expect(el.sliderHandlePosition).to.equal(5.5555555555555545); await sendKeys({ press: 'ArrowLeft', diff --git a/packages/color-wheel/src/ColorWheel.ts b/packages/color-wheel/src/ColorWheel.ts index 7c2733b6caa..d91161a28a1 100644 --- a/packages/color-wheel/src/ColorWheel.ts +++ b/packages/color-wheel/src/ColorWheel.ts @@ -45,6 +45,9 @@ export class ColorWheel extends Focusable { return [styles]; } + @property({ type: String, reflect: true }) + public override dir!: 'ltr' | 'rtl'; + @property({ type: Boolean, reflect: true }) public override disabled = false; From 1f8f2b3cfc21b36c686d96e51b728ca4a6472923 Mon Sep 17 00:00:00 2001 From: Michael Jordan Date: Mon, 10 Jul 2023 15:50:18 -0400 Subject: [PATCH 2/2] ci: update golden images cache --- .circleci/config.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 1b1c2f8224d..371bffc8a9e 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -10,7 +10,7 @@ executors: parameters: current_golden_images_hash: type: string - default: 2f06eaa56e1938ea37a5c753193074e1baead124 + default: 5f1c3b5d2ecc0c44683517e3148819b6c93b1805 wireit_cache_name: type: string default: wireit