From 0281d0b0b633795a8433398aca9041e0d2d3aa37 Mon Sep 17 00:00:00 2001 From: konnorrogers Date: Thu, 26 Oct 2023 15:37:59 -0400 Subject: [PATCH 1/7] only emit sl-change when you stop dragging --- .../color-picker/color-picker.component.ts | 44 +++++++++++++------ 1 file changed, 31 insertions(+), 13 deletions(-) diff --git a/src/components/color-picker/color-picker.component.ts b/src/components/color-picker/color-picker.component.ts index 96b5184cfc..3ed5f1c471 100644 --- a/src/components/color-picker/color-picker.component.ts +++ b/src/components/color-picker/color-picker.component.ts @@ -243,7 +243,8 @@ export default class SlColorPicker extends ShoelaceElement implements ShoelaceFo const container = this.shadowRoot!.querySelector('.color-picker__slider.color-picker__alpha')!; const handle = container.querySelector('.color-picker__slider-handle')!; const { width } = container.getBoundingClientRect(); - let oldValue = this.value; + let initialValue = this.value; + let currentValue = this.value; handle.focus(); event.preventDefault(); @@ -253,12 +254,17 @@ export default class SlColorPicker extends ShoelaceElement implements ShoelaceFo this.alpha = clamp((x / width) * 100, 0, 100); this.syncValues(); - if (this.value !== oldValue) { - oldValue = this.value; - this.emit('sl-change'); + if (this.value !== currentValue) { + currentValue = this.value; this.emit('sl-input'); } }, + onStop: () => { + if (this.value !== initialValue) { + initialValue = this.value; + this.emit('sl-change'); + } + }, initialEvent: event }); } @@ -267,7 +273,8 @@ export default class SlColorPicker extends ShoelaceElement implements ShoelaceFo const container = this.shadowRoot!.querySelector('.color-picker__slider.color-picker__hue')!; const handle = container.querySelector('.color-picker__slider-handle')!; const { width } = container.getBoundingClientRect(); - let oldValue = this.value; + let initialValue = this.value; + let currentValue = this.value; handle.focus(); event.preventDefault(); @@ -277,12 +284,17 @@ export default class SlColorPicker extends ShoelaceElement implements ShoelaceFo this.hue = clamp((x / width) * 360, 0, 360); this.syncValues(); - if (this.value !== oldValue) { - oldValue = this.value; - this.emit('sl-change'); + if (this.value !== currentValue) { + currentValue = this.value; this.emit('sl-input'); } }, + onStop: () => { + if (this.value !== initialValue) { + initialValue = this.value; + this.emit('sl-change'); + } + }, initialEvent: event }); } @@ -291,7 +303,8 @@ export default class SlColorPicker extends ShoelaceElement implements ShoelaceFo const grid = this.shadowRoot!.querySelector('.color-picker__grid')!; const handle = grid.querySelector('.color-picker__grid-handle')!; const { width, height } = grid.getBoundingClientRect(); - let oldValue = this.value; + let initialValue = this.value; + let currentValue = this.value; handle.focus(); event.preventDefault(); @@ -304,13 +317,18 @@ export default class SlColorPicker extends ShoelaceElement implements ShoelaceFo this.brightness = clamp(100 - (y / height) * 100, 0, 100); this.syncValues(); - if (this.value !== oldValue) { - oldValue = this.value; - this.emit('sl-change'); + if (this.value !== currentValue) { + currentValue = this.value; this.emit('sl-input'); } }, - onStop: () => (this.isDraggingGridHandle = false), + onStop: () => { + this.isDraggingGridHandle = false; + if (this.value !== initialValue) { + initialValue = this.value; + this.emit('sl-change'); + } + }, initialEvent: event }); } From 1c8a73fc0d3da2d60f17ffbbdb3a6525d0fa73cc Mon Sep 17 00:00:00 2001 From: konnorrogers Date: Thu, 26 Oct 2023 16:44:27 -0400 Subject: [PATCH 2/7] only emit sl-change when you stop dragging --- .../color-picker/color-picker.test.ts | 46 ++++++++++++++++--- src/internal/test.ts | 12 ++++- 2 files changed, 51 insertions(+), 7 deletions(-) diff --git a/src/components/color-picker/color-picker.test.ts b/src/components/color-picker/color-picker.test.ts index a05f7f137d..de6a9277d2 100644 --- a/src/components/color-picker/color-picker.test.ts +++ b/src/components/color-picker/color-picker.test.ts @@ -1,6 +1,6 @@ import '../../../dist/shoelace.js'; import { aTimeout, expect, fixture, html, oneEvent } from '@open-wc/testing'; -import { clickOnElement } from '../../internal/test.js'; +import { clickOnElement, dragElement } from '../../internal/test.js'; import { runFormControlBaseTests } from '../../internal/test/form-control-base-tests.js'; import { sendKeys } from '@web/test-runner-commands'; import { serialize } from '../../utilities/form.js'; @@ -31,11 +31,22 @@ describe('', () => { await clickOnElement(trigger); // open the dropdown await aTimeout(200); // wait for the dropdown to open - await clickOnElement(grid); // click on the grid + + // Simulate a drag event. "sl-change" should not fire until we stop dragging. + await dragElement(grid, 2, 0, { + afterMouseDown: async () => { + expect(changeHandler).to.have.not.been.called; + expect(inputHandler).to.have.been.calledOnce; + }, + afterMouseMove: async () => { + expect(inputHandler).to.have.been.calledTwice; + } + }); + await el.updateComplete; expect(changeHandler).to.have.been.calledOnce; - expect(inputHandler).to.have.been.calledOnce; + expect(inputHandler).to.have.been.calledTwice; }); it('should emit sl-change and sl-input when the hue slider is moved', async () => { @@ -50,10 +61,22 @@ describe('', () => { await clickOnElement(trigger); // open the dropdown await aTimeout(200); // wait for the dropdown to open - await clickOnElement(slider); // click on the hue slider + // Simulate a drag event. "sl-change" should not fire until we stop dragging. + await dragElement(slider, 20, 0, { + afterMouseDown: async () => { + expect(changeHandler).to.have.not.been.called; + expect(inputHandler).to.have.been.calledOnce; + }, + afterMouseMove: async () => { + // It's not twice because you can't change the hue of white! + expect(inputHandler).to.have.been.calledOnce; + } + }); + await el.updateComplete; expect(changeHandler).to.have.been.calledOnce; + // It's not twice because you can't change the hue of white! expect(inputHandler).to.have.been.calledOnce; }); @@ -69,11 +92,22 @@ describe('', () => { await clickOnElement(trigger); // open the dropdown await aTimeout(200); // wait for the dropdown to open - await clickOnElement(slider); // click on the opacity slider + + // Simulate a drag event. "sl-change" should not fire until we stop dragging. + await dragElement(slider, 2, 0, { + afterMouseDown: async () => { + expect(changeHandler).to.have.not.been.called; + expect(inputHandler).to.have.been.calledOnce; + }, + afterMouseMove: async () => { + expect(inputHandler).to.have.been.calledTwice; + } + }); + await el.updateComplete; expect(changeHandler).to.have.been.calledOnce; - expect(inputHandler).to.have.been.calledOnce; + expect(inputHandler).to.have.been.calledTwice; }); it('should emit sl-change and sl-input when toggling the format', async () => { diff --git a/src/internal/test.ts b/src/internal/test.ts index 607753fcdb..1201bd5d8f 100644 --- a/src/internal/test.ts +++ b/src/internal/test.ts @@ -73,11 +73,21 @@ export async function dragElement( /** The horizontal distance to drag in pixels */ deltaX = 0, /** The vertical distance to drag in pixels */ - deltaY = 0 + deltaY = 0, + callbacks: { + afterMouseDown?: () => Promise + afterMouseMove?: () => Promise + } = {} ): Promise { await moveMouseOnElement(el); await sendMouse({ type: 'down' }); + + await callbacks.afterMouseDown?.() + const { clickX, clickY } = determineMousePosition(el, 'center', deltaX, deltaY); await sendMouse({ type: 'move', position: [clickX, clickY] }); + + await callbacks.afterMouseMove?.() + await sendMouse({ type: 'up' }); } From 662cf8297f0939ffc0096cd3f314650f297ed899 Mon Sep 17 00:00:00 2001 From: konnorrogers Date: Thu, 26 Oct 2023 16:46:17 -0400 Subject: [PATCH 3/7] prettier --- src/components/color-picker/color-picker.test.ts | 12 ++++++------ src/internal/test.ts | 8 ++++---- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/components/color-picker/color-picker.test.ts b/src/components/color-picker/color-picker.test.ts index de6a9277d2..92249c96c4 100644 --- a/src/components/color-picker/color-picker.test.ts +++ b/src/components/color-picker/color-picker.test.ts @@ -34,11 +34,11 @@ describe('', () => { // Simulate a drag event. "sl-change" should not fire until we stop dragging. await dragElement(grid, 2, 0, { - afterMouseDown: async () => { + afterMouseDown: () => { expect(changeHandler).to.have.not.been.called; expect(inputHandler).to.have.been.calledOnce; }, - afterMouseMove: async () => { + afterMouseMove: () => { expect(inputHandler).to.have.been.calledTwice; } }); @@ -63,11 +63,11 @@ describe('', () => { await aTimeout(200); // wait for the dropdown to open // Simulate a drag event. "sl-change" should not fire until we stop dragging. await dragElement(slider, 20, 0, { - afterMouseDown: async () => { + afterMouseDown: () => { expect(changeHandler).to.have.not.been.called; expect(inputHandler).to.have.been.calledOnce; }, - afterMouseMove: async () => { + afterMouseMove: () => { // It's not twice because you can't change the hue of white! expect(inputHandler).to.have.been.calledOnce; } @@ -95,11 +95,11 @@ describe('', () => { // Simulate a drag event. "sl-change" should not fire until we stop dragging. await dragElement(slider, 2, 0, { - afterMouseDown: async () => { + afterMouseDown: () => { expect(changeHandler).to.have.not.been.called; expect(inputHandler).to.have.been.calledOnce; }, - afterMouseMove: async () => { + afterMouseMove: () => { expect(inputHandler).to.have.been.calledTwice; } }); diff --git a/src/internal/test.ts b/src/internal/test.ts index 1201bd5d8f..050f5efdef 100644 --- a/src/internal/test.ts +++ b/src/internal/test.ts @@ -75,19 +75,19 @@ export async function dragElement( /** The vertical distance to drag in pixels */ deltaY = 0, callbacks: { - afterMouseDown?: () => Promise - afterMouseMove?: () => Promise + afterMouseDown?: () => void | Promise; + afterMouseMove?: () => void | Promise; } = {} ): Promise { await moveMouseOnElement(el); await sendMouse({ type: 'down' }); - await callbacks.afterMouseDown?.() + await callbacks.afterMouseDown?.(); const { clickX, clickY } = determineMousePosition(el, 'center', deltaX, deltaY); await sendMouse({ type: 'move', position: [clickX, clickY] }); - await callbacks.afterMouseMove?.() + await callbacks.afterMouseMove?.(); await sendMouse({ type: 'up' }); } From b1108b809722f52fefed1479351f8a379059bdb4 Mon Sep 17 00:00:00 2001 From: konnorrogers Date: Fri, 27 Oct 2023 15:00:26 -0400 Subject: [PATCH 4/7] add changelog entry --- docs/pages/resources/changelog.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/docs/pages/resources/changelog.md b/docs/pages/resources/changelog.md index a92ea1a06b..a6c6022149 100644 --- a/docs/pages/resources/changelog.md +++ b/docs/pages/resources/changelog.md @@ -12,6 +12,10 @@ Components with the Experimental bad New versions of Shoelace are released as-needed and generally occur when a critical mass of changes have accumulated. At any time, you can see what's coming in the next release by visiting [next.shoelace.style](https://next.shoelace.style). +## Next + +- Fixed a bug where the `"sl-change"` event would always fire simultaneously with `"sl-input"` event in ``. The `` event now only fires when a user stops dragging a slider or stops dragging on the color canvas. + ## 2.11.2 - Fixed a bug in `` component that caused an error to be thrown when rendered with Lit [#1684] @@ -1670,4 +1674,4 @@ The following pages demonstrate why this change was necessary. ## 2.0.0-beta.1 -- Initial release +- Initial release \ No newline at end of file From 0f57c91ce6866b9b9b091bb8906dfb6ac62ea49d Mon Sep 17 00:00:00 2001 From: konnorrogers Date: Fri, 27 Oct 2023 16:16:27 -0400 Subject: [PATCH 5/7] update changelog --- docs/pages/resources/changelog.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/pages/resources/changelog.md b/docs/pages/resources/changelog.md index a6c6022149..755c17cfeb 100644 --- a/docs/pages/resources/changelog.md +++ b/docs/pages/resources/changelog.md @@ -1674,4 +1674,4 @@ The following pages demonstrate why this change was necessary. ## 2.0.0-beta.1 -- Initial release \ No newline at end of file +- Initial release From 5c6cb384ab9c1be718d282c84bb58f1e33a5e568 Mon Sep 17 00:00:00 2001 From: konnorrogers Date: Tue, 31 Oct 2023 13:37:39 -0400 Subject: [PATCH 6/7] update changelog --- docs/pages/resources/changelog.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/pages/resources/changelog.md b/docs/pages/resources/changelog.md index 755c17cfeb..4e6258062f 100644 --- a/docs/pages/resources/changelog.md +++ b/docs/pages/resources/changelog.md @@ -14,7 +14,8 @@ New versions of Shoelace are released as-needed and generally occur when a criti ## Next -- Fixed a bug where the `"sl-change"` event would always fire simultaneously with `"sl-input"` event in ``. The `` event now only fires when a user stops dragging a slider or stops dragging on the color canvas. +- Fixed a bug with bundled components using CDN builds not having translations on initial connect [#1696] +- Fixed a bug where the `"sl-change"` event would always fire simultaneously with `"sl-input"` event in ``. The `` event now only fires when a user stops dragging a slider or stops dragging on the color canvas. [#1689] ## 2.11.2 @@ -1674,4 +1675,4 @@ The following pages demonstrate why this change was necessary. ## 2.0.0-beta.1 -- Initial release +- Initial release \ No newline at end of file From 30ba2718dad9258a5b8b0c23c8f3583e1d8606c0 Mon Sep 17 00:00:00 2001 From: konnorrogers Date: Tue, 31 Oct 2023 13:52:45 -0400 Subject: [PATCH 7/7] update changelog --- docs/pages/resources/changelog.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/pages/resources/changelog.md b/docs/pages/resources/changelog.md index 4e6258062f..e1b8bc871d 100644 --- a/docs/pages/resources/changelog.md +++ b/docs/pages/resources/changelog.md @@ -1675,4 +1675,4 @@ The following pages demonstrate why this change was necessary. ## 2.0.0-beta.1 -- Initial release \ No newline at end of file +- Initial release