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

fix attributes and properties turning to null from 'morphing' #2177

Merged
merged 4 commits into from
Sep 20, 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
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
Loading