From 783b2067be23bf82eb222a361e5c899d89f05005 Mon Sep 17 00:00:00 2001
From: Westbrook Johnson <westbrook.johnson@gmail.com>
Date: Tue, 8 Aug 2023 18:25:03 -0400
Subject: [PATCH] fix(action-group): separate first selection management from
 later selection management

---
 packages/action-group/src/ActionGroup.ts      |  50 +++--
 .../stories/action-group-tooltip.stories.ts   | 196 +++++++++++++++++-
 .../stories/action-group.stories.ts           |   1 +
 .../action-group/test/action-group.test.ts    |  84 +++++++-
 4 files changed, 312 insertions(+), 19 deletions(-)

diff --git a/packages/action-group/src/ActionGroup.ts b/packages/action-group/src/ActionGroup.ts
index 027d16e2b1..4ff2d4f52c 100644
--- a/packages/action-group/src/ActionGroup.ts
+++ b/packages/action-group/src/ActionGroup.ts
@@ -18,7 +18,10 @@ import {
     SpectrumElement,
     TemplateResult,
 } from '@spectrum-web-components/base';
-import { property } from '@spectrum-web-components/base/src/decorators.js';
+import {
+    property,
+    query,
+} from '@spectrum-web-components/base/src/decorators.js';
 import type { ActionButton } from '@spectrum-web-components/action-button';
 import { RovingTabindexController } from '@spectrum-web-components/reactive-controllers/src/RovingTabindex.js';
 import { MutationController } from '@lit-labs/observers/mutation-controller.js';
@@ -40,9 +43,10 @@ export class ActionGroup extends SizedMixin(SpectrumElement, {
         return [styles];
     }
 
-    public set buttons(tabs: ActionButton[]) {
-        if (tabs === this.buttons) return;
-        this._buttons = tabs;
+    public set buttons(buttons: ActionButton[]) {
+        /* c8 ignore next 1 */
+        if (buttons === this.buttons) return;
+        this._buttons = buttons;
         this.rovingTabindexController.clearElementCache();
     }
 
@@ -65,6 +69,7 @@ export class ActionGroup extends SizedMixin(SpectrumElement, {
             callback: () => {
                 this.manageButtons();
             },
+            skipInitial: true,
         });
     }
 
@@ -128,6 +133,9 @@ export class ActionGroup extends SizedMixin(SpectrumElement, {
         return this._selected;
     }
 
+    @query('slot')
+    slotElement!: HTMLSlotElement;
+
     private dispatchChange(old: string[]): void {
         const applyDefault = this.dispatchEvent(
             new Event('change', {
@@ -146,6 +154,7 @@ export class ActionGroup extends SizedMixin(SpectrumElement, {
     }
 
     private setSelected(selected: string[], announce?: boolean): void {
+        /* c8 ignore next 1 */
         if (selected === this.selected) return;
 
         const old = this.selected;
@@ -167,7 +176,7 @@ export class ActionGroup extends SizedMixin(SpectrumElement, {
             el.selected = false;
             el.tabIndex = -1;
             el.setAttribute(
-                !this.selects ? 'aria-pressed' : 'aria-checked',
+                this.selects ? 'aria-checked' : /* c8 ignore */ 'aria-pressed',
                 'false'
             );
         });
@@ -363,17 +372,21 @@ export class ActionGroup extends SizedMixin(SpectrumElement, {
             if (this.static || changes?.get('static')) {
                 button.static = this.static;
             }
-            button.selected = this.selected.includes(button.value);
+            if (this.selects || !this.hasManaged) {
+                button.selected = this.selected.includes(button.value);
+            }
             if (this.size) {
                 button.size = this.size;
             }
         });
     }
 
+    private hasManaged = false;
+
     private manageButtons = (): void => {
-        const slot = this.shadowRoot.querySelector('slot');
-        if (!slot) return;
-        const assignedElements = slot.assignedElements({ flatten: true });
+        const assignedElements = this.slotElement.assignedElements({
+            flatten: true,
+        });
         const buttons = assignedElements.reduce((acc: unknown[], el) => {
             if (el.matches(this._buttonSelector)) {
                 acc.push(el);
@@ -386,15 +399,18 @@ export class ActionGroup extends SizedMixin(SpectrumElement, {
             return acc;
         }, []);
         this.buttons = buttons as ActionButton[];
-        // <selected> element merges selected so following paradigm here
-        const currentlySelectedButtons: string[] = [];
-        this.buttons.forEach((button: ActionButton) => {
-            if (button.selected) {
-                currentlySelectedButtons.push(button.value);
-            }
-        });
-        this.setSelected(this.selected.concat(currentlySelectedButtons));
+        if (this.selects || !this.hasManaged) {
+            // <select> element merges selected so following paradigm here
+            const currentlySelectedButtons: string[] = [];
+            this.buttons.forEach((button: ActionButton) => {
+                if (button.selected) {
+                    currentlySelectedButtons.push(button.value);
+                }
+            });
+            this.setSelected(this.selected.concat(currentlySelectedButtons));
+        }
         this.manageChildren();
         this.manageSelects();
+        this.hasManaged = true;
     };
 }
diff --git a/packages/action-group/stories/action-group-tooltip.stories.ts b/packages/action-group/stories/action-group-tooltip.stories.ts
index 4d2df7aade..625b054bb3 100644
--- a/packages/action-group/stories/action-group-tooltip.stories.ts
+++ b/packages/action-group/stories/action-group-tooltip.stories.ts
@@ -10,7 +10,12 @@ OF ANY KIND, either express or implied. See the License for the specific languag
 governing permissions and limitations under the License.
 */
 
-import { html, TemplateResult } from '@spectrum-web-components/base';
+import {
+    html,
+    SpectrumElement,
+    TemplateResult,
+} from '@spectrum-web-components/base';
+import { state } from '@spectrum-web-components/base/src/decorators.js';
 import { spreadProps } from '../../../test/lit-helpers.js';
 
 import '@spectrum-web-components/action-group/sp-action-group.js';
@@ -217,3 +222,192 @@ vertical.args = {
     vertical: true,
     selects: undefined,
 };
+
+class ActionGroupTooltips extends SpectrumElement {
+    @state()
+    alignment = 'left';
+
+    protected override render(): TemplateResult {
+        return html`
+            <sp-action-group quiet>
+                <sp-action-button
+                    quiet
+                    value="left"
+                    ?selected=${this.alignment === 'left'}
+                    @click=${() => {
+                        this.alignment = 'left';
+                    }}
+                >
+                    <sp-icon slot="icon">
+                        <svg
+                            role="img"
+                            fill="currentColor"
+                            viewBox="0 0 18 18"
+                            id="STextAlignLeft18N-icon"
+                            width="18"
+                            height="18"
+                            aria-hidden="true"
+                            focusable="false"
+                        >
+                            <rect
+                                fill-rule="evenodd"
+                                x="2"
+                                y="14"
+                                width="12"
+                                height="2"
+                                rx="0.5"
+                            ></rect>
+                            <rect
+                                fill-rule="evenodd"
+                                x="2"
+                                y="2"
+                                width="15"
+                                height="2"
+                                rx="0.5"
+                            ></rect>
+                            <rect
+                                fill-rule="evenodd"
+                                x="2"
+                                y="6"
+                                width="12"
+                                height="2"
+                                rx="0.5"
+                            ></rect>
+                            <rect
+                                fill-rule="evenodd"
+                                x="2"
+                                y="10"
+                                width="15"
+                                height="2"
+                                rx="0.5"
+                            ></rect>
+                        </svg>
+                    </sp-icon>
+                    <sp-tooltip self-managed placement="bottom" offset="0">
+                        Left align
+                    </sp-tooltip>
+                </sp-action-button>
+                <sp-action-button
+                    quiet
+                    value="center"
+                    ?selected=${this.alignment === 'center'}
+                    @click=${() => {
+                        this.alignment = 'center';
+                    }}
+                >
+                    <sp-icon slot="icon">
+                        <svg
+                            role="img"
+                            fill="currentColor"
+                            viewBox="0 0 18 18"
+                            id="STextAlignCenter18N-icon"
+                            width="18"
+                            height="18"
+                            aria-hidden="true"
+                            focusable="false"
+                        >
+                            <rect
+                                fill-rule="evenodd"
+                                x="4"
+                                y="14"
+                                width="10"
+                                height="2"
+                                rx="0.5"
+                            ></rect>
+                            <rect
+                                fill-rule="evenodd"
+                                x="1"
+                                y="10"
+                                width="16"
+                                height="2"
+                                rx="0.5"
+                            ></rect>
+                            <rect
+                                fill-rule="evenodd"
+                                x="1"
+                                y="2"
+                                width="16"
+                                height="2"
+                                rx="0.5"
+                            ></rect>
+                            <rect
+                                fill-rule="evenodd"
+                                x="4"
+                                y="6"
+                                width="10"
+                                height="2"
+                                rx="0.5"
+                            ></rect>
+                        </svg>
+                    </sp-icon>
+                    <sp-tooltip self-managed placement="bottom" offset="0">
+                        Center align
+                    </sp-tooltip>
+                </sp-action-button>
+                <sp-action-button
+                    quiet
+                    value="right"
+                    ?selected=${this.alignment === 'right'}
+                    @click=${() => {
+                        this.alignment = 'right';
+                    }}
+                >
+                    <sp-icon slot="icon">
+                        <svg
+                            role="img"
+                            fill="currentColor"
+                            viewBox="0 0 18 18"
+                            id="STextAlignRight18N-icon"
+                            width="18"
+                            height="18"
+                            aria-hidden="true"
+                            focusable="false"
+                        >
+                            <rect
+                                fill-rule="evenodd"
+                                x="4"
+                                y="14"
+                                width="12"
+                                height="2"
+                                rx="0.5"
+                            ></rect>
+                            <rect
+                                fill-rule="evenodd"
+                                x="1"
+                                y="2"
+                                width="15"
+                                height="2"
+                                rx="0.5"
+                            ></rect>
+                            <rect
+                                fill-rule="evenodd"
+                                x="4"
+                                y="6"
+                                width="12"
+                                height="2"
+                                rx="0.5"
+                            ></rect>
+                            <rect
+                                fill-rule="evenodd"
+                                x="1"
+                                y="10"
+                                width="15"
+                                height="2"
+                                rx="0.5"
+                            ></rect>
+                        </svg>
+                    </sp-icon>
+                    <sp-tooltip self-managed placement="bottom" offset="0">
+                        Right align
+                    </sp-tooltip>
+                </sp-action-button>
+            </sp-action-group>
+        `;
+    }
+}
+
+customElements.define('action-group-tooltips', ActionGroupTooltips);
+
+export const controlled = (): TemplateResult => html`
+    <action-group-tooltips></action-group-tooltips>
+`;
diff --git a/packages/action-group/stories/action-group.stories.ts b/packages/action-group/stories/action-group.stories.ts
index d594c01591..8dbe6947a5 100644
--- a/packages/action-group/stories/action-group.stories.ts
+++ b/packages/action-group/stories/action-group.stories.ts
@@ -153,6 +153,7 @@ function displaySelectionState(): void {
         selectedDiv.textContent = `Selected: ${JSON.stringify(group.selected)}`;
     }
 }
+
 export const Default = (args: Properties): TemplateResult =>
     renderButtons(args);
 
diff --git a/packages/action-group/test/action-group.test.ts b/packages/action-group/test/action-group.test.ts
index 65203b1dd0..6ab27814e6 100644
--- a/packages/action-group/test/action-group.test.ts
+++ b/packages/action-group/test/action-group.test.ts
@@ -11,6 +11,7 @@ governing permissions and limitations under the License.
 */
 
 import {
+    aTimeout,
     elementUpdated,
     expect,
     fixture,
@@ -20,7 +21,11 @@ import {
 
 import { ActionButton } from '@spectrum-web-components/action-button';
 import '@spectrum-web-components/action-button/sp-action-button.js';
-import { LitElement, TemplateResult } from '@spectrum-web-components/base';
+import {
+    LitElement,
+    SpectrumElement,
+    TemplateResult,
+} from '@spectrum-web-components/base';
 import '@spectrum-web-components/overlay/overlay-trigger.js';
 import '@spectrum-web-components/tooltip/sp-tooltip.js';
 import { ActionGroup } from '@spectrum-web-components/action-group';
@@ -35,6 +40,9 @@ import {
 } from '../../../test/testing-helpers';
 import { sendKeys } from '@web/test-runner-commands';
 import '@spectrum-web-components/action-group/sp-action-group.js';
+import { controlled } from '../stories/action-group-tooltip.stories.js';
+import { spy } from 'sinon';
+import { sendMouse } from '../../../test/plugins/browser.js';
 
 class QuietActionGroup extends LitElement {
     protected override render(): TemplateResult {
@@ -146,6 +154,49 @@ describe('ActionGroup', () => {
         expect(el.getAttribute('role')).to.equal('toolbar');
         expect(el.children[0].getAttribute('role')).to.equal('button');
     });
+    it('applies `static` attribute to its children', async () => {
+        const el = await fixture<ActionGroup>(
+            html`
+                <sp-action-group static="white">
+                    <sp-action-button id="first">First</sp-action-button>
+                    <sp-action-button id="second">Second</sp-action-button>
+                </sp-action-group>
+            `
+        );
+        const firstButton = el.querySelector('#first') as ActionButton;
+        const secondButton = el.querySelector('#second') as ActionButton;
+
+        await elementUpdated(el);
+
+        expect(firstButton.static).to.equal('white');
+        expect(secondButton.static).to.equal('white');
+
+        el.static = undefined;
+
+        await elementUpdated(el);
+
+        expect(firstButton.static).to.be.undefined;
+        expect(secondButton.static).to.be.undefined;
+    });
+    it('manages "label"', async () => {
+        const testLabel = 'Testable action group';
+        const el = await fixture<ActionGroup>(
+            html`
+                <sp-action-group label=${testLabel}>
+                    <sp-action-button id="first">First</sp-action-button>
+                    <sp-action-button id="second">Second</sp-action-button>
+                </sp-action-group>
+            `
+        );
+
+        expect(el.getAttribute('aria-label')).to.equal(testLabel);
+
+        el.label = '';
+
+        await elementUpdated(el);
+
+        expect(el.hasAttribute('aria-label')).to.be.false;
+    });
     it('applies `quiet` attribute to its children', async () => {
         const el = await fixture<ActionGroup>(
             html`
@@ -1175,4 +1226,35 @@ describe('ActionGroup', () => {
         expect(el.selected.length).to.equal(1);
         expect(el.selected[0]).to.equal('Third');
     });
+    it('processes `selects` correctly when mutations occur (because Overlays/Tooltips)', async () => {
+        const test = await fixture<SpectrumElement>(controlled());
+        const actionButtons = [
+            ...test.shadowRoot.querySelectorAll('sp-action-button'),
+        ] as ActionButton[];
+
+        expect(actionButtons[0].selected).to.be.true;
+        expect(actionButtons[1].selected).to.be.false;
+        expect(actionButtons[2].selected).to.be.false;
+
+        const changeSpy = spy();
+        test.addEventListener('change', () => changeSpy());
+        const rect = actionButtons[1].getBoundingClientRect();
+        sendMouse({
+            steps: [
+                {
+                    position: [
+                        rect.left + rect.width / 2,
+                        rect.top + rect.height / 2,
+                    ],
+                    type: 'click',
+                },
+            ],
+        });
+
+        await aTimeout(500);
+
+        expect(actionButtons[0].selected).to.be.false;
+        expect(actionButtons[1].selected).to.be.true;
+        expect(actionButtons[2].selected).to.be.false;
+    });
 });