From 3ca3b9c05bc0d4454c2253a8099ceee6fcdc67cd Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dan=20B=C3=BCschlen?= <danbueschlen@gmail.com>
Date: Thu, 21 Mar 2024 11:22:00 +0100
Subject: [PATCH] refactor(button-group): use slots instead of a list property
 to display buttons

---
 src/components/button-group/ButtonGroup.js    | 106 ++++++++++++------
 .../stories/button-group.stories.js           |  19 +++-
 .../button-group/test/button-group.test.js    |  69 +++++++-----
 3 files changed, 123 insertions(+), 71 deletions(-)

diff --git a/src/components/button-group/ButtonGroup.js b/src/components/button-group/ButtonGroup.js
index 97f917c0..2055fbdb 100644
--- a/src/components/button-group/ButtonGroup.js
+++ b/src/components/button-group/ButtonGroup.js
@@ -1,56 +1,94 @@
 import { html, LitElement } from "lit"
 import styles from "./button-group.css"
-import "../button/leu-button.js"
 
 /**
  * @tagname leu-button-group
+ * @slot - Slot for the buttons
+ * @prop {string} value - The value of the currenty selected (active) button
+ * @fires input - When the value of the group changes by clicking a button
  */
 export class LeuButtonGroup extends LitElement {
   static styles = styles
 
-  static properties = {
-    items: { type: Array, reflect: true },
-    value: { type: String, reflect: true },
-  }
-
   constructor() {
     super()
-    /** @type {Array} */
-    this.items = []
-    /** @type {string} */
-    this.value = null
+
+    this._items = []
+  }
+
+  /**
+   * @param {HTMLElement} button
+   * @returns {string}
+   */
+  static getButtonValue(button) {
+    return button.getAttribute("value") ?? button.innerText.trim()
+  }
+
+  get value() {
+    const activeButton = this._items.find((item) => item.active)
+    return activeButton ? LeuButtonGroup.getButtonValue(activeButton) : null
+  }
+
+  set value(newValue) {
+    this._items.forEach((item) => {
+      /* eslint-disable no-param-reassign */
+      item.active = LeuButtonGroup.getButtonValue(item) === newValue
+      /* eslint-enable no-param-reassign */
+    })
+  }
+
+  _handleSlotChange() {
+    /**
+     * Remove all event listeners that were added before.
+     * Just because a slotchange event was fired, it doesn't mean that all of the
+     * children of the slot have changed.
+     */
+    this._items.forEach((item) => {
+      item.removeEventListener("click", this._handleButtonClick)
+    })
+
+    const slot = this.shadowRoot.querySelector("slot")
+    this._items = slot.assignedElements({ flatten: true })
+
+    let foundActiveButtonBefore = false
+
+    this._items.forEach((item) => {
+      /* eslint-disable no-param-reassign */
+      item.addEventListener("click", () => this._handleButtonClick(item))
+      item.componentRole = "menuitemradio"
+
+      /**
+       * In case there are multiple active buttons
+       * only the first one will be kept active.
+       */
+      if (item.active && foundActiveButtonBefore) {
+        item.active = false
+      } else if (item.active) {
+        foundActiveButtonBefore = true
+      }
+
+      /* eslint-enable no-param-reassign */
+    })
   }
 
-  _setValue(newValue) {
-    this.value = newValue
+  _handleButtonClick(button) {
+    if (!button.active) {
+      this.value = LeuButtonGroup.getButtonValue(button)
 
-    this.dispatchEvent(
-      new CustomEvent("input", {
-        bubbles: true,
-        composed: true,
-        detail: { value: newValue },
-      })
-    )
+      this.dispatchEvent(
+        new CustomEvent("input", {
+          bubbles: true,
+          composed: true,
+          detail: { value: LeuButtonGroup.getButtonValue(button) },
+        })
+      )
+    }
   }
 
   render() {
     return html`
       <div role="menubar" class="group">
-        ${this.items.map(
-          (item) =>
-            html`
-              <leu-button
-                variant="secondary"
-                @click=${() => {
-                  this._setValue(item)
-                }}
-                componentrole="menuitemradio"
-                ?active=${this.value === item}
-              >
-                ${item}
-              </leu-button>
-            `
-        )}
+        <slot @slotchange=${this._handleSlotChange}></slot>
       </div>
     `
   }
diff --git a/src/components/button-group/stories/button-group.stories.js b/src/components/button-group/stories/button-group.stories.js
index d65335cd..9bf33179 100644
--- a/src/components/button-group/stories/button-group.stories.js
+++ b/src/components/button-group/stories/button-group.stories.js
@@ -1,5 +1,6 @@
 import { html } from "lit"
 import "../leu-button-group.js"
+import "../../button/leu-button.js"
 
 // https://stackoverflow.com/questions/72566428/storybook-angular-how-to-dynamically-update-args-from-the-template
 import { UPDATE_STORY_ARGS } from "@storybook/core-events" // eslint-disable-line
@@ -23,21 +24,27 @@ export default {
 }
 
 function Template({ items, value }, { id }) {
-  return html`
-    <leu-button-group
-      .items=${items}
+  return html` <leu-button-group
       .value=${value}
-      @click=${(event) => {
+      @input=${(event) => {
         updateStorybookArgss(id, {
           value: event.target.value,
         })
       }}
     >
+      ${items.map(
+        (i) =>
+          html`<leu-button
+            variant="secondary"
+            ?active=${value === i}
+            value=${`${i}-attr`}
+            >${i}
+          </leu-button>`
+      )}
     </leu-button-group>
     <br />
     <br />
-    <pre>value = '${value}'</pre>
-  `
+    <pre>value = '${value}'</pre>`
 }
 
 export const Regular = Template.bind({})
diff --git a/src/components/button-group/test/button-group.test.js b/src/components/button-group/test/button-group.test.js
index 05c41535..96391852 100644
--- a/src/components/button-group/test/button-group.test.js
+++ b/src/components/button-group/test/button-group.test.js
@@ -1,12 +1,23 @@
 import { html } from "lit"
-import { fixture, expect, oneEvent, elementUpdated } from "@open-wc/testing"
+import {
+  fixture,
+  expect,
+  oneEvent,
+  elementUpdated,
+  aTimeout,
+} from "@open-wc/testing"
 
 import "../leu-button-group.js"
-
-const items = ["Eins", "Zwei", "Drei"]
+import "../../button/leu-button.js"
 
 async function defaultFixture() {
-  return fixture(html` <leu-button-group .items=${items}></leu-button-group> `)
+  return fixture(html`
+    <leu-button-group>
+      <leu-button variant="secondary" value="Eins">Eins</leu-button>
+      <leu-button variant="secondary" value="Zweierlei">Zwei</leu-button>
+      <leu-button variant="secondary">Drei</leu-button>
+    </leu-button-group>
+  `)
 }
 
 describe("LeuButtonGroup", () => {
@@ -19,7 +30,7 @@ describe("LeuButtonGroup", () => {
   it("passes the a11y audit", async () => {
     const el = await defaultFixture()
 
-    await expect(el).shadowDom.to.be.accessible()
+    await expect(el).to.be.accessible()
   })
 
   it("has no value by default", async () => {
@@ -31,32 +42,36 @@ describe("LeuButtonGroup", () => {
   it("has the correct value after clicking a button", async () => {
     const el = await defaultFixture()
 
-    const buttons = el.shadowRoot.querySelectorAll("leu-button")
+    const buttons = Array.from(el.querySelectorAll("leu-button"))
 
-    buttons[1].click()
-    await expect(el.value).to.equal("Zwei")
+    setTimeout(() => buttons[1].click())
+    await oneEvent(el, "input")
+    await expect(el.value).to.equal("Zweierlei")
 
-    buttons[0].click()
+    setTimeout(() => buttons[0].click())
+    await oneEvent(el, "input")
     await expect(el.value).to.equal("Eins")
 
-    buttons[2].click()
+    setTimeout(() => buttons[2].click())
+    await oneEvent(el, "input")
     await expect(el.value).to.equal("Drei")
 
     // Should not change after clicking the same button again
-    buttons[2].click()
+    setTimeout(() => buttons[2].click())
+    await aTimeout(100) // There is no event to wait for so
     await expect(el.value).to.equal("Drei")
   })
 
-  it("renders the active button as a primary button", async () => {
+  it("sets the active attribute on the active button", async () => {
     const el = await defaultFixture()
-    el.value = "Zwei"
+    el.value = "Zweierlei"
     await elementUpdated(el)
 
-    const buttons = el.shadowRoot.querySelectorAll("leu-button")
+    const buttons = el.querySelectorAll("leu-button")
 
-    await expect(buttons[0].variant).to.equal("secondary")
-    await expect(buttons[1].variant).to.equal("primary")
-    await expect(buttons[2].variant).to.equal("secondary")
+    await expect(buttons[0].active).to.be.false
+    await expect(buttons[1].active).to.be.true
+    await expect(buttons[2].active).to.be.false
 
     buttons[0].click()
 
@@ -65,21 +80,13 @@ describe("LeuButtonGroup", () => {
     await expect(buttons[2].variant).to.equal("secondary")
   })
 
-  it("sets the correct aria-checked attribute", async () => {
+  it("sets the menuitemradio role on the buttons", async () => {
     const el = await defaultFixture()
-    el.value = "Drei"
-    await elementUpdated(el)
-
-    const buttons = el.shadowRoot.querySelectorAll("leu-button")
+    const buttons = el.querySelectorAll("leu-button")
 
-    await expect(buttons[0].getAttribute("aria-checked")).to.equal("false")
-    await expect(buttons[1].getAttribute("aria-checked")).to.equal("false")
-    await expect(buttons[2].getAttribute("aria-checked")).to.equal("true")
-
-    buttons[0].click()
-    await expect(buttons[0].getAttribute("aria-checked")).to.equal("false")
-    await expect(buttons[1].getAttribute("aria-checked")).to.equal("false")
-    await expect(buttons[2].getAttribute("aria-checked")).to.equal("false")
+    await expect(buttons[0].componentRole).to.equal("menuitemradio")
+    await expect(buttons[1].componentRole).to.equal("menuitemradio")
+    await expect(buttons[2].componentRole).to.equal("menuitemradio")
   })
 
   it("dispatches an input event when the value changes", async () => {
@@ -87,7 +94,7 @@ describe("LeuButtonGroup", () => {
     el.value = "Drei"
     await elementUpdated(el)
 
-    const buttons = el.shadowRoot.querySelectorAll("leu-button")
+    const buttons = el.querySelectorAll("leu-button")
 
     setTimeout(() => buttons[0].click())