Skip to content

Commit

Permalink
fix: review
Browse files Browse the repository at this point in the history
  • Loading branch information
jeripeierSBB committed Mar 6, 2024
1 parent e59deff commit 75d7e4d
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 11 deletions.
12 changes: 9 additions & 3 deletions src/components/card/common/card-action-common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,12 @@ import { property } from 'lit/decorators.js';
import { html } from 'lit/static-html.js';

import { IS_FOCUSABLE_QUERY } from '../../core/a11y';
import type { AbstractConstructor, SbbActionBaseElement } from '../../core/common-behaviors';
import { setAttribute, toggleDatasetEntry } from '../../core/dom';
import {
type AbstractConstructor,
hostAttributes,
type SbbActionBaseElement,
} from '../../core/common-behaviors';
import { toggleDatasetEntry } from '../../core/dom';
import { AgnosticMutationObserver } from '../../core/observers';
import type { SbbCardElement } from '../card';

Expand All @@ -20,6 +24,9 @@ export const SbbCardActionCommonElementMixin = <
>(
superClass: T,
): AbstractConstructor<SbbCardActionCommonElementMixinType> & T => {
@hostAttributes({
slot: 'action',
})
abstract class SbbCardActionCommonElement
extends superClass
implements Partial<SbbCardActionCommonElementMixinType>
Expand Down Expand Up @@ -68,7 +75,6 @@ export const SbbCardActionCommonElementMixin = <
toggleDatasetEntry(this._card, 'hasAction', true);
toggleDatasetEntry(this._card, 'hasActiveAction', this.active);
this._card.dataset.actionRole = this.getAttribute('role')!;
setAttribute(this, 'slot', 'action');

this._checkForSlottedActions();
this._cardMutationObserver.observe(this._card, {
Expand Down
14 changes: 11 additions & 3 deletions src/components/core/common-behaviors/host-attributes.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import { isServer, type ReactiveElement } from 'lit';

import type { AbstractConstructor } from './constructor';

function applyAttributes(instance: ReactiveElement, attributes: Record<string, string>): void {
for (const [name, value] of Object.entries(attributes)) {
if (value) {
Expand All @@ -11,7 +13,7 @@ function applyAttributes(instance: ReactiveElement, attributes: Record<string, s
}

/**
* Applies the given attibutes to the related element.
* Applies the given attributes to the related element.
* If an empty string is passed as a value, the attribute will be set
* without value.
*
Expand All @@ -28,18 +30,24 @@ function applyAttributes(instance: ReactiveElement, attributes: Record<string, s
* @param attributes A record of attributes to apply to the element.
*/
export const hostAttributes =
(attributes: Record<string, string>) => (target: typeof ReactiveElement) =>
target.addInitializer((instance: ReactiveElement) => {
(attributes: Record<string, string>) => (target: AbstractConstructor<ReactiveElement>) =>
(target as typeof ReactiveElement).addInitializer((instance: ReactiveElement) => {
if (isServer) {
applyAttributes(instance, attributes);
} else {
// It is not allowed to set attributes during constructor phase. Due to this we use a
// controller to assign the attributes during lit initialization phase.
// HostConnected or hostUpdate can be called either order. Due to that we apply in the
// first occurrence and directly remove the controller, so it's only called once.
instance.addController({
hostConnected() {
applyAttributes(instance, attributes);
instance.removeController(this);
},
hostUpdate() {
applyAttributes(instance, attributes);
instance.removeController(this);
},
});
}
});
9 changes: 4 additions & 5 deletions src/components/timetable-row/timetable-row.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,9 @@ import type { CSSResultGroup, TemplateResult } from 'lit';
import { html, LitElement, nothing } from 'lit';
import { customElement, property } from 'lit/decorators.js';

import { hostAttributes, LanguageController } from '../core/common-behaviors';
import { LanguageController } from '../core/common-behaviors';
import { removeTimezoneFromISOTimeString, durationToTime } from '../core/datetime';
import { setAttribute } from '../core/dom';
import {
i18nArrival,
i18nClass,
Expand Down Expand Up @@ -269,10 +270,7 @@ export const handleNotices = (notices: Notice[]): Notice[] => {

/**
* It displays information about the trip, acting as a container for all the `sbb-timetable-*` components.
*/
@hostAttributes({
role: 'rowgroup',
})
* */
@customElement('sbb-timetable-row')
export class SbbTimetableRowElement extends LitElement {
public static override styles: CSSResultGroup = style;
Expand Down Expand Up @@ -546,6 +544,7 @@ export class SbbTimetableRowElement extends LitElement {
const noticeAttributes = notices && handleNotices(notices);

const durationObj = duration ? durationToTime(duration, this._language.current) : null;
setAttribute(this, 'role', 'rowgroup');

return html`
<sbb-card size="l" id=${id}>
Expand Down

0 comments on commit 75d7e4d

Please sign in to comment.