From a7dae7c318fd791ca1a0095487fe6ced64b51023 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dan=20B=C3=BCschlen?= Date: Mon, 27 May 2024 17:33:57 +0200 Subject: [PATCH] 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 --- src/components/breadcrumb/Breadcrumb.js | 7 +- .../dropdown/stories/dropdown.stories.js | 10 +-- src/components/dropdown/test/dropdown.test.js | 10 +-- src/components/icon/Icon.js | 9 +- src/components/icon/icon.css | 3 +- src/components/icon/stories/icon.stories.js | 2 + src/components/menu/MenuItem.js | 40 +-------- .../menu/stories/menu-item.stories.js | 19 +++- src/components/menu/stories/menu.stories.js | 23 +++-- src/components/menu/test/menu-item.test.js | 89 ++----------------- src/components/menu/test/menu.test.js | 25 ++++-- src/components/select/Select.js | 16 ++-- 12 files changed, 93 insertions(+), 160 deletions(-) diff --git a/src/components/breadcrumb/Breadcrumb.js b/src/components/breadcrumb/Breadcrumb.js index 2fabb773..01e5020f 100644 --- 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` - + ${item.label} ` )} diff --git a/src/components/dropdown/stories/dropdown.stories.js b/src/components/dropdown/stories/dropdown.stories.js index 06c2f2fb..28ab4f63 100644 --- 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` - - + Als CSV Tabelle + Als XLS Tabelle
- - - + Als PNG exportieren + Als SVG exportieren + Als PDF exportieren ` } diff --git a/src/components/dropdown/test/dropdown.test.js b/src/components/dropdown/test/dropdown.test.js index 3d4c8b55..c4b5677f 100644 --- 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` - - + Als CSV Tabelle + Als XLS Tabelle
- - - + Als PNG exportieren + Als SVG exportieren + Als PDF exportieren `) } diff --git a/src/components/icon/Icon.js b/src/components/icon/Icon.js index 029ae1bf..f366f257 100644 --- 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`
` + } + const iconPath = paths[this.name] return html` diff --git a/src/components/icon/icon.css b/src/components/icon/icon.css index 1a8432f9..a9963f17 100644 --- 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 index a3abdf83..56818ef6 100644 --- 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 index 6598dc75..d1bebc8f 100644 --- 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`` - } - - if (name === "EMPTY") { - return html`
` - } - - return name - } - - renderBefore() { - if (this.before) { - const content = LeuMenuItem.getIconOrText(this.before) - return html`${content}` - } - - return nothing - } - - renderAfter() { - if (this.after) { - const content = LeuMenuItem.getIconOrText(this.after) - return html`${content}` - } - - 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()}${this.label}${this.renderAfter()} + + + ` /* 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 index 88ffd593..cde44096 100644 --- 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` + > + ${isIcon(args.before) + ? html`` + : html`${args.before}`} + ${args.label} + ${isIcon(args.after) + ? html`` + : html`${args.after}`} + ` } diff --git a/src/components/menu/stories/menu.stories.js b/src/components/menu/stories/menu.stories.js index 04cf170b..32e57df0 100644 --- 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` - - - + Menu Item 1 + Menu Item + 2 + Menu Item 3
- - + Menu Item 3CH + Menu Item 4 ` } diff --git a/src/components/menu/test/menu-item.test.js b/src/components/menu/test/menu-item.test.js index 1c0f7235..a3769030 100644 --- 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` + > + ${args.label} + `) } @@ -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( - "" - ) - }) - - 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( - "" - ) - }) - - 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( - "" - ) - }) - - 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( - "" - ) - }) - - 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( - "" - ) - }) - - 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( - "" - ) + 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 index 0c97f24a..b444ec0f 100644 --- 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` - - - + Menu Item 1 + Menu Item + 2 + Menu Item 3
- - + Menu Item 3CH + Menu Item 4 `) } @@ -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() }) }) diff --git a/src/components/select/Select.js b/src/components/select/Select.js index ca5b2bf7..e7c0612a 100644 --- a/src/components/select/Select.js +++ b/src/components/select/Select.js @@ -2,7 +2,6 @@ import { html, LitElement, nothing } from "lit" import { classMap } from "lit/directives/class-map.js" import { map } from "lit/directives/map.js" -import { ifDefined } from "lit/directives/if-defined.js" import { createRef, ref } from "lit/directives/ref.js" import { HasSlotController } from "../../lib/hasSlotController.js" @@ -260,21 +259,22 @@ export class LeuSelect extends LitElement { } return html` this.selectOption(option)} role="option" - label=${LeuSelect.getOptionLabel(option)} ?active=${isSelected} aria-selected=${isSelected} > + ${beforeIcon !== undefined + ? html`` + : nothing} + ${LeuSelect.getOptionLabel(option)} ` }) - : html`${this.optionFilter === "" ? "Keine Optionen" - : "Keine Resultate"} - disabled - >`} + : "Keine Resultate"}`} ` }