fix: review pt.6 - create common method for icon rendering

sbb-design-systems · Jan 30, 2024 · 8b9d0f9 · 8b9d0f9
1 parent 9d4d702
commit 8b9d0f9
Show file tree

Hide file tree

Showing 21 changed files with 66 additions and 72 deletions.
diff --git a/src/components/button/button-link/button-link.ts b/src/components/button/button-link/button-link.ts
@@ -38,11 +38,7 @@ export class SbbButtonLinkElement extends SbbButtonCommonElementMixin(SbbLinkBas
     /* eslint-disable lit/binding-positions */
     return html`
       <${unsafeStatic(TAG_NAME)} class="sbb-button" ${spread(attributes)}>
-        <span class="sbb-button__icon">
-          <slot name="icon">
-            ${this.iconName ? html`<sbb-icon name="${this.iconName}"></sbb-icon>` : nothing}
-          </slot>
-        </span>
+        ${this.renderIconSlot()}
         <span class="sbb-button__label">
           <slot></slot>
           ${

diff --git a/src/components/button/button-link/readme.md b/src/components/button/button-link/readme.md
@@ -87,7 +87,7 @@ Use the accessibility properties in case of an icon-only button to describe the
 | `size`     | `size`      | public  | `SbbButtonSize \| undefined`            | `'l'`       | Size variant, either l or m.                                                                                                                           |
 | `isStatic` | `is-static` | public  | `boolean`                               | `false`     | Set this property to true if you want only a visual representation of a button, but no interaction (a span instead of a link/button will be rendered). |
 | `negative` | `negative`  | public  | `boolean`                               | `false`     | Negative coloring variant flag.                                                                                                                        |
-| `disabled` | `disabled`  | public  | `boolean \| undefined`                  | `false`     | Whether the button is disabled.                                                                                                                        |
+| `disabled` | `disabled`  | public  | `boolean`                               | `false`     | Whether the button is disabled.                                                                                                                        |
 | `iconName` | `icon-name` | public  | `string \| undefined`                   |             | The icon name we want to use, choose from the small icon variants from the ui-icons category from here https://icons.app.sbb.ch.                       |
 | `href`     | `href`      | public  | `string \| undefined`                   |             | The href value you want to link to.                                                                                                                    |
 | `target`   | `target`    | public  | `LinkTargetType \| string \| undefined` |             | Where to display the linked URL.                                                                                                                       |

diff --git a/src/components/button/button/button.ts b/src/components/button/button/button.ts
@@ -1,4 +1,4 @@
-import { nothing, type TemplateResult } from 'lit';
+import type { TemplateResult } from 'lit';
 import { customElement } from 'lit/decorators.js';
 import { html } from 'lit/static-html.js';
 
@@ -24,12 +24,7 @@ export class SbbButtonElement extends SbbButtonCommonElementMixin(SbbButtonBaseE
 
     return html`
       <span class="sbb-button">
-        <span class="sbb-button__icon">
-          <slot name="icon">
-            ${this.iconName ? html`<sbb-icon name="${this.iconName}"></sbb-icon>` : nothing}
-          </slot>
-        </span>
-
+        ${this.renderIconSlot()}
         <span class="sbb-button__label">
           <slot></slot>
         </span>

diff --git a/src/components/button/button/readme.md b/src/components/button/button/readme.md
@@ -80,7 +80,7 @@ Use the accessibility properties in case of an icon-only button to describe the
 | `size`     | `size`      | public  | `SbbButtonSize \| undefined` | `'l'`       | Size variant, either l or m.                                                                                                                           |
 | `isStatic` | `is-static` | public  | `boolean`                    | `false`     | Set this property to true if you want only a visual representation of a button, but no interaction (a span instead of a link/button will be rendered). |
 | `negative` | `negative`  | public  | `boolean`                    | `false`     | Negative coloring variant flag.                                                                                                                        |
-| `disabled` | `disabled`  | public  | `boolean \| undefined`       | `false`     | Whether the button is disabled.                                                                                                                        |
+| `disabled` | `disabled`  | public  | `boolean`                    | `false`     | Whether the button is disabled.                                                                                                                        |
 | `iconName` | `icon-name` | public  | `string \| undefined`        |             | The icon name we want to use, choose from the small icon variants from the ui-icons category from here https://icons.app.sbb.ch.                       |
 | `type`     | `type`      | public  | `ButtonType \| undefined`    |             | The type attribute to use for the button.                                                                                                              |
 | `name`     | `name`      | public  | `string \| undefined`        |             | The name attribute to use for the button.                                                                                                              |

diff --git a/src/components/button/common/button-common.ts b/src/components/button/common/button-common.ts
@@ -1,11 +1,11 @@
-import type { CSSResultGroup, LitElement } from 'lit';
+import { type CSSResultGroup, html, type LitElement, type TemplateResult } from 'lit';
 import { property } from 'lit/decorators.js';
 
-import type {
-  AbstractConstructor,
-  SbbDisabledMixinType,
-  SbbIconNameMixinType,
-  SbbNegativeMixinType,
+import {
+  type AbstractConstructor,
+  type SbbDisabledMixinType,
+  type SbbIconNameMixinType,
+  type SbbNegativeMixinType,
 } from '../../core/common-behaviors';
 import {
   NamedSlotStateController,
@@ -33,6 +33,7 @@ export declare class SbbButtonCommonElementMixinType
   public disabled: boolean;
   public iconName: string;
   public negative: boolean;
+  public renderIconSlot: () => TemplateResult;
 }
 
 // eslint-disable-next-line @typescript-eslint/naming-convention
@@ -80,6 +81,10 @@ export const SbbButtonCommonElementMixin = <T extends AbstractConstructor<LitEle
       super.disconnectedCallback();
       this._handlerRepository.disconnect();
     }
+
+    public override renderIconSlot(): TemplateResult {
+      return html` <span class="sbb-button__icon"> ${super.renderIconSlot()} </span> `;
+    }
   }
   return SbbButtonCommonElement as unknown as AbstractConstructor<SbbButtonCommonElementMixinType> &
     T;

diff --git a/src/components/core/common-behaviors/disabled-mixin.ts b/src/components/core/common-behaviors/disabled-mixin.ts
@@ -11,9 +11,9 @@ export declare class SbbDisabledMixinType {
 export const SbbDisabledMixin = <T extends AbstractConstructor<LitElement>>(
   superClass: T,
 ): AbstractConstructor<SbbDisabledMixinType> & T => {
-  abstract class SbbDisabled extends superClass implements Partial<SbbDisabledMixinType> {
+  abstract class SbbDisabled extends superClass implements SbbDisabledMixinType {
     /** Whether the button is disabled. */
-    @property({ reflect: true, type: Boolean }) public disabled?: boolean = false;
+    @property({ reflect: true, type: Boolean }) public disabled: boolean = false;
   }
 
   return SbbDisabled as AbstractConstructor<SbbDisabledMixinType> & T;

diff --git a/src/components/core/common-behaviors/icon-name-mixin.ts b/src/components/core/common-behaviors/icon-name-mixin.ts
@@ -1,23 +1,35 @@
-import type { LitElement } from 'lit';
+import { html, type LitElement, nothing, type TemplateResult } from 'lit';
 import { property } from 'lit/decorators.js';
 
 import type { AbstractConstructor } from './constructor';
 
 export declare class SbbIconNameMixinType {
-  public iconName: string;
+  public iconName?: string;
+  public renderIconSlot(): TemplateResult;
 }
 
 // eslint-disable-next-line @typescript-eslint/naming-convention
 export const SbbIconNameMixin = <T extends AbstractConstructor<LitElement>>(
   superClass: T,
 ): AbstractConstructor<SbbIconNameMixinType> & T => {
-  abstract class SbbIconName extends superClass implements Partial<SbbIconNameMixinType> {
+  abstract class SbbIconName extends superClass implements SbbIconNameMixinType {
     /**
      * The icon name we want to use, choose from the small icon variants
      * from the ui-icons category from here
      * https://icons.app.sbb.ch.
      */
     @property({ attribute: 'icon-name', reflect: true }) public iconName?: string;
+
+    /**
+     * @private
+     */
+    public renderIconSlot(): TemplateResult {
+      return html`
+        <slot name="icon">
+          ${this.iconName ? html`<sbb-icon name="${this.iconName}"></sbb-icon>` : nothing}
+        </slot>
+      `;
+    }
   }
 
   return SbbIconName as AbstractConstructor<SbbIconNameMixinType> & T;

diff --git a/src/components/core/common-behaviors/negative-mixin.ts b/src/components/core/common-behaviors/negative-mixin.ts
@@ -11,9 +11,9 @@ export declare class SbbNegativeMixinType {
 export const SbbNegativeMixin = <T extends AbstractConstructor<LitElement>>(
   superClass: T,
 ): AbstractConstructor<SbbNegativeMixinType> & T => {
-  abstract class SbbNegative extends superClass implements Partial<SbbNegativeMixinType> {
+  abstract class SbbNegative extends superClass implements SbbNegativeMixinType {
     /** Negative coloring variant flag. */
-    @property({ reflect: true, type: Boolean }) public negative = false;
+    @property({ reflect: true, type: Boolean }) public negative: boolean = false;
   }
 
   return SbbNegative as AbstractConstructor<SbbNegativeMixinType> & T;

diff --git a/src/components/header/common/header-action-common.ts b/src/components/header/common/header-action-common.ts
@@ -1,3 +1,4 @@
+import { html, type TemplateResult } from 'lit';
 import { property } from 'lit/decorators.js';
 import type { CSSResultGroup, LitElement } from 'lit/development';
 
@@ -13,6 +14,7 @@ import style from './header-action.scss?lit&inline';
 export declare class SbbHeaderActionCommonElementMixinType implements SbbIconNameMixinType {
   public expandFrom: SbbHorizontalFrom;
   public iconName: string;
+  public renderIconSlot(): TemplateResult;
 }
 
 // eslint-disable-next-line @typescript-eslint/naming-convention
@@ -44,6 +46,10 @@ export const SbbHeaderActionCommonElementMixin = <T extends AbstractConstructor<
 
     private _handlerRepository = new HandlerRepository(this, actionElementHandlerAspect);
 
+    private _updateExpanded(): void {
+      toggleDatasetEntry(this, 'expanded', !isBreakpoint('zero', this.expandFrom));
+    }
+
     public override connectedCallback(): void {
       super.connectedCallback();
       this._documentResizeObserver.observe(document.documentElement);
@@ -57,8 +63,8 @@ export const SbbHeaderActionCommonElementMixin = <T extends AbstractConstructor<
       this._handlerRepository.disconnect();
     }
 
-    private _updateExpanded(): void {
-      toggleDatasetEntry(this, 'expanded', !isBreakpoint('zero', this.expandFrom));
+    public override renderIconSlot(): TemplateResult {
+      return html` <span class="sbb-header-action__icon"> ${super.renderIconSlot()} </span> `;
     }
   }
   return SbbHeaderActionCommonElement as unknown as AbstractConstructor<SbbHeaderActionCommonElementMixinType> &

diff --git a/src/components/header/header-button/header-button.ts b/src/components/header/header-button/header-button.ts
@@ -1,4 +1,4 @@
-import { nothing, type TemplateResult } from 'lit';
+import type { TemplateResult } from 'lit';
 import { customElement } from 'lit/decorators.js';
 import { html } from 'lit/static-html.js';
 
@@ -23,11 +23,7 @@ export class SbbHeaderButtonElement extends SbbHeaderActionCommonElementMixin(
     return html`
       <span class="sbb-header-action">
         <span class="sbb-header-action__wrapper">
-          <span class="sbb-header-action__icon">
-            <slot name="icon">
-              ${this.iconName ? html`<sbb-icon name="${this.iconName}"></sbb-icon>` : nothing}
-            </slot>
-          </span>
+          ${this.renderIconSlot()}
           <span class="sbb-header-action__text">
             <slot></slot>
           </span>

diff --git a/src/components/header/header-link/header-link.ts b/src/components/header/header-link/header-link.ts
@@ -32,11 +32,7 @@ export class SbbHeaderLinkElement extends SbbHeaderActionCommonElementMixin(SbbL
     return html`
       <a class="sbb-header-action" ${spread(attributes)}>
         <span class="sbb-header-action__wrapper">
-          <span class="sbb-header-action__icon">
-            <slot name="icon">
-              ${this.iconName ? html`<sbb-icon name="${this.iconName}"></sbb-icon>` : nothing}
-            </slot>
-          </span>
+          ${this.renderIconSlot()}
           <span class="sbb-header-action__text">
             <slot></slot>
             ${targetsNewWindow(this)

diff --git a/src/components/link/common/link-common.ts b/src/components/link/common/link-common.ts
@@ -1,4 +1,4 @@
-import type { CSSResultGroup, LitElement } from 'lit';
+import { type CSSResultGroup, html, type LitElement, type TemplateResult } from 'lit';
 import { property } from 'lit/decorators.js';
 
 import type {
@@ -32,6 +32,7 @@ export declare class SbbLinkCommonElementMixinType
   public disabled: boolean;
   public iconName: string;
   public negative: boolean;
+  public renderIconSlot(): TemplateResult;
 }
 
 // eslint-disable-next-line @typescript-eslint/naming-convention
@@ -81,6 +82,10 @@ export const SbbLinkCommonElementMixin = <T extends AbstractConstructor<LitEleme
       super.disconnectedCallback();
       this._handlerRepository.disconnect();
     }
+
+    public override renderIconSlot(): TemplateResult {
+      return html` <span class="sbb-link__icon"> ${super.renderIconSlot()} </span> `;
+    }
   }
   return SbbLinkCommonElement as unknown as AbstractConstructor<SbbLinkCommonElementMixinType> & T;
 };
diff --git a/src/components/link/link-button/link-button.ts b/src/components/link/link-button/link-button.ts
@@ -26,13 +26,7 @@ export class SbbLinkButtonElement extends SbbLinkCommonElementMixin(SbbButtonBas
     /* eslint-disable lit/binding-positions */
     return html`
       <span class="sbb-link">
-        ${this.variant !== 'inline'
-          ? html`<span class="sbb-link__icon">
-              <slot name="icon">
-                ${this.iconName ? html` <sbb-icon name="${this.iconName}"></sbb-icon>` : nothing}
-              </slot>
-            </span>`
-          : nothing}
+        ${this.variant !== 'inline' ? this.renderIconSlot() : nothing}
         <slot></slot>
       </span>
     `;

diff --git a/src/components/link/link-button/readme.md b/src/components/link/link-button/readme.md
@@ -64,7 +64,7 @@ and it has also three sizes (`xs`, `s`, which is the default, and `m`) that are
 | `isStatic`      | `is-static`      | public  | `boolean`                       | `false`   | Set this property to true if you want only a visual representation of a link, but no interaction (a span instead of a link/button will be rendered). |
 | `iconPlacement` | `icon-placement` | public  | `SbbIconPlacement \| undefined` | `'start'` | Moves the icon to the end of the component if set to true.                                                                                           |
 | `negative`      | `negative`       | public  | `boolean`                       | `false`   | Negative coloring variant flag.                                                                                                                      |
-| `disabled`      | `disabled`       | public  | `boolean \| undefined`          | `false`   | Whether the button is disabled.                                                                                                                      |
+| `disabled`      | `disabled`       | public  | `boolean`                       | `false`   | Whether the button is disabled.                                                                                                                      |
 | `iconName`      | `icon-name`      | public  | `string \| undefined`           |           | The icon name we want to use, choose from the small icon variants from the ui-icons category from here https://icons.app.sbb.ch.                     |
 | `type`          | `type`           | public  | `ButtonType \| undefined`       |           | The type attribute to use for the button.                                                                                                            |
 | `name`          | `name`           | public  | `string \| undefined`           |           | The name attribute to use for the button.                                                                                                            |

diff --git a/src/components/link/link/link.ts b/src/components/link/link/link.ts
@@ -38,15 +38,7 @@ export class SbbLinkElement extends SbbLinkCommonElementMixin(SbbLinkBaseElement
     /* eslint-disable lit/binding-positions */
     return html`
       <${unsafeStatic(TAG_NAME)} class='sbb-link' ${spread(attributes)}>
-        ${
-          this.variant !== 'inline'
-            ? html`<span class="sbb-link__icon">
-                <slot name="icon">
-                  ${this.iconName ? html` <sbb-icon name="${this.iconName}"></sbb-icon>` : nothing}
-                </slot>
-              </span>`
-            : nothing
-        }
+        ${this.variant !== 'inline' ? this.renderIconSlot() : nothing}
         <slot></slot>
         ${
           targetsNewWindow(this)

diff --git a/src/components/link/link/readme.md b/src/components/link/link/readme.md
@@ -63,7 +63,7 @@ and it has also three sizes (`xs`, `s`, which is the default, and `m`) that are
 | `isStatic`      | `is-static`      | public  | `boolean`                               | `false`   | Set this property to true if you want only a visual representation of a link, but no interaction (a span instead of a link/button will be rendered). |
 | `iconPlacement` | `icon-placement` | public  | `SbbIconPlacement \| undefined`         | `'start'` | Moves the icon to the end of the component if set to true.                                                                                           |
 | `negative`      | `negative`       | public  | `boolean`                               | `false`   | Negative coloring variant flag.                                                                                                                      |
-| `disabled`      | `disabled`       | public  | `boolean \| undefined`                  | `false`   | Whether the button is disabled.                                                                                                                      |
+| `disabled`      | `disabled`       | public  | `boolean`                               | `false`   | Whether the button is disabled.                                                                                                                      |
 | `iconName`      | `icon-name`      | public  | `string \| undefined`                   |           | The icon name we want to use, choose from the small icon variants from the ui-icons category from here https://icons.app.sbb.ch.                     |
 | `href`          | `href`           | public  | `string \| undefined`                   |           | The href value you want to link to.                                                                                                                  |
 | `target`        | `target`         | public  | `LinkTargetType \| string \| undefined` |           | Where to display the linked URL.                                                                                                                     |

diff --git a/src/components/menu/common/menu-action-common.ts b/src/components/menu/common/menu-action-common.ts
@@ -1,4 +1,4 @@
-import type { CSSResultGroup, LitElement } from 'lit';
+import { type CSSResultGroup, html, type LitElement, type TemplateResult } from 'lit';
 import { property } from 'lit/decorators.js';
 
 import type {
@@ -17,6 +17,7 @@ export declare class SbbMenuActionCommonElementMixinType
   public amount: string;
   public disabled: boolean;
   public iconName: string;
+  public renderIconSlot(): TemplateResult;
 }
 
 // eslint-disable-next-line @typescript-eslint/naming-convention
@@ -43,6 +44,10 @@ export const SbbMenuActionCommonElementMixin = <T extends AbstractConstructor<Li
       super.disconnectedCallback();
       this._handlerRepository.disconnect();
     }
+
+    public override renderIconSlot(): TemplateResult {
+      return html` <span class="sbb-menu-action__icon"> ${super.renderIconSlot()} </span> `;
+    }
   }
   return SbbMenuActionCommonElement as unknown as AbstractConstructor<SbbMenuActionCommonElementMixinType> &
     T;

diff --git a/src/components/menu/menu-button/menu-button.ts b/src/components/menu/menu-button/menu-button.ts
@@ -21,11 +21,7 @@ export class SbbMenuButtonElement extends SbbMenuActionCommonElementMixin(SbbBut
     return html`
       <span class="sbb-menu-action">
         <span class="sbb-menu-action__content">
-          <span class="sbb-menu-action__icon">
-            <slot name="icon"
-              >${this.iconName ? html`<sbb-icon name=${this.iconName}></sbb-icon>` : nothing}</slot
-            >
-          </span>
+          ${this.renderIconSlot()}
           <span class="sbb-menu-action__label">
             <slot></slot>
           </span>