From 16756b56c0f7f9561426acc202997fb098e8f19a Mon Sep 17 00:00:00 2001
From: Westbrook Johnson <westbrook.johnson@gmail.com>
Date: Fri, 17 Sep 2021 10:43:28 -0400
Subject: [PATCH] fix: update screen reader interface with menu items list

---
 .../stories/overlay-story-components.ts       | 84 +++++--------------
 packages/overlay/stories/overlay.stories.ts   | 61 ++++++++++++++
 packages/overlay/test/overlay.test.ts         | 21 ++++-
 packages/picker/src/Picker.ts                 | 15 ++++
 packages/picker/src/picker.css                |  7 +-
 packages/popover/src/popover.css              | 13 +++
 6 files changed, 133 insertions(+), 68 deletions(-)

diff --git a/packages/overlay/stories/overlay-story-components.ts b/packages/overlay/stories/overlay-story-components.ts
index 5aa38cb555..f128395a2a 100644
--- a/packages/overlay/stories/overlay-story-components.ts
+++ b/packages/overlay/stories/overlay-story-components.ts
@@ -20,7 +20,7 @@ import {
     query,
 } from '@spectrum-web-components/base';
 
-import { Overlay, Placement } from '../';
+import { Overlay, OverlayTrigger, Placement } from '../';
 import { RadioGroup } from '@spectrum-web-components/radio';
 import '@spectrum-web-components/button/sp-button.js';
 import { Button } from '@spectrum-web-components/button';
@@ -28,7 +28,6 @@ import '@spectrum-web-components/popover/sp-popover.js';
 import '@spectrum-web-components/radio/sp-radio.js';
 import '@spectrum-web-components/radio/sp-radio-group.js';
 import '@spectrum-web-components/overlay/overlay-trigger.js';
-import { Picker } from '@spectrum-web-components/picker';
 
 // Prevent infinite recursion in browser
 const MAX_DEPTH = 7;
@@ -307,72 +306,31 @@ class RecursivePopover extends LitElement {
 customElements.define('recursive-popover', RecursivePopover);
 
 export class PopoverContent extends LitElement {
-    @query('sp-picker')
-    public picker!: Picker;
+    @query('[slot="trigger"]')
+    public button!: Button;
+
+    @query('overlay-trigger')
+    public trigger!: OverlayTrigger;
 
     render(): TemplateResult {
         return html`
-            <sp-field-label for="picker1">Test</sp-field-label>
-
-            <sp-picker id="picker1" focusable .label=${'test'}>
-                <sp-menu-item value=${'timeSinceLastSynced'}>
-                    ${'test'}
-                </sp-menu-item>
-                <sp-menu-item value=${'timeSinceLastSynced'}>
-                    ${'test'}
-                </sp-menu-item>
-                <sp-menu-item value=${'timeSinceLastSynced'}>
-                    ${'test'}
-                </sp-menu-item>
-                <sp-menu-item value=${'timeSinceLastSynced'}>
-                    ${'test'}
-                </sp-menu-item>
-                <sp-menu-item value=${'timeSinceLastSynced'}>
-                    ${'test'}
-                </sp-menu-item>
-            </sp-picker>
-
-            <sp-field-label for="picker2">Test2</sp-field-label>
-
-            <sp-picker id="picker2" focusable .label=${'test'}>
-                <sp-menu-item value=${'timeSinceLastSynced'}>
-                    ${'test'}
-                </sp-menu-item>
-                <sp-menu-item value=${'timeSinceLastSynced'}>
-                    ${'test'}
-                </sp-menu-item>
-                <sp-menu-item value=${'timeSinceLastSynced'}>
-                    ${'test'}
-                </sp-menu-item>
-                <sp-menu-item value=${'timeSinceLastSynced'}>
-                    ${'test'}
-                </sp-menu-item>
-                <sp-menu-item value=${'timeSinceLastSynced'}>
-                    ${'test'}
-                </sp-menu-item>
-            </sp-picker>
-
-            <sp-field-label for="picker3">Test 3</sp-field-label>
-
-            <sp-picker id="picker3" focusable .label=${'test'}>
-                <sp-menu-item value=${'timeSinceLastSynced'}>
-                    ${'test'}
-                </sp-menu-item>
-                <sp-menu-item value=${'timeSinceLastSynced'}>
-                    ${'test'}
-                </sp-menu-item>
-                <sp-menu-item value=${'timeSinceLastSynced'}>
-                    ${'test'}
-                </sp-menu-item>
-                <sp-menu-item value=${'timeSinceLastSynced'}>
-                    ${'test'}
-                </sp-menu-item>
-                <sp-menu-item value=${'timeSinceLastSynced'}>
-                    ${'test'}
-                </sp-menu-item>
-            </sp-picker>
+            <overlay-trigger>
+                <sp-button slot="trigger">Open me</sp-button>
+                <sp-popover slot="click-content" direction="bottom" dialog>
+                    <p>This is all the content.</p>
+                    <p>This is all the content.</p>
+                    <p>This is all the content.</p>
+                    <p>This is all the content.</p>
+                </sp-popover>
+            </overlay-trigger>
         `;
     }
 }
 
 customElements.define('popover-content', PopoverContent);
+
+declare global {
+    interface HTMLElementTagNameMap {
+        'popover-content': PopoverContent;
+    }
+}
diff --git a/packages/overlay/stories/overlay.stories.ts b/packages/overlay/stories/overlay.stories.ts
index 2b69213e53..fb48fab470 100644
--- a/packages/overlay/stories/overlay.stories.ts
+++ b/packages/overlay/stories/overlay.stories.ts
@@ -835,6 +835,65 @@ export const detachedElement = (): TemplateResult => {
     `;
 };
 
+class DefinedOverlayReady extends HTMLElement {
+    ready!: (value: boolean | PromiseLike<boolean>) => void;
+
+    constructor() {
+        super();
+        this.readyPromise = new Promise((res) => {
+            this.ready = res;
+            this.setup();
+        });
+    }
+
+    async setup(): Promise<void> {
+        await nextFrame();
+
+        const overlay = document.querySelector(
+            `overlay-trigger`
+        ) as OverlayTrigger;
+        const button = document.querySelector(
+            `[slot="trigger"]`
+        ) as HTMLButtonElement;
+        overlay.addEventListener('sp-opened', this.handleTriggerOpened);
+        button.click();
+    }
+
+    handleTriggerOpened = async (): Promise<void> => {
+        await nextFrame();
+
+        const popover = document.querySelector('popover-content');
+        if (!popover) {
+            return;
+        }
+        popover.addEventListener('sp-opened', this.handlePopoverOpen);
+        popover.button.click();
+    };
+
+    handlePopoverOpen = async (): Promise<void> => {
+        await nextFrame();
+
+        this.ready(true);
+    };
+
+    private readyPromise: Promise<boolean> = Promise.resolve(false);
+
+    get updateComplete(): Promise<boolean> {
+        return this.readyPromise;
+    }
+}
+
+customElements.define('defined-overlay-ready', DefinedOverlayReady);
+
+const definedOverlayDecorator = (
+    story: () => TemplateResult
+): TemplateResult => {
+    return html`
+        ${story()}
+        <defined-overlay-ready></defined-overlay-ready>
+    `;
+};
+
 export const definedOverlayElement = (): TemplateResult => {
     return html`
         <overlay-trigger placement="bottom" type="modal">
@@ -845,3 +904,5 @@ export const definedOverlayElement = (): TemplateResult => {
         </overlay-trigger>
     `;
 };
+
+definedOverlayElement.decorators = [definedOverlayDecorator];
diff --git a/packages/overlay/test/overlay.test.ts b/packages/overlay/test/overlay.test.ts
index ef4fa92713..6dd7200b7a 100644
--- a/packages/overlay/test/overlay.test.ts
+++ b/packages/overlay/test/overlay.test.ts
@@ -561,7 +561,7 @@ describe('Overlay - type="modal"', () => {
         expect(firstOverlay, 'first overlay').to.not.be.null;
         expect(firstOverlay.isConnected).to.be.true;
         expect(firstHeadline.textContent).to.equal('Menu source: end');
-        const closed = oneEvent(document, 'sp-closed');
+        let closed = oneEvent(document, 'sp-closed');
         opened = oneEvent(document, 'sp-opened');
         // Right click to out of the "context menu" overlay to both close
         // the first overlay and have the event passed to the surfacing page
@@ -594,6 +594,21 @@ describe('Overlay - type="modal"', () => {
         expect(firstOverlay.isConnected).to.be.false;
         expect(secondOverlay.isConnected).to.be.true;
         expect(secondHeadline.textContent).to.equal('Menu source: start');
+        closed = oneEvent(document, 'sp-closed');
+        executeServerCommand('send-mouse', {
+            steps: [
+                {
+                    type: 'move',
+                    position: [width / 8, height / 8],
+                },
+                {
+                    type: 'click',
+                    position: [width / 8, height / 8],
+                },
+            ],
+        });
+        await closed;
+        await nextFrame();
     });
     it('opens children in the modal stack through shadow roots', async () => {
         const el = await fixture<OverlayTrigger>(definedOverlayElement());
@@ -607,14 +622,14 @@ describe('Overlay - type="modal"', () => {
             'popover-content'
         ) as PopoverContent;
         open = oneEvent(content, 'sp-opened');
-        content.picker.click();
+        content.button.click();
         await open;
         const activeOverlays = document.querySelectorAll('active-overlay');
         activeOverlays.forEach((overlay) => {
             expect(overlay.slot).to.equal('open');
         });
         let close = oneEvent(content, 'sp-closed');
-        content.picker.open = false;
+        content.trigger.removeAttribute('open');
         await close;
         close = oneEvent(el, 'sp-closed');
         el.removeAttribute('open');
diff --git a/packages/picker/src/Picker.ts b/packages/picker/src/Picker.ts
index e18b579dcb..b42cc51e78 100644
--- a/packages/picker/src/Picker.ts
+++ b/packages/picker/src/Picker.ts
@@ -431,19 +431,34 @@ export class PickerBase extends SizedMixin(Focusable) {
         `;
     }
 
+    protected get dismissHelper(): TemplateResult {
+        return html`
+            <div class="visually-hidden">
+                <button
+                    tabindex="-1"
+                    arial-label="Dismiss"
+                    @click=${this.close}
+                ></button>
+            </div>
+        `;
+    }
+
     protected get renderPopover(): TemplateResult {
         return html`
             <sp-popover
                 id="popover"
+                role="dialog"
                 @sp-overlay-closed=${this.onOverlayClosed}
                 .overlayCloseCallback=${this.overlayCloseCallback}
             >
+                ${this.dismissHelper}
                 <sp-menu
                     id="menu"
                     role="${this.listRole}"
                     selects="single"
                     @change=${this.handleChange}
                 ></sp-menu>
+                ${this.dismissHelper}
             </sp-popover>
         `;
     }
diff --git a/packages/picker/src/picker.css b/packages/picker/src/picker.css
index b065c28d85..a92b2187fc 100644
--- a/packages/picker/src/picker.css
+++ b/packages/picker/src/picker.css
@@ -77,13 +77,16 @@ sp-popover {
 }
 
 .visually-hidden {
-    clip: rect(0 0 0 0);
+    border: 0;
+    clip: rect(0, 0, 0, 0);
     clip-path: inset(50%);
     height: 1px;
+    margin: 0 -1px -1px 0;
     overflow: hidden;
+    padding: 0;
     position: absolute;
-    white-space: nowrap;
     width: 1px;
+    white-space: nowrap;
 }
 
 :host([dir='ltr']) #label.visually-hidden + .picker {
diff --git a/packages/popover/src/popover.css b/packages/popover/src/popover.css
index 661956228b..0660cd6485 100644
--- a/packages/popover/src/popover.css
+++ b/packages/popover/src/popover.css
@@ -68,3 +68,16 @@ governing permissions and limitations under the License.
     transform-origin: 0% 0%;
     transform: rotate(90deg);
 }
+
+::slotted(.visually-hidden) {
+    border: 0;
+    clip: rect(0, 0, 0, 0);
+    clip-path: inset(50%);
+    height: 1px;
+    margin: 0 -1px -1px 0;
+    overflow: hidden;
+    padding: 0;
+    position: absolute;
+    width: 1px;
+    white-space: nowrap;
+}