Skip to content

Commit

Permalink
fix attributes and properties turning to null from 'morphing' (#2177)
Browse files Browse the repository at this point in the history
* fix attributes and properties turning to null from 'morphing'

* add morphing tests

* prettier

* remove unnecessary console.log from button group test
  • Loading branch information
KonnorRogers authored Sep 20, 2024
1 parent 65126e8 commit 41ba672
Show file tree
Hide file tree
Showing 4 changed files with 94 additions and 1 deletion.
1 change: 1 addition & 0 deletions docs/pages/resources/changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ New versions of Shoelace are released as-needed and generally occur when a criti
- Scroll buttons for `<sl-tab-group>` auto hide when they are not clickable. The `fixed-scroll-controls` attribute can be included to prevent this behavior. [#2128]
- Added support for using `<sl-dropdown>` in `<sl-breadcrumb-item>` default slot [#2015]
- Added the `countdown` attribute to `<sl-alert>` to show a visual indicator before the toast disappears [#1899]
- Fixed a bug with morphing and DOM diffing that would cause elements with reflected initial attributes to not reset. [#2177]
- Fixed a bug that caused errors to show in the console when components disconnect before before `firstUpdated()` executes [#2127]
- Fixed a bug that made pagination work incorrectly in `<sl-carousel>` [#2155]
- Fixed a bug in `<sl-tab-group>` that caused the active tab indicator to be the wrong size when the tab's content changes [#2164]
Expand Down
1 change: 0 additions & 1 deletion src/components/button-group/button-group.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,6 @@ describe('<sl-button-group>', () => {

allButtons[0].dispatchEvent(new MouseEvent('mouseout', { bubbles: true }));
await elementUpdated(allButtons[0]);
console.log(allButtons[0]);
expect(allButtons[0]).to.not.have.attribute('data-sl-button-group__button--hover');
});
});
Expand Down
57 changes: 57 additions & 0 deletions src/components/button/button.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,63 @@ describe('<sl-button>', () => {
});
});
});
describe('when an attribute is removed', () => {
it("should return to 'default' when attribute removed with no initial attribute", async () => {
const el = await fixture<SlButton>(html`<sl-button>Button label</sl-button>`);

expect(el.variant).to.equal('default');
expect(el.getAttribute('variant')).to.equal('default');

el.removeAttribute('variant');
await el.updateComplete;

expect(el.variant).to.equal('default');
expect(el.getAttribute('variant')).to.equal('default');
});

it("should return to 'default' when attribute removed with an initial attribute", async () => {
const el = await fixture<SlButton>(html`<sl-button variant="primary">Button label</sl-button>`);

expect(el.variant).to.equal('primary');
expect(el.getAttribute('variant')).to.equal('primary');

el.removeAttribute('variant');
await el.updateComplete;

expect(el.variant).to.equal('default');
expect(el.getAttribute('variant')).to.equal('default');
});
});

describe('when a property is set to null', () => {
it("should return to 'default' when property set to null with no initial attribute", async () => {
const el = await fixture<SlButton>(html`<sl-button>Button label</sl-button>`);

expect(el.variant).to.equal('default');
expect(el.getAttribute('variant')).to.equal('default');

// @ts-expect-error Its a test. Stop.
el.variant = null;
await el.updateComplete;

expect(el.variant).to.equal('default');
expect(el.getAttribute('variant')).to.equal('default');
});

it("should return to 'default' when property set to null with an initial attribute", async () => {
const el = await fixture<SlButton>(html`<sl-button variant="primary">Button label</sl-button>`);

expect(el.variant).to.equal('primary');
expect(el.getAttribute('variant')).to.equal('primary');

// @ts-expect-error Its a test. Stop.
el.variant = null;
await el.updateComplete;

expect(el.variant).to.equal('default');
expect(el.getAttribute('variant')).to.equal('default');
});
});

describe('when provided no parameters', () => {
it('passes accessibility test', async () => {
Expand Down
36 changes: 36 additions & 0 deletions src/internal/shoelace-element.ts
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,42 @@ export default class ShoelaceElement extends LitElement {
(this.constructor as typeof ShoelaceElement).define(name, component);
});
}

#hasRecordedInitialProperties = false;

// Store the constructor value of all `static properties = {}`
initialReflectedProperties: Map<string, unknown> = new Map();

attributeChangedCallback(name: string, oldValue: string | null, newValue: string | null) {
if (!this.#hasRecordedInitialProperties) {
(this.constructor as typeof ShoelaceElement).elementProperties.forEach(
(obj, prop: keyof typeof this & string) => {
// eslint-disable-next-line
if (obj.reflect && this[prop] != null) {
this.initialReflectedProperties.set(prop, this[prop]);
}
}
);

this.#hasRecordedInitialProperties = true;
}

super.attributeChangedCallback(name, oldValue, newValue);
}

protected willUpdate(changedProperties: Parameters<LitElement['willUpdate']>[0]): void {
super.willUpdate(changedProperties);

// Run the morph fixing *after* willUpdate.
this.initialReflectedProperties.forEach((value, prop: string & keyof typeof this) => {
// If a prop changes to `null`, we assume this happens via an attribute changing to `null`.
// eslint-disable-next-line
if (changedProperties.has(prop) && this[prop] == null) {
// Silly type gymnastics to appease the compiler.
(this as Record<string, unknown>)[prop] = value;
}
});
}
}

export interface ShoelaceFormControl extends ShoelaceElement {
Expand Down

0 comments on commit 41ba672

Please sign in to comment.