Skip to content

Commit

Permalink
refactor: remove link-button properties and create separate classes (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
DavideMininni-Fincons authored Feb 19, 2024
1 parent 891fa31 commit 727f01c
Show file tree
Hide file tree
Showing 306 changed files with 7,140 additions and 5,560 deletions.
50 changes: 32 additions & 18 deletions CODING_STANDARDS.md
Original file line number Diff line number Diff line change
Expand Up @@ -80,37 +80,51 @@ leave it out.
This applies especially to providing two different APIs to accomplish the same thing. Always
prefer sticking to a _single_ API for accomplishing something.

#### Click event handling on action elements
#### Action elements

As we have to "reimplement" button and anchor functionality in order to comply with
accessibility, we need to consider all native behavior of a native `<button>` and `<anchor>`
element.

This has been implemented to the best of our know-how in the form of the
`actionElementHandlerAspect` and `linkHandlerAspect` together with the `HandlerRepository`.
This has been implemented to the best of our know-how in the form of two "base" classes,
named `SbbButtonBaseElement` and `SbbLinkBaseElement`,
which holds all the logic needed to "emulate" the native `<button>` and `<anchor>` elements.
In detail, the new classes implement:

This can be used as follows:
- the native component related properties (`form`, `name`... for the button, `href`, `target`... for the link);
- the interaction logic (like `click`, `keypress` and so on);
- the accessibility attributes (like `role`);
- the rendering of the wrapper tag with its attributes (`span` for the button, `a` for the link).

```ts
import { actionElementHandlerAspect, HandlerRepository } from '../core/eventing';

class ActionElement implements LinkButtonProperties {
private _handlerRepository = new HandlerRepository(this._element, actionElementHandlerAspect);
These classes can be used as follows:
components which require basic button or link functionality have to extend the corresponding "base" class,
and they need to implement the `renderTemplate` method, which should return the component's inner content.

public override connectedCallback(): void {
super.connectedCallback();
this._handlerRepository.connect();
}
```ts
import { SbbButtonBaseElement } from '../../core/common-behaviors';
import { html } from 'lit';

public override disconnectedCallback(): void {
super.disconnectedCallback();
this._handlerRepository.disconnect();
@customElement('my-custom-button')
class MyCustomButtonElement extends SbbButtonBaseElement {
protected override renderTemplate(): TemplateResult {
return html`<span>My button label</span>`;
}
}
```

Be aware that this does not cover every functionality of a native button or anchor. Compare with
existing components (e.g. `<sbb-button>`) to what needs to be done additionally.
This code will result in the following HTML:

```html
<my-custom-button role="button" tabindex="0">
#shadow-root (open)
<span class="my-custom-button">
<span>My button label</span>
</span>
</my-custom-button>
```

Be aware that this does not cover every functionality of a native button or anchor.
Compare with existing components (e.g. `<sbb-button>`, `<sbb-breadcrumb>`, etc.) to what needs to be done additionally.

#### I18N

Expand Down
2 changes: 1 addition & 1 deletion scripts/chromatic-stories-generator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ async function generateChromaticStories(): Promise<void> {
console.log(`Generating chromatic story files:`);
for (const storyFile of walk(
join(dirname(fileURLToPath(import.meta.url)), '../src'),
/.stories.ts$/,
/[.]stories.ts$/,
)) {
if (storyFile.includes('chromatic')) {
continue;
Expand Down
1 change: 0 additions & 1 deletion src/components/accordion/accordion.stories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import { SbbExpansionPanelElement } from '../expansion-panel';

import readme from './readme.md?raw';
import './accordion';
import '../icon';

const numberOfPanels: InputType = {
control: {
Expand Down
2 changes: 1 addition & 1 deletion src/components/action-group/action-group.scss
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,6 @@ $horizontal-orientations: (
}
}

::slotted(sbb-link) {
::slotted(:is(sbb-link, sbb-link-button)) {
white-space: nowrap;
}
14 changes: 8 additions & 6 deletions src/components/action-group/action-group.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@ import type { CSSResultGroup, TemplateResult, PropertyValues } from 'lit';
import { html, LitElement } from 'lit';
import { customElement, property } from 'lit/decorators.js';

import type { SbbButtonElement, SbbButtonSize } from '../button';
import type { SbbButtonElement, SbbButtonLinkElement, SbbButtonSize } from '../button';
import type { SbbHorizontalFrom, SbbOrientation } from '../core/interfaces';
import type { SbbLinkElement, SbbLinkSize } from '../link';
import type { SbbLinkButtonElement, SbbLinkElement, SbbLinkSize } from '../link';

import style from './action-group.scss?lit&inline';

Expand Down Expand Up @@ -50,9 +50,9 @@ export class SbbActionGroupElement extends LitElement {
public linkSize: SbbLinkSize = 'm';

private _syncButtons(): void {
this.querySelectorAll?.('sbb-button').forEach(
(b: SbbButtonElement) => (b.size = this.buttonSize),
);
this.querySelectorAll?.<SbbButtonElement | SbbButtonLinkElement>(
'sbb-button, sbb-button-link',
).forEach((b: SbbButtonElement | SbbButtonLinkElement) => (b.size = this.buttonSize));
}

protected override willUpdate(changedProperties: PropertyValues<this>): void {
Expand All @@ -65,7 +65,9 @@ export class SbbActionGroupElement extends LitElement {
}

private _syncLinks(): void {
this.querySelectorAll?.('sbb-link').forEach((link: SbbLinkElement) => {
this.querySelectorAll?.<SbbLinkElement | SbbLinkButtonElement>(
'sbb-link, sbb-link-button',
).forEach((link: SbbLinkElement | SbbLinkButtonElement) => {
link.variant = 'block';
link.size = this.linkSize;
});
Expand Down
19 changes: 10 additions & 9 deletions src/components/alert/alert/alert.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,21 @@ import { spread } from '@open-wc/lit-helpers';
import { type CSSResultGroup, html, LitElement, nothing, type TemplateResult } from 'lit';
import { customElement, property } from 'lit/decorators.js';

import { LanguageController } from '../../core/common-behaviors';
import {
LanguageController,
type LinkProperties,
type LinkTargetType,
SbbIconNameMixin,
} from '../../core/common-behaviors';
import { EventEmitter } from '../../core/eventing';
import { i18nCloseAlert, i18nFindOutMore } from '../../core/i18n';
import type { LinkProperties, LinkTargetType } from '../../core/interfaces';
import type { TitleLevel } from '../../title';

import style from './alert.scss?lit&inline';

import '../../button';
import '../../divider';
import '../../icon';
import '../../link';
import '../../title';

Expand All @@ -28,7 +33,7 @@ export type SbbAlertState = 'closed' | 'opening' | 'opened';
* @event {CustomEvent<void>} dismissalRequested - Emits when dismissal of an alert was requested.
*/
@customElement('sbb-alert')
export class SbbAlertElement extends LitElement implements LinkProperties {
export class SbbAlertElement extends SbbIconNameMixin(LitElement) implements LinkProperties {
public static override styles: CSSResultGroup = style;
public static readonly events = {
willOpen: 'willOpen',
Expand All @@ -53,7 +58,7 @@ export class SbbAlertElement extends LitElement implements LinkProperties {
* Choose the icons from https://icons.app.sbb.ch.
* Styling is optimized for icons of type HIM-CUS.
*/
@property({ attribute: 'icon-name' }) public iconName?: string;
@property({ attribute: 'icon-name' }) public override iconName: string = 'info';

/** Content of title. */
@property({ attribute: 'title-content' }) public titleContent?: string;
Expand Down Expand Up @@ -135,11 +140,7 @@ export class SbbAlertElement extends LitElement implements LinkProperties {
<!-- sub wrapper needed to properly support fade in animation -->
<div class="sbb-alert__transition-sub-wrapper">
<div class="sbb-alert">
<span class="sbb-alert__icon">
<slot name="icon">
<sbb-icon name=${this.iconName || 'info'}></sbb-icon>
</slot>
</span>
<span class="sbb-alert__icon"> ${this.renderIconSlot()} </span>
<span class="sbb-alert__content">
<sbb-title
class="sbb-alert__title"
Expand Down
28 changes: 14 additions & 14 deletions src/components/alert/alert/readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ The component can optionally display a `sbb-icon` at the component start using t

It's possible to place an action, which by clicking navigates somewhere to display more information.
This can be done using the `linkContent` property combined with the `href` one.
The `target` and `rel` property are also configurable via the self-named properties.
The `target` and `rel` properties are also configurable via the self-named properties.

```html
<sbb-alert
Expand Down Expand Up @@ -75,19 +75,19 @@ Avoid slotting block elements (e.g. `<div>`) as this violates semantic rules and

## Properties

| Name | Attribute | Privacy | Type | Default | Description |
| -------------------- | --------------------- | ------- | --------------------------------------- | ------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| `readonly` | `readonly` | public | `boolean` | `false` | Whether the alert is readonly. In readonly mode, there is no dismiss button offered to the user. |
| `size` | `size` | public | `'m' \| 'l'` | `'m'` | You can choose between `m` or `l` size. |
| `disableAnimation` | `disable-animation` | public | `boolean` | `false` | Whether the fade in animation should be disabled. |
| `iconName` | `icon-name` | public | `string \| undefined` | | Name of the icon which will be forward to the nested `sbb-icon`. Choose the icons from https://icons.app.sbb.ch. Styling is optimized for icons of type HIM-CUS. |
| `titleContent` | `title-content` | public | `string \| undefined` | | Content of title. |
| `titleLevel` | `title-level` | public | `TitleLevel` | `'3'` | Level of title, will be rendered as heading tag (e.g. h3). Defaults to level 3. |
| `linkContent` | `link-content` | public | `string \| undefined` | | Content of the link. |
| `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. |
| `rel` | `rel` | public | `string \| undefined` | | The relationship of the linked URL as space-separated link types. |
| `accessibilityLabel` | `accessibility-label` | public | `string \| undefined` | | This will be forwarded as aria-label to the relevant nested element. |
| Name | Attribute | Privacy | Type | Default | Description |
| -------------------- | --------------------- | ------- | --------------------------------------- | -------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| `readonly` | `readonly` | public | `boolean` | `false` | Whether the alert is readonly. In readonly mode, there is no dismiss button offered to the user. |
| `size` | `size` | public | `'m' \| 'l'` | `'m'` | You can choose between `m` or `l` size. |
| `disableAnimation` | `disable-animation` | public | `boolean` | `false` | Whether the fade in animation should be disabled. |
| `iconName` | `icon-name` | public | `string \| undefined` | `'info'` | Name of the icon which will be forward to the nested `sbb-icon`. Choose the icons from https://icons.app.sbb.ch. Styling is optimized for icons of type HIM-CUS. |
| `titleContent` | `title-content` | public | `string \| undefined` | | Content of title. |
| `titleLevel` | `title-level` | public | `TitleLevel` | `'3'` | Level of title, will be rendered as heading tag (e.g. h3). Defaults to level 3. |
| `linkContent` | `link-content` | public | `string \| undefined` | | Content of the link. |
| `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. |
| `rel` | `rel` | public | `string \| undefined` | | The relationship of the linked URL as space-separated link types. |
| `accessibilityLabel` | `accessibility-label` | public | `string \| undefined` | | This will be forwarded as aria-label to the relevant nested element. |

## Methods

Expand Down
7 changes: 2 additions & 5 deletions src/components/autocomplete/autocomplete.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { customElement, property, state } from 'lit/decorators.js';
import { ref } from 'lit/directives/ref.js';

import { assignId, getNextElementIndex } from '../core/a11y';
import { SlotChildObserver } from '../core/common-behaviors';
import { SbbNegativeMixin, SlotChildObserver } from '../core/common-behaviors';
import {
setAttribute,
getDocumentWritingMode,
Expand Down Expand Up @@ -39,7 +39,7 @@ let nextId = 0;
* @event {CustomEvent<void>} didClose - Emits whenever the `sbb-autocomplete` is closed.
*/
@customElement('sbb-autocomplete')
export class SbbAutocompleteElement extends SlotChildObserver(LitElement) {
export class SbbAutocompleteElement extends SlotChildObserver(SbbNegativeMixin(LitElement)) {
public static override styles: CSSResultGroup = style;
public static readonly events = {
willOpen: 'willOpen',
Expand Down Expand Up @@ -69,9 +69,6 @@ export class SbbAutocompleteElement extends SlotChildObserver(LitElement) {
@property({ attribute: 'preserve-icon-space', reflect: true, type: Boolean })
public preserveIconSpace?: boolean;

/** Negative coloring variant flag. */
@property({ reflect: true, type: Boolean }) public negative = false;

/** The state of the autocomplete. */
@state() private _state: SbbOverlayState = 'closed';

Expand Down
2 changes: 1 addition & 1 deletion src/components/autocomplete/readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -101,9 +101,9 @@ using `aria-activedescendant` to support navigation though the autocomplete opti
| `trigger` | `trigger` | public | `string \| HTMLInputElement \| undefined` | | The input element that will trigger the autocomplete opening; accepts both an element's id or an HTMLElement. By default, the autocomplete will open on focus, click, input or `ArrowDown` keypress of the 'trigger' element. If not set, will search for the first 'input' child of a 'sbb-form-field' ancestor. |
| `disableAnimation` | `disable-animation` | public | `boolean` | `false` | Whether the animation is disabled. |
| `preserveIconSpace` | `preserve-icon-space` | public | `boolean \| undefined` | | Whether the icon space is preserved when no icon is set. |
| `negative` | `negative` | public | `boolean` | `false` | Negative coloring variant flag. |
| `originElement` | - | public | `HTMLElement` | | Returns the element where autocomplete overlay is attached to. |
| `triggerElement` | - | public | `HTMLInputElement \| undefined` | | Returns the trigger element. |
| `negative` | `negative` | public | `boolean` | `false` | Negative coloring variant flag. |

## Methods

Expand Down
Loading

0 comments on commit 727f01c

Please sign in to comment.