Skip to content

Commit

Permalink
refactor: refactor checkbox part
Browse files Browse the repository at this point in the history
  • Loading branch information
@jeripeierSBB
jeripeierSBB committed Dec 7, 2023
1 parent 606ae6b commit 3a91a60
Show file tree
Hide file tree
Showing 8 changed files with 87 additions and 137 deletions.
12 changes: 5 additions & 7 deletions src/components/checkbox/checkbox-group/checkbox-group.e2e.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,12 @@ describe('sbb-checkbox-group', () => {
await waitForLitRender(element);
expect(element).to.have.attribute('disabled');

expect(checkboxOne).to.have.attribute('data-group-disabled');
expect(checkboxTwo).to.have.attribute('data-group-disabled');
expect(checkboxOne).to.have.attribute('disabled');
expect(checkboxTwo).to.have.attribute('disabled');
expect(checkboxThree).to.have.attribute('data-group-disabled');
expect(checkboxThree).to.have.attribute('disabled');

element.removeAttribute('disabled');
await waitForLitRender(element);
expect(checkboxTwo).not.to.have.attribute('data-group-disabled');
expect(checkboxTwo).to.have.attribute('disabled');
});

Expand Down Expand Up @@ -76,9 +74,9 @@ describe('sbb-checkbox-group', () => {
element.setAttribute('required', 'true');
await waitForLitRender(element);
expect(element).to.have.attribute('required');
expect(checkboxOne).to.have.attribute('data-group-required');
expect(checkboxTwo).to.have.attribute('data-group-required');
expect(checkboxThree).to.have.attribute('data-group-required');
expect(checkboxOne).to.have.attribute('required');
expect(checkboxTwo).to.have.attribute('required');
expect(checkboxThree).to.have.attribute('required');
});

it('arrow navigation', async () => {
Expand Down
68 changes: 15 additions & 53 deletions src/components/checkbox/checkbox-group/checkbox-group.ts
View file
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
import { CSSResultGroup, html, LitElement, nothing, TemplateResult, PropertyValues } from 'lit';
import { CSSResultGroup, html, LitElement, nothing, PropertyValues, TemplateResult } from 'lit';
import { customElement, property, state } from 'lit/decorators.js';

import { isArrowKeyPressed, getNextElementIndex, interactivityChecker } from '../../core/a11y';
import { getNextElementIndex, interactivityChecker, isArrowKeyPressed } from '../../core/a11y';
import { toggleDatasetEntry } from '../../core/dom';
import {
ConnectedAbortController,
createNamedSlotState,
HandlerRepository,
namedSlotChangeHandlerAspect,
ConnectedAbortController,
} from '../../core/eventing';
import { SbbHorizontalFrom, SbbOrientation } from '../../core/interfaces';
import type { SbbCheckbox, SbbCheckboxSize } from '../checkbox';
Expand Down Expand Up @@ -41,6 +41,13 @@ export class SbbCheckboxGroup extends LitElement {
@property({ reflect: true })
public orientation: SbbOrientation = 'horizontal';

/** List of contained checkbox elements. */
public get checkboxes(): SbbCheckbox[] {
return Array.from(this.querySelectorAll?.('sbb-checkbox') ?? []).filter(
(el: SbbCheckbox) => el.closest('sbb-checkbox-group') === this,
);
}

/** State of listed named slots, by indicating whether any element for a named slot is defined. */
@state() private _namedSlots = createNamedSlotState('error');

Expand All @@ -49,60 +56,35 @@ export class SbbCheckboxGroup extends LitElement {
namedSlotChangeHandlerAspect((m) => (this._namedSlots = m(this._namedSlots))),
);

private _didLoad = false;
private _abort: ConnectedAbortController = new ConnectedAbortController(this);

private _updateDisabled(): void {
for (const checkbox of this._checkboxes) {
toggleDatasetEntry(checkbox, 'groupDisabled', this.disabled);
}
}

private _updateRequired(): void {
for (const checkbox of this._checkboxes) {
toggleDatasetEntry(checkbox, 'groupRequired', this.required);
}
}

private _updateSize(): void {
for (const checkbox of this._checkboxes) {
checkbox.size = this.size;
}
}

public override connectedCallback(): void {
super.connectedCallback();
const signal = this._abort.signal;
this.addEventListener('keydown', (e) => this._handleKeyDown(e), { signal });
toggleDatasetEntry(this, 'hasSelectionPanel', !!this.querySelector?.('sbb-selection-panel'));
this._handlerRepository.connect();
this._updateCheckboxes();
}

protected override willUpdate(changedProperties: PropertyValues<this>): void {
if (changedProperties.has('disabled')) {
this._updateDisabled();
this.checkboxes.forEach((c) => c.requestUpdate?.('disabled'));
}
if (changedProperties.has('required')) {
this._updateRequired();
this.checkboxes.forEach((c) => c.requestUpdate?.('required'));
}
if (changedProperties.has('size')) {
this._updateSize();
this.checkboxes.forEach((c) => c.requestUpdate?.('size'));
}
}

protected override firstUpdated(): void {
this._didLoad = true;
this._updateCheckboxes();
}

public override disconnectedCallback(): void {
super.disconnectedCallback();
this._handlerRepository.disconnect();
}

private _handleKeyDown(evt: KeyboardEvent): void {
const enabledCheckboxes: SbbCheckbox[] = this._checkboxes.filter(
const enabledCheckboxes: SbbCheckbox[] = this.checkboxes.filter(
(checkbox: SbbCheckbox) => !checkbox.disabled && interactivityChecker.isVisible(checkbox),
);

Expand All @@ -123,30 +105,10 @@ export class SbbCheckboxGroup extends LitElement {
}
}

private _updateCheckboxes(): void {
if (!this._didLoad) {
return;
}

const checkboxes = this._checkboxes;

for (const checkbox of checkboxes) {
checkbox.size = this.size;
toggleDatasetEntry(checkbox, 'groupDisabled', this.disabled);
toggleDatasetEntry(checkbox, 'groupRequired', this.required);
}
}

private get _checkboxes(): SbbCheckbox[] {
return Array.from(this.querySelectorAll?.('sbb-checkbox') ?? []).filter(
(el: SbbCheckbox) => el.closest('sbb-checkbox-group') === this,
);
}

protected override render(): TemplateResult {
return html`
<div class="sbb-checkbox-group">
<slot @slotchange=${() => this._updateCheckboxes()}></slot>
<slot></slot>
</div>
${this._namedSlots.error
? html`<div class="sbb-checkbox-group__error">
Expand Down
1 change: 1 addition & 0 deletions src/components/checkbox/checkbox-group/readme.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ Two values are available, `s` and `m`, which is the default
| `size` | `size` | public | `SbbCheckboxSize` | `'m'` | Size variant, either m or s. |
| `horizontalFrom` | `horizontal-from` | public | `SbbHorizontalFrom \| undefined` | | Overrides the behaviour of `orientation` property. |
| `orientation` | `orientation` | public | `SbbOrientation` | `'horizontal'` | Indicates the orientation of the checkboxes inside the `<sbb-checkbox-group>`. |
| `checkboxes` | - | public | `SbbCheckbox[]` | | List of contained checkbox elements. |

## Slots

Expand Down
2 changes: 1 addition & 1 deletion src/components/checkbox/checkbox/checkbox.scss
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
outline: none !important;
}

:host(:is([data-group-disabled], [disabled])) {
:host([disabled]) {
--sbb-checkbox-label-color: var(--sbb-color-charcoal-default);
--sbb-checkbox-cursor: default;
--sbb-checkbox-subtext-color: var(--sbb-color-smoke-default);
Expand Down
102 changes: 45 additions & 57 deletions src/components/checkbox/checkbox/checkbox.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { CSSResultGroup, html, LitElement, nothing, TemplateResult, PropertyValu
import { customElement, property, state } from 'lit/decorators.js';
import { ref } from 'lit/directives/ref.js';

import { setAttributes, isValidAttribute } from '../../core/dom';
import { setAttributes } from '../../core/dom';
import {
createNamedSlotState,
documentLanguage,
Expand All @@ -22,9 +22,9 @@ import {
SbbCheckedStateChange,
SbbDisabledStateChange,
} from '../../core/interfaces';
import { AgnosticMutationObserver } from '../../core/observers';
import '../../visual-checkbox';
import '../../icon';
import type { SbbCheckboxGroup } from '../checkbox-group';

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

Expand All @@ -35,11 +35,6 @@ export type SbbCheckboxStateChange = Extract<

export type SbbCheckboxSize = 's' | 'm';

/** Configuration for the attribute to look at if component is nested in a sbb-checkbox-group */
const checkboxObserverConfig: MutationObserverInit = {
attributeFilter: ['data-group-required', 'data-group-disabled'],
};

/**
* It displays a checkbox enhanced with the SBB Design.
*
Expand All @@ -62,10 +57,30 @@ export class SbbCheckbox extends LitElement {
@property() public value?: string;

/** Whether the checkbox is disabled. */
@property({ reflect: true, type: Boolean }) public disabled = false;
@property({ reflect: true, type: Boolean })
public set disabled(value: boolean) {
this._disabled = value;
}
public get disabled(): boolean {
return this._disabled || (this.group?.disabled ?? false);
}
private _disabled = false;

/** Whether the checkbox is required. */
@property({ reflect: true, type: Boolean }) public required = false;
@property({ reflect: true, type: Boolean })
public set required(value: boolean) {
this._required = value;
}
public get required(): boolean {
return this._required || (this.group?.required ?? false);
}
private _required = false;

/** Reference to the connected checkbox group. */
public get group(): SbbCheckboxGroup | null {
return this._group;
}
private _group: SbbCheckboxGroup | null;

/** Whether the checkbox is indeterminate. */
@property({ reflect: true, type: Boolean }) public indeterminate = false;
Expand All @@ -84,13 +99,14 @@ export class SbbCheckbox extends LitElement {
@property({ reflect: true, type: Boolean }) public checked = false;

/** Label size variant, either m or s. */
@property({ reflect: true }) public size: SbbCheckboxSize = 'm';

/** Whether the component must be set disabled due disabled attribute on sbb-checkbox-group. */
@state() private _disabledFromGroup = false;

/** Whether the component must be set required due required attribute on sbb-checkbox-group. */
@state() private _requiredFromGroup = false;
@property({ reflect: true })
public set size(value: SbbCheckboxSize) {
this._size = value;
}
public get size(): SbbCheckboxSize {
return this.group?.size ?? this._size;
}
private _size: SbbCheckboxSize = 'm';

/** State of listed named slots, by indicating whether any element for a named slot is defined. */
@state() private _namedSlots = createNamedSlotState('icon', 'subtext', 'suffix');
Expand All @@ -107,11 +123,6 @@ export class SbbCheckbox extends LitElement {
private _selectionPanelElement: HTMLElement;
private _abort: ConnectedAbortController = new ConnectedAbortController(this);

/** MutationObserver on data attributes. */
private _checkboxAttributeObserver = new AgnosticMutationObserver(
this._onCheckboxAttributesChange.bind(this),
);

/**
* @deprecated only used for React. Will probably be removed once React 19 is available.
*/
Expand Down Expand Up @@ -161,40 +172,18 @@ export class SbbCheckbox extends LitElement {
formElementHandlerAspect,
);

// Set up the initial disabled/required values and start observe attributes changes.
private _setupInitialStateAndAttributeObserver(): void {
const parentGroup = this.closest?.('sbb-checkbox-group');
if (parentGroup) {
this._requiredFromGroup = parentGroup.required;
this._disabledFromGroup = parentGroup.disabled;
this.size = parentGroup.size;
}
this._checkboxAttributeObserver.observe(this, checkboxObserverConfig);
}

/** Observe changes on data attributes and set the appropriate values. */
private _onCheckboxAttributesChange(mutationsList: MutationRecord[]): void {
for (const mutation of mutationsList) {
if (mutation.attributeName === 'data-group-disabled') {
this._disabledFromGroup = !!isValidAttribute(this, 'data-group-disabled');
}
if (mutation.attributeName === 'data-group-required') {
this._requiredFromGroup = !!isValidAttribute(this, 'data-group-required');
}
}
}

public override connectedCallback(): void {
super.connectedCallback();
const signal = this._abort.signal;
this.addEventListener('click', (e) => this._handleClick(e), { signal });
this.addEventListener('keyup', (e) => this._handleKeyup(e), { signal });
this._group = this.closest('sbb-checkbox-group') as SbbCheckboxGroup;
// We can use closest here, as we expect the parent sbb-selection-panel to be in light DOM.
this._selectionPanelElement = this.closest?.('sbb-selection-panel');
this._isSelectionPanelInput =
!!this._selectionPanelElement && !this.closest?.('sbb-selection-panel [slot="content"]');

const signal = this._abort.signal;
this.addEventListener('click', (e) => this._handleClick(e), { signal });
this.addEventListener('keyup', (e) => this._handleKeyup(e), { signal });
this._handlerRepository.connect();
this._setupInitialStateAndAttributeObserver();
this._checkboxLoaded.emit();
}

Expand All @@ -214,19 +203,18 @@ export class SbbCheckbox extends LitElement {
public override disconnectedCallback(): void {
super.disconnectedCallback();
this._handlerRepository.disconnect();
this._checkboxAttributeObserver.disconnect();
}

// Forward the click on the inner label.
private _handleClick(event: MouseEvent): void {
if (!this.disabled && !this._disabledFromGroup && getEventTarget(event) === this) {
if (!this.disabled && getEventTarget(event) === this) {
this.shadowRoot.querySelector('label').click();
}
}

private _handleKeyup(event: KeyboardEvent): void {
// The native checkbox input toggles state on keyup with space.
if (!this.disabled && !this._disabledFromGroup && event.key === ' ') {
if (!this.disabled && event.key === ' ') {
// The toggle needs to happen after the keyup event finishes, so we schedule
// it to be triggered after the current event loop.
setTimeout(() => this._checkbox.click());
Expand Down Expand Up @@ -266,10 +254,10 @@ export class SbbCheckbox extends LitElement {
const attributes: Record<string, string | boolean> = {
role: 'checkbox',
'aria-checked': this.indeterminate ? 'mixed' : this.checked?.toString() ?? 'false',
'aria-required': (this.required || this._requiredFromGroup).toString(),
'aria-disabled': (this.disabled || this._disabledFromGroup).toString(),
'aria-required': this.required.toString(),
'aria-disabled': this.disabled.toString(),
'data-is-selection-panel-input': this._isSelectionPanelInput,
...(this.disabled || this._disabledFromGroup ? undefined : { tabIndex: '0' }),
...(this.disabled ? undefined : { tabIndex: '0' }),
};
setAttributes(this, attributes);

Expand All @@ -280,8 +268,8 @@ export class SbbCheckbox extends LitElement {
type="checkbox"
aria-hidden="true"
tabindex=${-1}
?disabled=${this.disabled || this._disabledFromGroup}
?required=${this.required || this._requiredFromGroup}
?disabled=${this.disabled}
?required=${this.required}
?checked=${this.checked}
.value=${this.value || nothing}
@input=${() => this._handleInputEvent()}
Expand All @@ -300,7 +288,7 @@ export class SbbCheckbox extends LitElement {
<sbb-visual-checkbox
?checked=${this.checked}
?indeterminate=${this.indeterminate}
?disabled=${this.disabled || this._disabledFromGroup}
?disabled=${this.disabled}
></sbb-visual-checkbox>
</span>
<span class="sbb-checkbox__label">
Expand Down
Loading

0 comments on commit 3a91a60

Please sign in to comment.