Skip to content

Commit

Permalink
feat: button variant refactoring (#2438)
Browse files Browse the repository at this point in the history
## Breaking changes 

### Link and button base classes

__Check the next paragraphs for more information about `sbb-button` and `sbb-link` variants splitting__

All of these components has been split in two, one with only-button and one with only-link behavior. 
- `sbb-card-action`: split in `sbb-card-button` and `sbb-card-link`;
- `sbb-menu-action`: split in `sbb-menu-button` and `sbb-menu-link`;
- `sbb-navigation-action`: split in `sbb-navigation-button` and `sbb-navigation-link`.

These components has been split in three, one with only-button and one with only-link behavior, plus a static variant which replaces the `isStatic` flag. 
- `sbb-button`: split in `sbb-button`, `sbb-button-link` and `sbb-button-static`;
- `sbb-link`: split in `sbb-link`, `sbb-link-button` and `sbb-link-static`.

Check your code and change accordingly, eg. 
- `sbb-button` with `href` set => change to `sbb-button-link`;
- `sbb-menu-action` with `name` and `form` set => change to `sbb-menu-button`;
- `sbb-link` with `href` set => no changes;
- `sbb-navigation-action` with `value` set => change to `sbb-navigation-button`;
- `sbb-button` with `isStatic` set to true => change to `sbb-button-static`;
- `sbb-link` used within an anchor tag => change to `sbb-link-static`;
- ...

Since the `isStatic` flag has been removed, check for usage of nested buttons / links and possibly fix them using the two new static variants.

### Link variants split

All these components have been split:
- `sbb-link` has been split into two components, based on variant
  - `<sbb-link>` becomes `<sbb-block-link>`
  - `<sbb-link variant="block">` becomes `<sbb-block-link>`
  - `<sbb-link variant="inline">` becomes `<sbb-link>`

- `sbb-link-button` has been split into two components, based on variant
  - `<sbb-link-button>` becomes `<sbb-block-link-button>`
  - `<sbb-link-button variant="block">` becomes `<sbb-block-link-button>`
  - `<sbb-link-button variant="inline">` becomes `<sbb-link-button>`

- `sbb-link-static` has been split into two components, based on variant
  - `<sbb-link-static>` becomes `<sbb-block-link-static>`
  - `<sbb-link-static variant="block">` becomes `<sbb-block-link-static>`
  - `<sbb-link-static variant="inline">` becomes `<sbb-link-static>`

Other changes:
- `sbb-action-group` only accepts `sbb-block-link | sbb-block-link-button`
- `sbb-link-list` only accepts `sbb-block-link | sbb-block-link-button`
- `sbb-skiplink-list` only accepts `sbb-block-link | sbb-block-link-button`
- `sbb-toast` only accepts `sbb-link | sbb-link-button`
- `link-variables` mixin renamed to `block-link-variables`,
- `link-variables--negative` mixin renamed to `block-link-variables--negative`,
- `link-variables--inline` mixin renamed to `link-variables`,
- `link-variables--inline-negative` mixin renamed to `link-variables--negative`
- `link-inline-consolidation` mixin renamed to `link-consolidation`,
- `link-inline` mixin renamed to `link`,
- `link-inline-negative` mixin renamed to `link-negative`

### Button variants split

- The `variant` property in `sbb-button` has been removed and for each value a new component has been added:
  - `sbb-secondary-button` for `secondary` variant of the `sbb-button`;
  - `sbb-tertiary-button` for `tertiary` variant of the `sbb-button`;
  - `sbb-transparent-button` for `transparent` variant of the `sbb-button`;
  - `sbb-secondary-button-link` for `secondary` variant of the `sbb-button-link`;
  - `sbb-tertiary-button-link` for `tertiary` variant of the `sbb-button-link`;
  - `sbb-transparent-button-link` for `transparent` variant of the `sbb-button-link`;
  - `sbb-secondary-button-static` for `secondary` variant of the `sbb-button-static`;
  - `sbb-tertiary-button-static` for `tertary` variant of the `sbb-button-static`;
  - `sbb-transparent-button-static` for `transparent` variant of the `sbb-button-static`.

  Replace any usage of the `sbb-button` with the `variant`property set with the corresponding new component:
```
// old code
<sbb-button variant='secondary'></sbb-button>
<sbb-button variant='tertiary'></sbb-button>
<sbb-button variant='transparent'></sbb-button>
<sbb-button-link variant='secondary'></sbb-button-link>
...

// new code
<sbb-secondary-button></sbb-secondary-button>
<sbb-tertiary-button></sbb-tertiary-button>
<sbb-transparent-button></sbb-secondary-button>
<sbb-secondary-button-link></sbb-secondary-button-link>
...
```

- When the `sbb-button` is used in prefix/suffix slot of the `sbb-form-field`, it takes some custom style rules. This behavior has now been removed and a new component named `sbb-mini-button` has been created to handle this specific case; the component uses the same style used in the `sbb-form-field-clear`, the `sbb-datepicker-previous-day` and the `sbb-datepicker-next-day`.
Replace the `sbb-button` usage within the `sbb-form-field` with the new `sbb-mini-button`
```
// old code
<sbb-form-field>
  <input/>
  <sbb-button slot='suffix' icon-name='pen-small'></sbb-button>
</sbb-form-field>

// new code
<sbb-form-field>
  <input/>
  <sbb-mini-button slot='suffix' icon-name='pen-small'></sbb-button>
</sbb-form-field>
```

- Replace any `sbb-button` in the `sbb-toast` with `sbb-transparent-button`, since the variant is not automatically set anymore.
  • Loading branch information
DavideMininni-Fincons authored Mar 8, 2024
1 parent a92255a commit 98ea7f5
Show file tree
Hide file tree
Showing 198 changed files with 6,127 additions and 1,545 deletions.
4 changes: 4 additions & 0 deletions src/components/accordion/__snapshots__/accordion.spec.snap.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ snapshots["sbb-accordion renders - Dom"] =
<sbb-expansion-panel-header
aria-controls="sbb-expansion-panel-content-1"
aria-expanded="false"
data-action=""
data-button=""
data-slot-names="unnamed"
dir="ltr"
id="sbb-expansion-panel-header-1"
Expand All @@ -36,6 +38,8 @@ snapshots["sbb-accordion renders - Dom"] =
<sbb-expansion-panel-header
aria-controls="sbb-expansion-panel-content-2"
aria-expanded="false"
data-action=""
data-button=""
data-slot-names="unnamed"
dir="ltr"
id="sbb-expansion-panel-header-2"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,22 @@ snapshots["sbb-action-group renders renders - Dom"] =
link-size="m"
orientation="horizontal"
>
<sbb-button
<sbb-secondary-button
data-action=""
data-button=""
data-sbb-button=""
data-slot-names="unnamed"
dir="ltr"
role="button"
size="l"
tabindex="0"
variant="secondary"
>
Button
</sbb-button>
</sbb-secondary-button>
<sbb-block-link
data-action=""
data-link=""
data-sbb-link=""
data-slot-names="unnamed"
dir="ltr"
href="https://github.com/lyne-design-system/lyne-components"
Expand All @@ -43,7 +48,7 @@ snapshots["sbb-action-group renders renders - ShadowDom"] =
`;
/* end snapshot sbb-action-group renders renders - ShadowDom */

snapshots["sbb-action-group renders A11y tree Chrome"] =
snapshots["sbb-action-group renders A11y tree Chrome"] =
`<p>
{
"role": "WebArea",
Expand Down
26 changes: 16 additions & 10 deletions src/components/action-group/action-group.e2e.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import { assert, expect, fixture } from '@open-wc/testing';
import { html } from 'lit/static-html.js';

import type { SbbButtonElement } from '../button';
import type { SbbSecondaryButtonElement } from '../button';
import { waitForLitRender } from '../core/testing';
import type { SbbBlockLinkElement } from '../link';
import '../button/secondary-button';
import '../link/block-link';

import { SbbActionGroupElement } from './action-group';

Expand All @@ -13,7 +15,7 @@ describe('sbb-action-group', () => {
beforeEach(async () => {
element = await fixture(html`
<sbb-action-group align-group="start" orientation="horizontal">
<sbb-button variant="secondary">Button</sbb-button>
<sbb-secondary-button>Button</sbb-secondary-button>
<sbb-block-link
icon-name="chevron-small-left-small"
icon-placement="start"
Expand All @@ -23,6 +25,7 @@ describe('sbb-action-group', () => {
</sbb-block-link>
</sbb-action-group>
`);
await waitForLitRender(element);
});

it('renders', async () => {
Expand All @@ -31,19 +34,21 @@ describe('sbb-action-group', () => {

describe('property sync', () => {
it('should sync default size with sbb-button', async () => {
const links = Array.from(
document.querySelectorAll('sbb-action-group sbb-button'),
) as SbbButtonElement[];
expect(links.every((l) => l.size === 'l')).to.be.ok;
const buttons = Array.from(
document.querySelectorAll('sbb-action-group sbb-secondary-button'),
) as SbbSecondaryButtonElement[];
expect(buttons.length).to.be.greaterThan(0);
expect(buttons.every((l) => l.size === 'l')).to.be.ok;
});

it('should update attributes with button-size="m"', async () => {
element.setAttribute('button-size', 'm');
await waitForLitRender(element);
const links = Array.from(
document.querySelectorAll('sbb-action-group sbb-button'),
) as SbbButtonElement[];
expect(links.every((l) => l.size === 'm')).to.be.ok;
const buttons = Array.from(
document.querySelectorAll('sbb-action-group sbb-secondary-button'),
) as SbbSecondaryButtonElement[];
expect(buttons.length).to.be.greaterThan(0);
expect(buttons.every((l) => l.size === 'm')).to.be.ok;
});

it('should update attributes with link-size="s"', async () => {
Expand All @@ -52,6 +57,7 @@ describe('sbb-action-group', () => {
const links = Array.from(
document.querySelectorAll('sbb-action-group sbb-block-link'),
) as SbbBlockLinkElement[];
expect(links.length).to.be.greaterThan(0);
expect(links.every((l) => l.size === 's')).to.be.ok;
});
});
Expand Down
20 changes: 10 additions & 10 deletions src/components/action-group/action-group.spec.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
import { expect, fixture } from '@open-wc/testing';
import { html } from 'lit/static-html.js';

import type { SbbButtonElement } from '../button';
import type { SbbSecondaryButtonElement } from '../button';
import { waitForLitRender } from '../core/testing';
import { testA11yTreeSnapshot } from '../core/testing/a11y-tree-snapshot';

import type { SbbActionGroupElement } from './action-group';
import './action-group';
import '../button';
import '../button/secondary-button';
import '../link/block-link';

describe('sbb-action-group', () => {
Expand All @@ -17,7 +17,7 @@ describe('sbb-action-group', () => {
beforeEach(async () => {
element = await fixture(html`
<sbb-action-group align-group="start" orientation="horizontal">
<sbb-button variant="secondary">Button</sbb-button>
<sbb-secondary-button>Button</sbb-secondary-button>
<sbb-block-link
icon-name="chevron-small-left-small"
href="https://github.com/lyne-design-system/lyne-components"
Expand All @@ -43,19 +43,19 @@ describe('sbb-action-group', () => {
describe('property sync', () => {
const assertButtons = (
root: SbbActionGroupElement,
assertion: (link: SbbButtonElement) => boolean,
): boolean => Array.from(root.querySelectorAll('sbb-button')).every(assertion);
assertion: (link: SbbSecondaryButtonElement) => boolean,
): boolean => Array.from(root.querySelectorAll('sbb-secondary-button')).every(assertion);

it('should sync default button-size property with sbb-button', async () => {
const root = (await fixture(html`
<sbb-action-group align-group="start" orientation="horizontal">
<sbb-button variant="secondary">Button</sbb-button>
<sbb-secondary-button>Button</sbb-secondary-button>
<sbb-block-link
icon-name="chevron-small-left-small"
href="https://github.com/lyne-design-system/lyne-components"
>
Link
</sbb-block-link>
Link </sbb-block-link
>˙
</sbb-action-group>
`)) as SbbActionGroupElement;
expect(assertButtons(root, (b) => b.size === 'l')).to.be.ok;
Expand All @@ -64,7 +64,7 @@ describe('sbb-action-group', () => {
it('should sync button-size property with sbb-button', async () => {
const root = (await fixture(html`
<sbb-action-group align-group="start" orientation="horizontal" button-size="m">
<sbb-button variant="secondary">Button</sbb-button>
<sbb-secondary-button>Button</sbb-secondary-button>
<sbb-block-link
icon-name="chevron-small-left-small"
href="https://github.com/lyne-design-system/lyne-components"
Expand All @@ -79,7 +79,7 @@ describe('sbb-action-group', () => {
it('should sync link-size property with sbb-link', async () => {
const root = (await fixture(html`
<sbb-action-group align-group="start" orientation="horizontal" link-size="s">
<sbb-button variant="secondary">Button</sbb-button>
<sbb-secondary-button>Button</sbb-secondary-button>
<sbb-block-link
icon-name="chevron-small-left-small"
href="https://github.com/lyne-design-system/lyne-components"
Expand Down
4 changes: 2 additions & 2 deletions src/components/action-group/action-group.stories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,10 @@ import { sbbSpread } from '../core/dom';
import readme from './readme.md?raw';
import './action-group';
import '../link/block-link';
import '../button';
import '../button/secondary-button';

const secondaryButtonTemplate = (alignSelf?: string): TemplateResult => html`
<sbb-button align-self=${alignSelf || nothing} variant="secondary"> Button 1 </sbb-button>
<sbb-secondary-button align-self=${alignSelf || nothing}> Button 1 </sbb-secondary-button>
`;

const buttonTemplate = (alignSelf?: string): TemplateResult => html`
Expand Down
10 changes: 5 additions & 5 deletions src/components/action-group/action-group.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import type { CSSResultGroup, TemplateResult, PropertyValues } from 'lit';
import type { CSSResultGroup, PropertyValues, TemplateResult } from 'lit';
import { html, LitElement } from 'lit';
import { customElement, property } from 'lit/decorators.js';

import type { SbbButtonElement, SbbButtonLinkElement, SbbButtonSize } from '../button';
import type { SbbButtonCommonElement, SbbButtonSize } from '../button';
import type { SbbHorizontalFrom, SbbOrientation } from '../core/interfaces';
import type {
SbbBlockLinkButtonElement,
Expand Down Expand Up @@ -55,9 +55,9 @@ export class SbbActionGroupElement extends LitElement {
public linkSize: SbbLinkSize = 'm';

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

protected override willUpdate(changedProperties: PropertyValues<this>): void {
Expand Down
8 changes: 4 additions & 4 deletions src/components/action-group/readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ indicate the minimum breakpoint from which the orientation changes to `horizonta

```html
<sbb-action-group orientation="vertical" horizontal-from="small">
<sbb-button variant="secondary">Action 1</sbb-button>
<sbb-secondary-button>Action 1</sbb-secondary-button>
<sbb-button>Action 2</sbb-button>
<sbb-block-link
align-self="end"
Expand All @@ -33,7 +33,7 @@ Default values are `l` for `sbb-button` and `m` for `sbb-block-link`.

```html
<sbb-action-group button-size="m" link-size="s">
<sbb-button variant="secondary">Action 1</sbb-button>
<sbb-secondary-button>Action 1</sbb-secondary-button>
<sbb-block-link
icon-name="chevron-small-left-small"
href="https://github.com/lyne-design-system/lyne-components"
Expand All @@ -57,8 +57,8 @@ instances.

```html
<sbb-action-group align-group="end">
<sbb-button align-self="start" variant="secondary">Action 1</sbb-button>
<sbb-button variant="secondary">Action 2</sbb-button>
<sbb-secondary-button align-self="start">Action 1</sbb-secondary-button>
<sbb-secondary-button>Action 2</sbb-secondary-button>
<sbb-button>Action 3</sbb-button>
</sbb-action-group>
```
Expand Down
10 changes: 7 additions & 3 deletions src/components/alert/alert-group/alert-group.e2e.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { expect, fixture } from '@open-wc/testing';
import { html } from 'lit/static-html.js';

import type { SbbButtonElement } from '../../button';
import type { SbbTransparentButtonElement } from '../../button';
import { waitForCondition, EventSpy, waitForLitRender } from '../../core/testing';
import type { SbbAlertElement } from '../alert';

Expand Down Expand Up @@ -43,7 +43,9 @@ describe('sbb-alert-group', () => {
// When clicking on close button of the first alert
const closeButton = element
.querySelector<SbbAlertElement>('sbb-alert')!
.shadowRoot!.querySelector<SbbButtonElement>('.sbb-alert__close-button-wrapper sbb-button')!;
.shadowRoot!.querySelector<SbbTransparentButtonElement>(
'.sbb-alert__close-button-wrapper sbb-transparent-button',
)!;

closeButton.focus();
closeButton.click();
Expand All @@ -65,7 +67,9 @@ describe('sbb-alert-group', () => {
// When clicking on close button of the second alert
element
.querySelector<SbbAlertElement>('sbb-alert')!
.shadowRoot!.querySelector<SbbButtonElement>('.sbb-alert__close-button-wrapper sbb-button')!
.shadowRoot!.querySelector<SbbTransparentButtonElement>(
'.sbb-alert__close-button-wrapper sbb-transparent-button',
)!
.click();
await waitForLitRender(element);

Expand Down
19 changes: 13 additions & 6 deletions src/components/alert/alert/__snapshots__/alert.spec.snap.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,18 +43,20 @@ snapshots["sbb-alert should render default properties"] =
role="separator"
>
</sbb-divider>
<sbb-button
<sbb-transparent-button
aria-label="Close message"
class="sbb-alert__close-button"
data-action=""
data-button=""
data-sbb-button=""
dir="ltr"
icon-name="cross-small"
negative=""
role="button"
size="m"
tabindex="0"
variant="transparent"
>
</sbb-button>
</sbb-transparent-button>
</span>
</div>
</div>
Expand Down Expand Up @@ -96,6 +98,9 @@ snapshots["sbb-alert should render customized properties"] =
</p>
<sbb-link
aria-label="label"
data-action=""
data-link=""
data-sbb-link=""
data-slot-names="unnamed"
dir="ltr"
href="https://www.sbb.ch"
Expand All @@ -118,18 +123,20 @@ snapshots["sbb-alert should render customized properties"] =
role="separator"
>
</sbb-divider>
<sbb-button
<sbb-transparent-button
aria-label="Close message"
class="sbb-alert__close-button"
data-action=""
data-button=""
data-sbb-button=""
dir="ltr"
icon-name="cross-small"
negative=""
role="button"
size="m"
tabindex="0"
variant="transparent"
>
</sbb-button>
</sbb-transparent-button>
</span>
</div>
</div>
Expand Down
7 changes: 3 additions & 4 deletions src/components/alert/alert/alert.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import type { SbbTitleLevel } from '../../title';

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

import '../../button';
import '../../button/transparent-button';
import '../../divider';
import '../../icon';
import '../../link';
Expand Down Expand Up @@ -165,15 +165,14 @@ export class SbbAlertElement extends SbbIconNameMixin(LitElement) {
negative
class="sbb-alert__close-button-divider"
></sbb-divider>
<sbb-button
variant="transparent"
<sbb-transparent-button
negative
size="m"
icon-name="cross-small"
@click=${() => this.requestDismissal()}
aria-label=${i18nCloseAlert[this._language.current]}
class="sbb-alert__close-button"
></sbb-button>
></sbb-transparent-button>
</span>`
: nothing}
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ snapshots["sbb-breadcrumb-group renders - Dom"] =
role="navigation"
>
<sbb-breadcrumb
data-action=""
data-link=""
dir="ltr"
href="https://example.com"
icon-name="pie-small"
Expand All @@ -16,6 +18,8 @@ snapshots["sbb-breadcrumb-group renders - Dom"] =
>
</sbb-breadcrumb>
<sbb-breadcrumb
data-action=""
data-link=""
dir="ltr"
href="https://example.com/one"
role="link"
Expand All @@ -26,6 +30,8 @@ snapshots["sbb-breadcrumb-group renders - Dom"] =
</sbb-breadcrumb>
<sbb-breadcrumb
aria-current="page"
data-action=""
data-link=""
dir="ltr"
href="https://example.com/one"
role="link"
Expand Down
Loading

0 comments on commit 98ea7f5

Please sign in to comment.