fix(slider,overlay): ensure that pointer events in Slider are handled…

… as expected in Overlay (#4438) * fix(slider,overlay): ensure that pointer events in Slider are handled as expected in Overlay * chore: code review suggestion
adobe · May 10, 2024 · db193e8 · db193e8
1 parent 16efafc
commit db193e8
Show file tree

Hide file tree

Showing 4 changed files with 105 additions and 33 deletions.
diff --git a/packages/overlay/src/OverlayStack.ts b/packages/overlay/src/OverlayStack.ts
@@ -60,13 +60,14 @@ class OverlayStack {
      * @param event {ClickEvent}
      */
     handlePointerup = (): void => {
-        if (!this.stack.length) return;
-        if (!this.pointerdownPath?.length) return;
-
         // Test against the composed path in `pointerdown` in case the visitor moved their
         // pointer during the course of the interaction.
+        // Ensure that this value is cleared even if the work in this method goes undone.
         const composedPath = this.pointerdownPath;
         this.pointerdownPath = undefined;
+        if (!this.stack.length) return;
+        if (!composedPath?.length) return;
+
         const lastIndex = this.stack.length - 1;
         const nonAncestorOverlays = this.stack.filter((overlay, i) => {
             const inStack = composedPath.find(

diff --git a/packages/overlay/stories/overlay-element.stories.ts b/packages/overlay/stories/overlay-element.stories.ts
@@ -174,6 +174,29 @@ click.args = {
     type: 'auto',
 };
 
+export const withSlider = (): TemplateResult => html`
+    <sp-button id="triggerEl" variant="primary">Button popover</sp-button>
+    <sp-overlay trigger="triggerEl@click" placement="bottom">
+        <sp-popover tip>
+            <sp-dialog no-divider class="options-popover-content">
+                <p>Try clicking the slider after popover opens</p>
+                <p>It shouldn't close the popover</p>
+                <sp-slider
+                    value="5"
+                    step="0.5"
+                    min="0"
+                    max="20"
+                    label="Awesomeness"
+                ></sp-slider>
+                <sp-button>Press me</sp-button>
+            </sp-dialog>
+        </sp-popover>
+    </sp-overlay>
+`;
+withSlider.swc_vrt = {
+    skip: true,
+};
+
 export const hover = (args: Properties): TemplateResult => Template(args);
 hover.args = {
     interaction: 'hover',

diff --git a/packages/overlay/test/overlay-element.test.ts b/packages/overlay/test/overlay-element.test.ts
@@ -32,16 +32,21 @@ import '@spectrum-web-components/button/sp-button.js';
 import { sendMouse } from '../../../test/plugins/browser.js';
 import { Button } from '@spectrum-web-components/button';
 import { sendKeys } from '@web/test-runner-commands';
-import { click, receivesFocus } from '../stories/overlay-element.stories.js';
+import {
+    click,
+    receivesFocus,
+    withSlider,
+} from '../stories/overlay-element.stories.js';
 import {
     removeSlottableRequest,
     SlottableRequestEvent,
 } from '../src/slottable-request-event.js';
 import { stub } from 'sinon';
 import { OverlayStateEvent } from '@spectrum-web-components/overlay/src/events.js';
+import { Slider } from '@spectrum-web-components/slider/src/Slider.js';
 
 const OVERLAY_TYPES = ['modal', 'page', 'hint', 'auto', 'manual'] as const;
-type OverlayTypes = typeof OVERLAY_TYPES[number];
+type OverlayTypes = (typeof OVERLAY_TYPES)[number];
 
 async function styledFixture<T extends Element>(
     story: TemplateResult
@@ -536,23 +541,19 @@ describe('sp-overlay', () => {
             expect(hint2.open).to.be.false;
         });
         it('stays open when pointer enters overlay from trigger element', async () => {
-            const test = await styledFixture(
-                html`
-                    <div>
-                        <sp-button id="test-button">
-                            This is a button.
-                        </sp-button>
-                        <sp-overlay
-                            trigger="test-button@hover"
-                            type="hint"
-                            placement="bottom"
-                            offset="-10"
-                        >
-                            <sp-tooltip>Help text.</sp-tooltip>
-                        </sp-overlay>
-                    </div>
-                `
-            );
+            const test = await styledFixture(html`
+                <div>
+                    <sp-button id="test-button">This is a button.</sp-button>
+                    <sp-overlay
+                        trigger="test-button@hover"
+                        type="hint"
+                        placement="bottom"
+                        offset="-10"
+                    >
+                        <sp-tooltip>Help text.</sp-tooltip>
+                    </sp-overlay>
+                </div>
+            `);
 
             const button = test.querySelector('sp-button') as Button;
             const overlay = test.querySelector(
@@ -675,16 +676,14 @@ describe('sp-overlay', () => {
             await closed;
         });
         it('stays open when pointer enters overlay from trigger element: self managed', async () => {
-            const button = await styledFixture(
-                html`
-                    <sp-button>
-                        This is a button.
-                        <sp-tooltip self-managed placement="bottom">
-                            Help text.
-                        </sp-tooltip>
-                    </sp-button>
-                `
-            );
+            const button = await styledFixture(html`
+                <sp-button>
+                    This is a button.
+                    <sp-tooltip self-managed placement="bottom">
+                        Help text.
+                    </sp-tooltip>
+                </sp-button>
+            `);
 
             const el = button.querySelector('sp-tooltip') as Tooltip;
             const buttonRect = button.getBoundingClientRect();
@@ -809,6 +808,56 @@ describe('sp-overlay', () => {
 
             expect(document.activeElement === overlay).to.be.true;
         });
+        it('does not close when clicking a Slider track in the Overlay', async function () {
+            const test = await fixture(html`
+                <div>${withSlider()}</div>
+            `);
+            const el = test.querySelector('sp-overlay') as Overlay;
+            const button = test.querySelector('sp-button') as Button;
+            const slider = el.querySelector('sp-slider') as Slider;
+            const track = slider.shadowRoot.querySelector(
+                '#track'
+            ) as HTMLDivElement;
+
+            expect(el.open).to.be.false;
+
+            const opened = oneEvent(el, 'sp-opened');
+            const buttonRect = button.getBoundingClientRect();
+            sendMouse({
+                steps: [
+                    {
+                        type: 'click',
+                        position: [
+                            buttonRect.left + buttonRect.width / 2,
+                            buttonRect.top + buttonRect.height / 2,
+                        ],
+                    },
+                ],
+            });
+            await opened;
+
+            expect(el.open).to.be.true;
+            expect(slider.value).to.equal(5);
+
+            const sliderRect = track.getBoundingClientRect();
+
+            await sendMouse({
+                steps: [
+                    {
+                        type: 'click',
+                        position: [
+                            sliderRect.left + sliderRect.width - 5,
+                            sliderRect.top + sliderRect.height / 2,
+                        ],
+                    },
+                ],
+            });
+
+            await aTimeout(500);
+
+            expect(slider.value).to.equal(19.5);
+            expect(el.open).to.be.true;
+        });
     });
     describe('[type="manual"]', () => {
         opensDeclaratively('manual');

diff --git a/packages/slider/src/HandleController.ts b/packages/slider/src/HandleController.ts
@@ -401,7 +401,6 @@ export class HandleController {
         if (!this.draggingHandle) {
             return;
         }
-        event.stopPropagation();
         input.value = this.calculateHandlePosition(event, model).toString();
         model.handle.value = parseFloat(input.value);
         this.host.indeterminate = false;