Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: remove false check for boolean attributes #2565

Merged
merged 4 commits into from
Apr 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/components/autocomplete/autocomplete.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ describe(`sbb-autocomplete with ${fixture.name}`, () => {
});

it('should stay closed when disabled', async () => {
input.setAttribute('disabled', '');
input.toggleAttribute('disabled', true);

input.focus();
await waitForLitRender(element);
Expand All @@ -166,7 +166,7 @@ describe(`sbb-autocomplete with ${fixture.name}`, () => {
});

it('should stay closed when readonly', async () => {
input.setAttribute('readonly', '');
input.toggleAttribute('readonly', true);

input.focus();
await waitForLitRender(element);
Expand Down
15 changes: 7 additions & 8 deletions src/components/autocomplete/autocomplete.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type { CSSResultGroup, TemplateResult, PropertyValues } from 'lit';
import type { CSSResultGroup, PropertyValues, TemplateResult } from 'lit';
import { html, LitElement, nothing } from 'lit';
import { customElement, property } from 'lit/decorators.js';
import { ref } from 'lit/directives/ref.js';
Expand All @@ -7,11 +7,10 @@ import { getNextElementIndex } from '../core/a11y/index.js';
import { SbbConnectedAbortController } from '../core/controllers/index.js';
import { hostAttributes } from '../core/decorators/index.js';
import {
getDocumentWritingMode,
findReferencedElement,
isSafari,
isValidAttribute,
getDocumentWritingMode,
isBrowser,
isSafari,
} from '../core/dom/index.js';
import { EventEmitter } from '../core/eventing/index.js';
import type { SbbOpenedClosedState } from '../core/interfaces/index.js';
Expand All @@ -23,7 +22,7 @@ import {
setAriaComboBoxAttributes,
setOverlayPosition,
} from '../core/overlay/index.js';
import type { SbbOptionElement, SbbOptGroupElement } from '../option/index.js';
import type { SbbOptGroupElement, SbbOptionElement } from '../option/index.js';

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

Expand Down Expand Up @@ -133,7 +132,7 @@ export class SbbAutocompleteElement extends SbbNegativeMixin(SbbHydrationMixin(L

/** The autocomplete should inherit 'readonly' state from the trigger. */
private get _readonly(): boolean {
return !!this.triggerElement && isValidAttribute(this.triggerElement, 'readonly');
return this.triggerElement?.hasAttribute('readonly') ?? false;
}

private get _options(): SbbOptionElement[] {
Expand Down Expand Up @@ -236,7 +235,7 @@ export class SbbAutocompleteElement extends SbbNegativeMixin(SbbHydrationMixin(L
const formField = this.closest?.('sbb-form-field') ?? this.closest?.('[data-form-field]');

if (formField) {
this.negative = isValidAttribute(formField, 'negative');
this.negative = formField.hasAttribute('negative');
}

if (this._didLoad) {
Expand Down Expand Up @@ -513,7 +512,7 @@ export class SbbAutocompleteElement extends SbbNegativeMixin(SbbHydrationMixin(L

private _setNextActiveOption(event: KeyboardEvent): void {
const filteredOptions = this._options.filter(
(opt) => !opt.disabled && !isValidAttribute(opt, 'data-group-disabled'),
(opt) => !opt.disabled && !opt.hasAttribute('data-group-disabled'),
);

// Get and activate the next active option
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import {
SbbLanguageController,
} from '../../core/controllers/index.js';
import { hostAttributes } from '../../core/decorators/index.js';
import { setAttribute } from '../../core/dom/index.js';
import { setOrRemoveAttribute } from '../../core/dom/index.js';
import { i18nBreadcrumbEllipsisButtonLabel } from '../../core/i18n/index.js';
import { SbbNamedSlotListMixin, type WithListChildren } from '../../core/mixins/index.js';
import { AgnosticResizeObserver } from '../../core/observers/index.js';
Expand Down Expand Up @@ -48,7 +48,7 @@ export class SbbBreadcrumbGroupElement extends SbbNamedSlotListMixin<
/* The state of the breadcrumb group. */
@state()
private set _state(state: 'collapsed' | 'manually-expanded' | null) {
setAttribute(this, 'data-state', state);
setOrRemoveAttribute(this, 'data-state', state);
}
private get _state(): 'collapsed' | 'manually-expanded' | null {
return this.getAttribute('data-state') as 'collapsed' | 'manually-expanded' | null;
Expand Down
2 changes: 1 addition & 1 deletion src/components/card/card-button/card-button.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ describe(`sbb-card-button with ${fixture.name}`, () => {
);
expect(element).not.to.have.attribute('data-has-active-action');

element.querySelector<SbbCardButtonElement>('sbb-card-button')!.setAttribute('active', '');
element.querySelector<SbbCardButtonElement>('sbb-card-button')!.toggleAttribute('active', true);
await waitForLitRender(element);

expect(element).to.have.attribute('data-has-active-action');
Expand Down
2 changes: 1 addition & 1 deletion src/components/card/card-link/card-link.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ describe(`sbb-card-link with ${fixture.name}`, () => {
);
expect(element).not.to.have.attribute('data-has-active-action');

element.querySelector<SbbCardLinkElement>('sbb-card-link')!.setAttribute('active', '');
element.querySelector<SbbCardLinkElement>('sbb-card-link')!.toggleAttribute('active', true);
await waitForLitRender(element);

expect(element).to.have.attribute('data-has-active-action');
Expand Down
2 changes: 1 addition & 1 deletion src/components/core/dom/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ export * from './find-referenced-element.js';
export * from './get-document-writing-mode.js';
export * from './host-context.js';
export * from './input-element.js';
export * from './is-valid-attribute.js';
export * from './set-or-remove-attribute.js';
export * from './platform.js';
export * from './scroll.js';
export * from './ssr.js';
22 changes: 0 additions & 22 deletions src/components/core/dom/is-valid-attribute.ts

This file was deleted.

4 changes: 1 addition & 3 deletions src/components/core/dom/scroll.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
import { isValidAttribute } from './is-valid-attribute.js';

export function pageScrollDisabled(): boolean {
return isValidAttribute(document.body, 'data-sbb-scroll-disabled');
return document.body.hasAttribute('data-sbb-scroll-disabled');
}

/**
Expand Down
13 changes: 13 additions & 0 deletions src/components/core/dom/set-or-remove-attribute.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
/**
* Set the attribute only if value is not 'false', otherwise remove attribute.
* @param element The element that will have the attribute
* @param attribute The attribute name
* @param value The attribute value
*/
export function setOrRemoveAttribute(element: HTMLElement, attribute: string, value?: any): void {
if (!value) {
element.removeAttribute(attribute);
} else {
element.setAttribute(attribute, value);
}
}
9 changes: 4 additions & 5 deletions src/components/datepicker/common/datepicker-button.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,13 @@ import {
SbbLanguageController,
} from '../../core/controllers/index.js';
import { type DateAdapter, defaultDateAdapter } from '../../core/datetime/index.js';
import { isValidAttribute } from '../../core/dom/index.js';
import { i18nToday } from '../../core/i18n/index.js';
import { SbbNegativeMixin } from '../../core/mixins/index.js';
import {
datepickerControlRegisteredEventFactory,
getDatePicker,
type SbbInputUpdateEvent,
type SbbDatepickerElement,
type SbbInputUpdateEvent,
} from '../datepicker/index.js';
import '../../icon/index.js';

Expand Down Expand Up @@ -88,7 +87,7 @@ export abstract class SbbDatepickerButton extends SbbNegativeMixin(SbbButtonBase
}

private _handleClick(): void {
if (!this.datePickerElement || isValidAttribute(this, 'data-disabled')) {
if (!this.datePickerElement || this.hasAttribute('data-disabled')) {
return;
}
const startingDate: Date =
Expand All @@ -107,15 +106,15 @@ export abstract class SbbDatepickerButton extends SbbNegativeMixin(SbbButtonBase
private _syncUpstreamProperties(): void {
const formField = this.closest?.('sbb-form-field') ?? this.closest?.('[data-form-field]');
if (formField) {
this.negative = isValidAttribute(formField, 'negative');
this.negative = formField.hasAttribute('negative');

// We can't use getInputElement of SbbFormFieldElement as async awaiting is not supported in connectedCallback.
// We here only have to look for input.
const inputElement = formField.querySelector('input');

if (inputElement) {
this._inputDisabled =
isValidAttribute(inputElement, 'disabled') || isValidAttribute(inputElement, 'readonly');
inputElement.hasAttribute('disabled') || inputElement.hasAttribute('readonly');
this._setDisabledRenderAttributes();
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ describe(`sbb-datepicker-next-day with ${fixture.name}`, () => {

it('disabled due disabled picker', async () => {
expect(input.value).to.be.equal('Sa, 21.01.2023');
input.setAttribute('disabled', '');
input.toggleAttribute('disabled', true);

await waitForLitRender(element);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ describe(`sbb-datepicker-previous-day with ${fixture.name}`, () => {

it('disabled due disabled picker', async () => {
expect(input.value).to.be.equal('Fr, 20.01.2023');
input.setAttribute('disabled', '');
input.toggleAttribute('disabled', true);
await waitForLitRender(element);

expect(element).to.have.attribute('data-disabled');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,10 @@ import { sbbInputModalityDetector } from '../../core/a11y/index.js';
import { SbbLanguageController } from '../../core/controllers/index.js';
import { readDataNow } from '../../core/datetime/data-now.js';
import { hostAttributes } from '../../core/decorators/index.js';
import { isValidAttribute } from '../../core/dom/index.js';
import { i18nShowCalendar } from '../../core/i18n/index.js';
import { SbbNegativeMixin } from '../../core/mixins/index.js';
import type { SbbPopoverElement, SbbPopoverTriggerElement } from '../../popover/index.js';
import type { SbbInputUpdateEvent, SbbDatepickerElement } from '../datepicker/index.js';
import type { SbbDatepickerElement, SbbInputUpdateEvent } from '../datepicker/index.js';
import { datepickerControlRegisteredEventFactory, getDatePicker } from '../datepicker/index.js';

import style from './datepicker-toggle.scss?lit&inline';
Expand Down Expand Up @@ -72,7 +71,7 @@ export class SbbDatepickerToggleElement extends SbbNegativeMixin(LitElement) {

const formField = this.closest?.('sbb-form-field') ?? this.closest?.('[data-form-field]');
if (formField) {
this.negative = isValidAttribute(formField, 'negative');
this.negative = formField.hasAttribute('negative');
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/components/datepicker/datepicker/datepicker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {
import { readDataNow } from '../../core/datetime/data-now.js';
import type { DateAdapter } from '../../core/datetime/index.js';
import { defaultDateAdapter } from '../../core/datetime/index.js';
import { findInput, findReferencedElement, isValidAttribute } from '../../core/dom/index.js';
import { findInput, findReferencedElement } from '../../core/dom/index.js';
import { EventEmitter } from '../../core/eventing/index.js';
import { i18nDateChangedTo, i18nDatePickerPlaceholder } from '../../core/i18n/index.js';
import type { SbbDateLike, SbbValidationChangeEvent } from '../../core/interfaces/index.js';
Expand Down Expand Up @@ -415,7 +415,7 @@ export class SbbDatepickerElement extends LitElement {
this._inputElement?.min,
this._inputElement?.max,
));
const wasValid = !isValidAttribute(this._inputElement, 'data-sbb-invalid');
const wasValid = !this._inputElement.hasAttribute('data-sbb-invalid');
this._inputElement.toggleAttribute('data-sbb-invalid', !isEmptyOrValid);
if (wasValid !== isEmptyOrValid) {
this._validationChange.emit({ valid: isEmptyOrValid });
Expand Down
6 changes: 3 additions & 3 deletions src/components/dialog/dialog.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ import { customElement, property } from 'lit/decorators.js';
import { ref } from 'lit/directives/ref.js';
import { html, unsafeStatic } from 'lit/static-html.js';

import { SbbFocusHandler, IS_FOCUSABLE_QUERY, setModalityOnNextFocus } from '../core/a11y/index.js';
import { IS_FOCUSABLE_QUERY, SbbFocusHandler, setModalityOnNextFocus } from '../core/a11y/index.js';
import { SbbLanguageController, SbbSlotStateController } from '../core/controllers/index.js';
import { hostContext, isValidAttribute, SbbScrollHandler } from '../core/dom/index.js';
import { hostContext, SbbScrollHandler } from '../core/dom/index.js';
import { EventEmitter } from '../core/eventing/index.js';
import { i18nCloseDialog, i18nDialog, i18nGoBack } from '../core/i18n/index.js';
import type { SbbOpenedClosedState } from '../core/interfaces/index.js';
Expand Down Expand Up @@ -296,7 +296,7 @@ export class SbbDialogElement extends SbbNegativeMixin(LitElement) {
private _closeOnSbbDialogCloseClick(event: Event): void {
const target = event.target as HTMLElement;

if (target.hasAttribute('sbb-dialog-close') && !isValidAttribute(target, 'disabled')) {
if (target.hasAttribute('sbb-dialog-close') && !target.hasAttribute('disabled')) {
// Check if the target is a submission element within a form and return the form, if present
const closestForm =
target.getAttribute('type') === 'submit'
Expand Down
3 changes: 1 addition & 2 deletions src/components/form-error/form-error.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import type { CSSResultGroup, TemplateResult } from 'lit';
import { html, LitElement } from 'lit';
import { customElement } from 'lit/decorators.js';

import { isValidAttribute } from '../core/dom/index.js';
import { SbbNegativeMixin } from '../core/mixins/index.js';

import style from './form-error.scss?lit&inline';
Expand All @@ -24,7 +23,7 @@ export class SbbFormErrorElement extends SbbNegativeMixin(LitElement) {
this.id ||= `sbb-form-error-${nextId++}`;
const formField = this.closest?.('sbb-form-field') ?? this.closest?.('[data-form-field]');
if (formField) {
this.negative = isValidAttribute(formField, 'negative');
this.negative = formField.hasAttribute('negative');
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,15 +42,15 @@ describe(`sbb-form-field-clear with ${fixture.name}`, () => {
});

it('is hidden if the form field is disabled', async () => {
input.setAttribute('disabled', '');
input.toggleAttribute('disabled', true);

await waitForLitRender(element);

expect(element).to.have.style('display', 'none');
});

it('is hidden if the form field is readonly', async () => {
input.setAttribute('readonly', '');
input.toggleAttribute('readonly', true);

await waitForLitRender(element);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {
SbbLanguageController,
} from '../../core/controllers/index.js';
import { hostAttributes } from '../../core/decorators/index.js';
import { hostContext, isValidAttribute } from '../../core/dom/index.js';
import { hostContext } from '../../core/dom/index.js';
import { i18nClearInput } from '../../core/i18n/index.js';
import { SbbNegativeMixin } from '../../core/mixins/index.js';
import type { SbbFormFieldElement } from '../form-field/index.js';
Expand Down Expand Up @@ -40,7 +40,7 @@ export class SbbFormFieldClearElement extends SbbNegativeMixin(SbbButtonBaseElem
(hostContext('[data-form-field]', this) as SbbFormFieldElement);

if (this._formField) {
this.negative = isValidAttribute(this._formField, 'negative');
this.negative = this._formField.hasAttribute('negative');
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/components/form-field/form-field/form-field.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,7 @@ describe(`sbb-form-field with ${fixture.name}`, () => {
});

it('should focus select on form field click readonly', async () => {
select.setAttribute('readonly', '');
select.toggleAttribute('readonly', true);
await waitForLitRender(element);

expect(element).not.to.have.attribute('data-input-focused');
Expand Down
16 changes: 6 additions & 10 deletions src/components/form-field/form-field/form-field.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {
SbbLanguageController,
SbbSlotStateController,
} from '../../core/controllers/index.js';
import { isFirefox, isValidAttribute } from '../../core/dom/index.js';
import { isFirefox, setOrRemoveAttribute } from '../../core/dom/index.js';
import { i18nOptional } from '../../core/i18n/index.js';
import { SbbNegativeMixin } from '../../core/mixins/index.js';
import { AgnosticMutationObserver } from '../../core/observers/index.js';
Expand Down Expand Up @@ -357,8 +357,8 @@ export class SbbFormFieldElement extends SbbNegativeMixin(LitElement) {
if (!this._input) {
return;
}
this.toggleAttribute('data-readonly', isValidAttribute(this._input, 'readonly'));
this.toggleAttribute('data-disabled', isValidAttribute(this._input, 'disabled'));
this.toggleAttribute('data-readonly', this._input.hasAttribute('readonly'));
this.toggleAttribute('data-disabled', this._input.hasAttribute('disabled'));
this.toggleAttribute(
'data-invalid',
this._input.hasAttribute('data-sbb-invalid') ||
Expand Down Expand Up @@ -405,10 +405,8 @@ export class SbbFormFieldElement extends SbbNegativeMixin(LitElement) {
}

const ariaDescribedby = ids.join(' ');
if (ariaDescribedby) {
this._input?.setAttribute('aria-describedby', ariaDescribedby);
} else {
this._input?.removeAttribute('aria-describedby');
if (this._input) {
setOrRemoveAttribute(this._input, 'aria-describedby', ariaDescribedby);
}
}

Expand Down Expand Up @@ -437,9 +435,7 @@ export class SbbFormFieldElement extends SbbNegativeMixin(LitElement) {
private _syncNegative(): void {
this.querySelectorAll?.(
'sbb-form-error,sbb-mini-button,sbb-popover-trigger,sbb-form-field-clear,sbb-datepicker-next-day,sbb-datepicker-previous-day,sbb-datepicker-toggle,sbb-select,sbb-autocomplete',
).forEach((element) =>
this.negative ? element.setAttribute('negative', '') : element.removeAttribute('negative'),
);
).forEach((element) => element.toggleAttribute('negative', this.negative));
}

protected override render(): TemplateResult {
Expand Down
Loading
Loading