refactor(menu-item): use slots instead of attributes to display icons…

… and the label feat(icon): render a placeholder if the name of the icon is unknown or not set
statistikZH · May 27, 2024 · a7dae7c · a7dae7c
1 parent d4edc51
commit a7dae7c
Show file tree

Hide file tree

Showing 12 changed files with 93 additions and 160 deletions.
diff --git a/src/components/breadcrumb/Breadcrumb.js b/src/components/breadcrumb/Breadcrumb.js
@@ -250,10 +250,9 @@ export class LeuBreadcrumb extends LitElement {
                 ${this._dropdownItems.map(
                   (item) =>
                     html`
-                      <leu-menu-item
-                        label=${item.label}
-                        href=${item.href}
-                      ></leu-menu-item>
+                      <leu-menu-item href=${item.href}
+                        >${item.label}</leu-menu-item
+                      >
                     `
                 )}
               </leu-menu>

diff --git a/src/components/dropdown/stories/dropdown.stories.js b/src/components/dropdown/stories/dropdown.stories.js
@@ -15,12 +15,12 @@ export default {
 function Template({ label, expanded }) {
   return html` <leu-dropdown label=${label} ?expanded=${expanded}>
     <leu-menu>
-      <leu-menu-item label="Als CSV Tabelle"></leu-menu-item>
-      <leu-menu-item label="Als XLS Tabelle"></leu-menu-item>
+      <leu-menu-item>Als CSV Tabelle</leu-menu-item>
+      <leu-menu-item>Als XLS Tabelle</leu-menu-item>
       <hr />
-      <leu-menu-item label="Als PNG exportieren"></leu-menu-item>
-      <leu-menu-item label="Als SVG exportieren"></leu-menu-item>
-      <leu-menu-item label="Als PDF exportieren" disabled></leu-menu-item>
+      <leu-menu-item>Als PNG exportieren</leu-menu-item>
+      <leu-menu-item>Als SVG exportieren</leu-menu-item>
+      <leu-menu-item disabled>Als PDF exportieren</leu-menu-item>
     </leu-menu>
   </leu-dropdown>`
 }

diff --git a/src/components/dropdown/test/dropdown.test.js b/src/components/dropdown/test/dropdown.test.js
@@ -6,12 +6,12 @@ import "../leu-dropdown.js"
 async function defaultFixture() {
   return fixture(html` <leu-dropdown label="Download">
     <leu-menu>
-      <leu-menu-item label="Als CSV Tabelle"></leu-menu-item>
-      <leu-menu-item label="Als XLS Tabelle"></leu-menu-item>
+      <leu-menu-item>Als CSV Tabelle</leu-menu-item>
+      <leu-menu-item>Als XLS Tabelle</leu-menu-item>
       <hr />
-      <leu-menu-item label="Als PNG exportieren"></leu-menu-item>
-      <leu-menu-item label="Als SVG exportieren"></leu-menu-item>
-      <leu-menu-item label="Als PDF exportieren"></leu-menu-item>
+      <leu-menu-item>Als PNG exportieren</leu-menu-item>
+      <leu-menu-item>Als SVG exportieren</leu-menu-item>
+      <leu-menu-item>Als PDF exportieren</leu-menu-item>
     </leu-menu>
   </leu-dropdown>`)
 }

diff --git a/src/components/icon/Icon.js b/src/components/icon/Icon.js
@@ -6,6 +6,7 @@ import { paths } from "./paths.js"
  * A component to render all defined zhWeb icons.
  * The `fill` of the icon is set to `currentColor` and
  * can be overriden by setting the css `color` property.
+ * If the icon name is not found, a placeholder will be displayed.
  *
  * @tagname leu-icon
  * @prop {import("./paths").IconPathName} name - The name of the icon to display.
@@ -22,12 +23,16 @@ export class LeuIcon extends LitElement {
     super()
 
     /**
-     * @type {import("./paths").IconPathName}
+     * @type {import("./paths").IconPathName | ""}
      */
-    this.name = "addNew"
+    this.name = ""
   }
 
   render() {
+    if (!paths[this.name]) {
+      return html`<div class="placeholder"></div>`
+    }
+
     const iconPath = paths[this.name]
 
     return html`

diff --git a/src/components/icon/icon.css b/src/components/icon/icon.css
@@ -1,4 +1,5 @@
-svg {
+svg,
+.placeholder {
   display: block;
   width: var(--leu-icon-size, 1.5rem);
   height: var(--leu-icon-size, 1.5rem);

diff --git a/src/components/icon/stories/icon.stories.js b/src/components/icon/stories/icon.stories.js
@@ -30,11 +30,13 @@ function Template({ name, size, color }) {
 export const Regular = Template.bind({})
 Regular.args = {
   size: 24,
+  name: "addNew",
 }
 
 export const Small = Template.bind({})
 Small.args = {
   size: 16,
+  name: "check",
 }
 
 export const Colored = Template.bind({})

diff --git a/src/components/menu/MenuItem.js b/src/components/menu/MenuItem.js
@@ -1,13 +1,10 @@
-import { LitElement, nothing } from "lit"
+import { LitElement } from "lit"
 import { html, unsafeStatic } from "lit/static-html.js"
 import { ifDefined } from "lit/directives/if-defined.js"
 
 import styles from "./menu-item.css"
 
 import "../icon/leu-icon.js"
-import { paths as iconPaths } from "../icon/paths.js"
-
-const ICON_NAMES = Object.keys(iconPaths)
 
 /**
  * @tagname leu-menu-item
@@ -57,36 +54,6 @@ export class LeuMenuItem extends LitElement {
     this.highlighted = false
   }
 
-  static getIconOrText(name) {
-    if (ICON_NAMES.includes(name)) {
-      return html`<leu-icon name=${name}></leu-icon>`
-    }
-
-    if (name === "EMPTY") {
-      return html`<div class="icon-placeholder"></div>`
-    }
-
-    return name
-  }
-
-  renderBefore() {
-    if (this.before) {
-      const content = LeuMenuItem.getIconOrText(this.before)
-      return html`<span class="before">${content}</span>`
-    }
-
-    return nothing
-  }
-
-  renderAfter() {
-    if (this.after) {
-      const content = LeuMenuItem.getIconOrText(this.after)
-      return html`<span class="after">${content}</span>`
-    }
-
-    return nothing
-  }
-
   getTagName() {
     return this.href ? "a" : "button"
   }
@@ -97,8 +64,9 @@ export class LeuMenuItem extends LitElement {
     return html`<${unsafeStatic(
       this.getTagName()
     )} class="button" href=${ifDefined(this.href)} ?disabled=${this.disabled}>
-      ${this.renderBefore()}<span class="label">${this.label}</span
-      >${this.renderAfter()}
+      <slot class="before" name="before"></slot>
+      <span class="label"><slot></slot></span>
+      <slot class="after" name="after"></slot>
     </${unsafeStatic(this.getTagName())}>`
     /* eslint-enable lit/binding-positions, lit/no-invalid-html */
   }

diff --git a/src/components/menu/stories/menu-item.stories.js b/src/components/menu/stories/menu-item.stories.js
@@ -2,6 +2,12 @@ import { html } from "lit"
 import { ifDefined } from "lit/directives/if-defined.js"
 
 import "../leu-menu-item.js"
+import "../../icon/leu-icon.js"
+import { paths as iconPaths } from "../../icon/paths.js"
+
+function isIcon(name) {
+  return name === "EMPTY" || Object.keys(iconPaths).includes(name)
+}
 
 export default {
   title: "Menu/Item",
@@ -20,13 +26,18 @@ export default {
 function Template(args) {
   return html`
     <leu-menu-item
-      label=${args.label}
-      before=${ifDefined(args.before)}
-      after=${ifDefined(args.after)}
       href=${ifDefined(args.href)}
       ?active=${args.active}
       ?disabled=${args.disabled}
-    ></leu-menu-item>
+    >
+      ${isIcon(args.before)
+        ? html`<leu-icon slot="before" name=${args.before}></leu-icon>`
+        : html`<span slot="before">${args.before}</span>`}
+      ${args.label}
+      ${isIcon(args.after)
+        ? html`<leu-icon slot="after" name=${args.after}></leu-icon>`
+        : html`<span slot="after">${args.after}</span>`}
+    </leu-menu-item>
   `
 }
 

diff --git a/src/components/menu/stories/menu.stories.js b/src/components/menu/stories/menu.stories.js
@@ -1,6 +1,7 @@
 import { html } from "lit"
 import "../leu-menu.js"
 import "../leu-menu-item.js"
+import "../../icon/leu-icon.js"
 
 export default {
   title: "Menu",
@@ -15,12 +16,24 @@ export default {
 
 function Template() {
   return html` <leu-menu>
-    <leu-menu-item label="Menu Item 1" before="EMPTY"></leu-menu-item>
-    <leu-menu-item label="Menu Item 2" before="check" active></leu-menu-item>
-    <leu-menu-item label="Menu Item 3" before="EMPTY"></leu-menu-item>
+    <leu-menu-item
+      ><leu-icon slot="before"></leu-icon>Menu Item 1</leu-menu-item
+    >
+    <leu-menu-item active
+      ><leu-icon slot="before" name="check"></leu-icon>Menu Item
+      2</leu-menu-item
+    >
+    <leu-menu-item
+      ><leu-icon slot="before"></leu-icon>Menu Item 3</leu-menu-item
+    >
     <hr />
-    <leu-menu-item label="Menu Item 3" before="pin" after="CH"></leu-menu-item>
-    <leu-menu-item label="Menu Item 4"></leu-menu-item>
+    <leu-menu-item
+      ><leu-icon slot="before" name="pin"></leu-icon>Menu Item 3<span
+        slot="after"
+        >CH</span
+      ></leu-menu-item
+    >
+    <leu-menu-item>Menu Item 4</leu-menu-item>
   </leu-menu>`
 }
 

diff --git a/src/components/menu/test/menu-item.test.js b/src/components/menu/test/menu-item.test.js
@@ -8,13 +8,12 @@ import "../leu-menu-item.js"
 async function defaultFixture(args = {}) {
   return fixture(html`
     <leu-menu-item
-      label=${args.label}
-      before=${ifDefined(args.before)}
-      after=${ifDefined(args.after)}
       href=${ifDefined(args.href)}
       ?active=${args.active}
       ?disabled=${args.disabled}
-    ></leu-menu-item>
+    >
+      ${args.label}
+    </leu-menu-item>
   `)
 }
 
@@ -43,9 +42,7 @@ describe("LeuMenuItem", () => {
   it("renders a label", async () => {
     const el = await defaultFixture({ label: "Download" })
 
-    const button = el.shadowRoot.querySelector("button")
-
-    expect(button).to.have.trimmed.text("Download")
+    expect(el).to.have.trimmed.text("Download")
   })
 
   it("renders a button", async () => {
@@ -66,83 +63,7 @@ describe("LeuMenuItem", () => {
 
     expect(link).to.exist
     expect(link).to.have.attribute("href", "https://zh.ch")
-    expect(link).to.have.trimmed.text("Kanton Zürich")
-  })
-
-  it("renders a before icon", async () => {
-    const el = await defaultFixture({ label: "Download", before: "download" })
-
-    const before = el.shadowRoot.querySelector(".before")
-    expect(before).to.exist
-
-    expect(el).shadowDom.to.equal(
-      "<button class='button'><span class='before'></span><span class='label'>Download</span></button>"
-    )
-  })
-
-  it("renders a before label", async () => {
-    const el = await defaultFixture({ label: "Download", before: "DE" })
-
-    const before = el.shadowRoot.querySelector(".before")
-    expect(before).to.exist
-    expect(before).to.have.trimmed.text("DE")
-
-    expect(el).shadowDom.to.equal(
-      "<button class='button'><span class='before'>DE</span><span class='label'>Download</span></button>"
-    )
-  })
-
-  it("renders a before placeholder", async () => {
-    const el = await defaultFixture({ label: "Download", before: "EMPTY" })
-
-    const before = el.shadowRoot.querySelector(".before")
-    expect(before).to.exist
-    expect(before).to.not.have.trimmed.text()
-
-    const iconPlaceholder = before.querySelector(".icon-placeholder")
-    expect(iconPlaceholder).to.exist
-
-    expect(el).shadowDom.to.equal(
-      "<button class='button'><span class='before'><div class='icon-placeholder'></div></span><span class='label'>Download</span></button>"
-    )
-  })
-
-  it("renders a after icon", async () => {
-    const el = await defaultFixture({ label: "Download", after: "download" })
-
-    const after = el.shadowRoot.querySelector(".after")
-    expect(after).to.exist
-
-    expect(el).shadowDom.to.equal(
-      "<button class='button'><span class='label'>Download</span><span class='after'></span></button>"
-    )
-  })
-
-  it("renders a after label", async () => {
-    const el = await defaultFixture({ label: "Download", after: "DE" })
-
-    const after = el.shadowRoot.querySelector(".after")
-    expect(after).to.exist
-    expect(after).to.have.trimmed.text("DE")
-
-    expect(el).shadowDom.to.equal(
-      "<button class='button'><span class='label'>Download</span><span class='after'>DE</span></button>"
-    )
-  })
-
-  it("renders a after placeholder", async () => {
-    const el = await defaultFixture({ label: "Download", after: "EMPTY" })
-
-    const after = el.shadowRoot.querySelector(".after")
-    expect(after).to.exist
-    expect(after).to.not.have.trimmed.text()
-
-    const iconPlaceholder = after.querySelector(".icon-placeholder")
-    expect(iconPlaceholder).to.exist
-
-    expect(el).shadowDom.to.equal(
-      "<button class='button'><span class='label'>Download</span><span class='after'><div class='icon-placeholder'></div></span></button>"
-    )
+    expect(el).to.have.trimmed.text("Kanton Zürich")
   })
 
   it("passes the disabled attribute to the button", async () => {

diff --git a/src/components/menu/test/menu.test.js b/src/components/menu/test/menu.test.js
@@ -3,15 +3,28 @@ import { fixture, expect } from "@open-wc/testing"
 
 import "../leu-menu.js"
 import "../leu-menu-item.js"
+import "../../icon/leu-icon.js"
 
 async function defaultFixture() {
   return fixture(html` <leu-menu>
-    <leu-menu-item label="Menu Item 1" before="EMPTY"></leu-menu-item>
-    <leu-menu-item label="Menu Item 2" before="check" active></leu-menu-item>
-    <leu-menu-item label="Menu Item 3" before="EMPTY"></leu-menu-item>
+    <leu-menu-item
+      ><leu-icon slot="before"></leu-icon>Menu Item 1</leu-menu-item
+    >
+    <leu-menu-item active
+      ><leu-icon slot="before" name="check"></leu-icon>Menu Item
+      2</leu-menu-item
+    >
+    <leu-menu-item
+      ><leu-icon slot="before"></leu-icon>Menu Item 3</leu-menu-item
+    >
     <hr />
-    <leu-menu-item label="Menu Item 3" before="pin" after="CH"></leu-menu-item>
-    <leu-menu-item label="Menu Item 4"></leu-menu-item>
+    <leu-menu-item
+      ><leu-icon name="pin" slot="before"></leu-icon>Menu Item 3<slot
+        name="after"
+        >CH</slot
+      ></leu-menu-item
+    >
+    <leu-menu-item>Menu Item 4</leu-menu-item>
   </leu-menu>`)
 }
 
@@ -25,6 +38,6 @@ describe("LeuMenu", () => {
   it("passes the a11y audit", async () => {
     const el = await defaultFixture()
 
-    await expect(el).shadowDom.to.be.accessible()
+    await expect(el).dom.to.be.accessible()
   })
 })