Skip to content

Commit

Permalink
refactor: use new listen and observes decorators
Browse files Browse the repository at this point in the history
  • Loading branch information
bennypowers committed Jul 9, 2024
1 parent 5ec3e01 commit 13d7511
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 29 deletions.
20 changes: 12 additions & 8 deletions elements/pf-accordion/pf-accordion.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { LitElement, html } from 'lit';
import { observed } from '@patternfly/pfe-core/decorators.js';
import { listen, observes } from '@patternfly/pfe-core/decorators.js';
import { property } from 'lit/decorators/property.js';
import { customElement } from 'lit/decorators/custom-element.js';

Expand Down Expand Up @@ -114,9 +114,6 @@ export class PfAccordion extends LitElement {
@property({ type: Boolean, reflect: true }) bordered = false;

/** Whether to apply the `large` style variant */
@observed(function largeChanged(this: PfAccordion) {
[...this.headers, ...this.panels].forEach(el => el.toggleAttribute('large', this.large));
})
@property({ type: Boolean, reflect: true }) large = false;

@property({ type: Boolean, reflect: true }) fixed = false;
Expand Down Expand Up @@ -183,7 +180,6 @@ export class PfAccordion extends LitElement {

connectedCallback() {
super.connectedCallback();
this.addEventListener('change', this.#onChange as EventListener);
this.#mo.observe(this, { childList: true });
this.#init();
}
Expand Down Expand Up @@ -221,6 +217,13 @@ export class PfAccordion extends LitElement {
return c && results.every(Boolean);
}

@observes('large')
protected largeChanged() {
for (const el of [...this.headers, ...this.panels]) {
el.toggleAttribute('large', this.large);
}
}

/**
* Initialize the accordion by connecting headers and panels
* with aria controls and labels; set up the default disclosure
Expand Down Expand Up @@ -280,7 +283,8 @@ export class PfAccordion extends LitElement {
panel.hidden = true;
}

#onChange(event: PfAccordionHeaderChangeEvent) {
@listen('change')
protected onChange(event: PfAccordionHeaderChangeEvent) {
if (event instanceof PfAccordionHeaderChangeEvent && event.accordion === this) {
const index = this.#getIndex(event.target);
if (event.expanded) {
Expand Down Expand Up @@ -358,8 +362,8 @@ export class PfAccordion extends LitElement {
}

// If the header and panel exist, open both
this.#expandHeader(header, index),
this.#expandPanel(panel),
this.#expandHeader(header, index);
this.#expandPanel(panel);

header.focus();

Expand Down
19 changes: 9 additions & 10 deletions elements/pf-jump-links/pf-jump-links-item.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { InternalsController } from '@patternfly/pfe-core/controllers/internals-

import style from './pf-jump-links-item.css';

import { observed } from '@patternfly/pfe-core/decorators/observed.js';
import { observes } from '@patternfly/pfe-core/decorators/observes.js';

/**
* @cssprop --pf-c-jump-links__link--PaddingTop -- padding around the link
Expand All @@ -28,29 +28,28 @@ export class PfJumpLinksItem extends LitElement {
};

/** Whether this item is active. */
@observed('activeChanged')
@property({ type: Boolean, reflect: true }) active = false;

/** hypertext reference for this link */
@property({ reflect: true }) href?: string;

#internals = InternalsController.of(this, { role: 'listitem' });

override connectedCallback() {
super.connectedCallback();
this.activeChanged();
}
#internals = InternalsController.of(this, {
role: 'listitem',
});

render() {
return html`
<a href="${ifDefined(this.href)}" @focus="${this.#onFocus}" @click="${this.#onClick}">
<a href="${ifDefined(this.href)}"
@focus="${this.#onFocus}"
@click="${this.#onClick}">
<slot></slot>
</a>
<slot name="subsection"></slot>
`;
}

private activeChanged() {
@observes('active')
protected activeChanged() {
this.#internals.ariaCurrent = this.active ? 'location' : null;

Check failure on line 53 in elements/pf-jump-links/pf-jump-links-item.ts

View workflow job for this annotation

GitHub Actions / SSR Tests (Playwright)

elements/pf-jump-links/test/pf-jump-links.e2e.ts:14:3 › pf-jump-links › ssr

2) elements/pf-jump-links/test/pf-jump-links.e2e.ts:14:3 › pf-jump-links › ssr ─────────────────── Error: TypeError: Cannot read private member from an object whose class did not declare it at elements/pf-jump-links/pf-jump-links-item.ts:53 51 | @observes('active') 52 | protected activeChanged() { > 53 | this.#internals.ariaCurrent = this.active ? 'location' : null; | ^ 54 | } 55 | 56 | #onClick() { at __classPrivateFieldGet (/__w/patternfly-elements/patternfly-elements/node_modules/tslib/tslib.js:327:98) at PfJumpLinksItem.call [as activeChanged] (file:///__w/patternfly-elements/patternfly-elements/elements/pf-jump-links/pf-jump-links-item.ts:53:5) at PfJumpLinksItem.set [as active] (file:///__w/patternfly-elements/patternfly-elements/core/pfe-core/decorators/observes.ts:24:16) at new PfJumpLinksItem (file:///__w/patternfly-elements/patternfly-elements/elements/pf-jump-links/pf-jump-links-item.ts:31:53) at new LitElementRenderer (file:///__w/patternfly-elements/patternfly-elements/node_modules/@lit-labs/ssr/src/lib/lit-element-renderer.ts:35:20) at getElementRenderer (file:///__w/patternfly-elements/patternfly-elements/node_modules/@lit-labs/ssr/src/lib/element-renderer.ts:41:14) at renderTemplateResult (file:///__w/patternfly-elements/patternfly-elements/node_modules/@lit-labs/ssr/src/lib/render-value.ts:781:26) at renderTemplateResult.next (<anonymous>) at renderValue (file:///__w/patternfly-elements/patternfly-elements/node_modules/@lit-labs/ssr/src/lib/render-value.ts:652:12) at renderValue.next (<anonymous>) at renderTemplateResult (file:///__w/patternfly-elements/patternfly-elements/node_modules/@lit-labs/ssr/src/lib/render-value.ts:727:16) at renderTemplateResult.next (<anonymous>) at renderValue (file:///__w/patternfly-elements/patternfly-elements/node_modules/@lit-labs/ssr/src/lib/render-value.ts:652:12) at renderValue.next (<anonymous>) at render (file:///__w/patternfly-elements/patternfly-elements/node_modules/@lit-labs/ssr/src/lib/render.ts:44:10) at render.next (<anonymous>) at collectResult (file:///__w/patternfly-elements/patternfly-elements/node_modules/@lit-labs/ssr/src/lib/render-result.ts:27:14) at ssr (file:///__w/patternfly-elements/patternfly-elements/tools/pfe-tools/ssr/ssr.ts:7:10) at renderGlobal (file:///__w/patternfly-elements/patternfly-elements/tools/pfe-tools/ssr/global.ts:14:10) at file:///__w/patternfly-elements/patternfly-elements/tools/pfe-tools/test/playwright/SSRPage.ts:43:33 expect(received).toEqual(expected) // deep equality Expected: 200 Received: 500 at __classPrivateFieldGet (/__w/patternfly-elements/patternfly-elements/node_modules/tslib/tslib.js:327:98) at PfJumpLinksItem.call (/__w/patternfly-elements/patternfly-elements/elements/pf-jump-links/pf-jump-links-item.ts:53:5) at PfJumpLinksItem.set (/__w/patternfly-elements/patternfly-elements/core/pfe-core/decorators/observes.ts:24:16) at PfJumpLinksItem (/__w/patternfly-elements/patternfly-elements/elements/pf-jump-links/pf-jump-links-item.ts:31:53) at LitElementRenderer (/__w/patternfly-elements/patternfly-elements/node_modules/@lit-labs/ssr/src/lib/lit-element-renderer.ts:35:20) at getElementRenderer (/__w/patternfly-elements/patternfly-elements/node_modules/@lit-labs/ssr/src/lib/element-renderer.ts:41:14) at renderTemplateResult (/__w/patternfly-elements/patternfly-elements/node_modules/@lit-labs/ssr/src/lib/render-value.ts:781:26) at renderValue (/__w/patternfly-elements/patternfly-elements/node_modules/@lit-labs/ssr/src/lib/render-value.ts:652:12) at renderTemplateResult (/__w/patternfly-elements/patternfly-elements/node_modules/@lit-labs/ssr/src/lib/render-value.ts:727:16) at renderValue (/__w/patternfly-elements/patternfly-elements/node_modules/@lit-labs/ssr/src/lib/render-value.ts:652:12) at
}

Expand Down
12 changes: 6 additions & 6 deletions elements/pf-modal/pf-modal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { ifDefined } from 'lit/directives/if-defined.js';
import { classMap } from 'lit/directives/class-map.js';

import { ComposedEvent } from '@patternfly/pfe-core';
import { bound, initializer, observed } from '@patternfly/pfe-core/decorators.js';
import { bound, initializer, observes } from '@patternfly/pfe-core/decorators.js';
import { getRandomId } from '@patternfly/pfe-core/functions/random.js';

import { SlotController } from '@patternfly/pfe-core/controllers/slot-controller.js';
Expand Down Expand Up @@ -67,7 +67,7 @@ export class ModalOpenEvent extends ComposedEvent {
*/
@customElement('pf-modal')
export class PfModal extends LitElement implements HTMLDialogElement {
static override readonly shadowRootOptions = {
static override readonly shadowRootOptions: ShadowRootInit = {
...LitElement.shadowRootOptions,
delegatesFocus: true,
};
Expand All @@ -88,11 +88,9 @@ export class PfModal extends LitElement implements HTMLDialogElement {
*/
@property({ reflect: true }) position?: 'top';

@observed
@property({ type: Boolean, reflect: true }) open = false;

/** Optional ID of the trigger element */
@observed
@property() trigger?: string;

/** @see https://developer.mozilla.org/en-US/docs/Web/API/HTMLDialogElement/returnValue */
Expand Down Expand Up @@ -190,7 +188,8 @@ export class PfModal extends LitElement implements HTMLDialogElement {
}
}

protected async _openChanged(oldValue?: boolean, newValue?: boolean) {
@observes('open')
protected async openChanged(oldValue?: boolean, newValue?: boolean) {
// loosening types to prevent running these effects in unexpected circumstances
// eslint-disable-next-line eqeqeq
if (oldValue == null || newValue == null || oldValue == newValue) {
Expand All @@ -216,7 +215,8 @@ export class PfModal extends LitElement implements HTMLDialogElement {
}
}

protected _triggerChanged() {
@observes('trigger')
protected triggerChanged() {
if (this.trigger) {
this.#triggerElement = (this.getRootNode() as Document | ShadowRoot)

Check failure on line 221 in elements/pf-modal/pf-modal.ts

View workflow job for this annotation

GitHub Actions / SSR Tests (Playwright)

elements/pf-modal/test/pf-modal.e2e.ts:18:3 › pf-modal › ssr

3) elements/pf-modal/test/pf-modal.e2e.ts:18:3 › pf-modal › ssr ────────────────────────────────── Error: TypeError: this.getRootNode is not a function at elements/pf-modal/pf-modal.ts:221 219 | protected triggerChanged() { 220 | if (this.trigger) { > 221 | this.#triggerElement = (this.getRootNode() as Document | ShadowRoot) | ^ 222 | .getElementById(this.trigger); 223 | this.#triggerElement?.addEventListener('click', this.onTriggerClick); 224 | } at PfModal.call [as triggerChanged] (file:///__w/patternfly-elements/patternfly-elements/elements/pf-modal/pf-modal.ts:221:36) at PfModal.propName [as trigger] (file:///__w/patternfly-elements/patternfly-elements/core/pfe-core/decorators/observes.ts:24:16) at PfModal._$attributeToProperty [as _$AK] (file:///__w/patternfly-elements/patternfly-elements/node_modules/@lit/reactive-element/src/reactive-element.ts:1218:12) at _$AK (file:///__w/patternfly-elements/patternfly-elements/node_modules/lit-element/src/lit-element.ts:272:17) at LitElementRenderer.attributeChangedCallback (file:///__w/patternfly-elements/patternfly-elements/node_modules/@lit-labs/ssr/src/lib/lit-element-renderer.ts:84:5) at LitElementRenderer.setAttribute (file:///__w/patternfly-elements/patternfly-elements/node_modules/@lit-labs/ssr/src/lib/element-renderer.ts:153:12) at renderTemplateResult (file:///__w/patternfly-elements/patternfly-elements/node_modules/@lit-labs/ssr/src/lib/render-value.ts:789:20) at renderTemplateResult.next (<anonymous>) at renderValue (file:///__w/patternfly-elements/patternfly-elements/node_modules/@lit-labs/ssr/src/lib/render-value.ts:652:12) at renderValue.next (<anonymous>) at renderTemplateResult (file:///__w/patternfly-elements/patternfly-elements/node_modules/@lit-labs/ssr/src/lib/render-value.ts:727:16) at renderTemplateResult.next (<anonymous>) at renderValue (file:///__w/patternfly-elements/patternfly-elements/node_modules/@lit-labs/ssr/src/lib/render-value.ts:652:12) at renderValue.next (<anonymous>) at render (file:///__w/patternfly-elements/patternfly-elements/node_modules/@lit-labs/ssr/src/lib/render.ts:44:10) at render.next (<anonymous>) at collectResult (file:///__w/patternfly-elements/patternfly-elements/node_modules/@lit-labs/ssr/src/lib/render-result.ts:27:14) at ssr (file:///__w/patternfly-elements/patternfly-elements/tools/pfe-tools/ssr/ssr.ts:7:10) at renderGlobal (file:///__w/patternfly-elements/patternfly-elements/tools/pfe-tools/ssr/global.ts:14:10) at file:///__w/patternfly-elements/patternfly-elements/tools/pfe-tools/test/playwright/SSRPage.ts:43:33 expect(received).toEqual(expected) // deep equality Expected: 200 Received: 500 at PfModal.call (/__w/patternfly-elements/patternfly-elements/elements/pf-modal/pf-modal.ts:221:36) at PfModal.propName (/__w/patternfly-elements/patternfly-elements/core/pfe-core/decorators/observes.ts:24:16) at PfModal._$attributeToProperty (/__w/patternfly-elements/patternfly-elements/node_modules/@lit/reactive-element/src/reactive-element.ts:1218:12) at _$AK (/__w/patternfly-elements/patternfly-elements/node_modules/lit-element/src/lit-element.ts:272:17) at LitElementRenderer.attributeChangedCallback (/__w/patternfly-elements/patternfly-elements/node_modules/@lit-labs/ssr/src/lib/lit-element-renderer.ts:84:5) at LitElementRenderer.setAttribute (/__w/patternfly-elements/patternfly-elements/node_modules/@lit-labs/ssr/src/lib/element-renderer.ts:153:12) at renderTemplateResult (/__w/patternfly-elements/patternfly-elements/node_modules/@lit-labs/ssr/src/lib/render-value.ts:789:20) at renderValue (/__w/patternfly-elements/patternfly-elements/node_modules/@lit-labs/ssr/src/lib/render-value.ts:652:12) at renderTemplateResult (/__w/patternfly-elements/patternfly-elements/n
.getElementById(this.trigger);
Expand Down
10 changes: 5 additions & 5 deletions elements/pf-tabs/pf-tab.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { queryAssignedElements } from 'lit/decorators/query-assigned-elements.js
import { classMap } from 'lit/directives/class-map.js';
import { consume } from '@lit/context';

import { observed } from '@patternfly/pfe-core/decorators.js';
import { observes } from '@patternfly/pfe-core/decorators.js';
import { getRandomId } from '@patternfly/pfe-core/functions/random.js';

import { InternalsController } from '@patternfly/pfe-core/controllers/internals-controller.js';
Expand Down Expand Up @@ -65,10 +65,8 @@ export class PfTab extends LitElement {
@queryAssignedElements({ slot: 'icon', flatten: true })
private icons!: HTMLElement[];

@observed
@property({ reflect: true, type: Boolean }) active = false;

@observed
@property({ reflect: true, type: Boolean }) disabled = false;

@consume({ context, subscribe: true })
Expand Down Expand Up @@ -144,14 +142,16 @@ export class PfTab extends LitElement {
return this.dispatchEvent(new TabExpandEvent(this));
}

private _activeChanged(old: boolean) {
@observes('active')
protected activeChanged(old: boolean) {
this.#internals.ariaSelected = String(!!this.active);
if (this.active && !old) {
this.#activate();
}
}

private _disabledChanged() {
@observes('disabled')
protected disabledChanged() {
this.#internals.ariaDisabled = this.disabled ? 'true' : this.ariaDisabled ?? 'false';
}
}
Expand Down

0 comments on commit 13d7511

Please sign in to comment.