diff --git a/projects/core/custom-elements.json b/projects/core/custom-elements.json index d4e701a88..06b0ad97b 100644 --- a/projects/core/custom-elements.json +++ b/projects/core/custom-elements.json @@ -1326,7 +1326,6 @@ "type": { "text": "boolean" }, - "default": "false", "attribute": "disabled", "inheritedFrom": { "name": "CdsBaseButton", @@ -1344,6 +1343,19 @@ "name": "CdsBaseButton", "module": "internal/base/button.base.js" } + }, + { + "kind": "field", + "name": "_disabled", + "type": { + "text": "boolean" + }, + "privacy": "private", + "default": "false", + "inheritedFrom": { + "name": "CdsBaseButton", + "module": "internal/base/button.base.js" + } } ], "attributes": [ @@ -1439,7 +1451,6 @@ "type": { "text": "boolean" }, - "default": "false", "fieldName": "disabled", "inheritedFrom": { "name": "CdsBaseButton", @@ -1665,7 +1676,6 @@ "type": { "text": "boolean" }, - "default": "false", "attribute": "disabled", "inheritedFrom": { "name": "CdsBaseButton", @@ -1683,6 +1693,19 @@ "name": "CdsBaseButton", "module": "internal/base/button.base.js" } + }, + { + "kind": "field", + "name": "_disabled", + "type": { + "text": "boolean" + }, + "privacy": "private", + "default": "false", + "inheritedFrom": { + "name": "CdsBaseButton", + "module": "internal/base/button.base.js" + } } ], "attributes": [ @@ -1791,7 +1814,6 @@ "type": { "text": "boolean" }, - "default": "false", "fieldName": "disabled", "inheritedFrom": { "name": "CdsBaseButton", @@ -2003,7 +2025,6 @@ "type": { "text": "boolean" }, - "default": "false", "attribute": "disabled", "inheritedFrom": { "name": "CdsBaseButton", @@ -2021,6 +2042,19 @@ "name": "CdsBaseButton", "module": "internal/base/button.base.js" } + }, + { + "kind": "field", + "name": "_disabled", + "type": { + "text": "boolean" + }, + "privacy": "private", + "default": "false", + "inheritedFrom": { + "name": "CdsBaseButton", + "module": "internal/base/button.base.js" + } } ], "superclass": { @@ -2134,7 +2168,6 @@ "type": { "text": "boolean" }, - "default": "false", "fieldName": "disabled", "inheritedFrom": { "name": "CdsBaseButton", @@ -2305,7 +2338,6 @@ "type": { "text": "boolean" }, - "default": "false", "attribute": "disabled", "inheritedFrom": { "name": "CdsBaseButton", @@ -2323,6 +2355,19 @@ "name": "CdsBaseButton", "module": "internal/base/button.base.js" } + }, + { + "kind": "field", + "name": "_disabled", + "type": { + "text": "boolean" + }, + "privacy": "private", + "default": "false", + "inheritedFrom": { + "name": "CdsBaseButton", + "module": "internal/base/button.base.js" + } } ], "superclass": { @@ -2403,7 +2448,6 @@ "type": { "text": "boolean" }, - "default": "false", "fieldName": "disabled", "inheritedFrom": { "name": "CdsBaseButton", @@ -2503,7 +2547,6 @@ "type": { "text": "boolean" }, - "default": "false", "fieldName": "disabled", "inheritedFrom": { "name": "CdsBaseButton", @@ -2613,7 +2656,6 @@ "type": { "text": "boolean" }, - "default": "false", "attribute": "disabled", "inheritedFrom": { "name": "CdsBaseButton", @@ -2631,6 +2673,19 @@ "name": "CdsBaseButton", "module": "internal/base/button.base.js" } + }, + { + "kind": "field", + "name": "_disabled", + "type": { + "text": "boolean" + }, + "privacy": "private", + "default": "false", + "inheritedFrom": { + "name": "CdsBaseButton", + "module": "internal/base/button.base.js" + } } ] } @@ -2839,7 +2894,6 @@ "type": { "text": "boolean" }, - "default": "false", "attribute": "disabled", "inheritedFrom": { "name": "CdsBaseButton", @@ -2857,6 +2911,19 @@ "name": "CdsBaseButton", "module": "internal/base/button.base.js" } + }, + { + "kind": "field", + "name": "_disabled", + "type": { + "text": "boolean" + }, + "privacy": "private", + "default": "false", + "inheritedFrom": { + "name": "CdsBaseButton", + "module": "internal/base/button.base.js" + } } ], "attributes": [ @@ -2972,7 +3039,6 @@ "type": { "text": "boolean" }, - "default": "false", "fieldName": "disabled", "inheritedFrom": { "name": "CdsBaseButton", @@ -3172,12 +3238,15 @@ }, { "kind": "field", - "name": "_disabled", + "name": "disabled", "type": { "text": "boolean" }, - "privacy": "private", - "default": "false" + "attribute": "disabled", + "inheritedFrom": { + "name": "CdsBaseButton", + "module": "internal/base/button.base.js" + } }, { "kind": "method", @@ -3198,9 +3267,18 @@ }, { "kind": "method", - "name": "enableButton", + "name": "restoreButton", "privacy": "private" }, + { + "kind": "field", + "name": "_disabledExternally", + "type": { + "text": "boolean" + }, + "privacy": "private", + "default": "false" + }, { "kind": "field", "name": "pressed", @@ -3275,12 +3353,11 @@ }, { "kind": "field", - "name": "disabled", + "name": "popup", "type": { - "text": "boolean" + "text": "string" }, - "default": "false", - "attribute": "disabled", + "attribute": "popup", "inheritedFrom": { "name": "CdsBaseButton", "module": "internal/base/button.base.js" @@ -3288,11 +3365,12 @@ }, { "kind": "field", - "name": "popup", + "name": "_disabled", "type": { - "text": "string" + "text": "boolean" }, - "attribute": "popup", + "privacy": "private", + "default": "false", "inheritedFrom": { "name": "CdsBaseButton", "module": "internal/base/button.base.js" @@ -3344,6 +3422,17 @@ "description": "`default`: shows the content of the button\n- `loading`: disables the button and shows a spinner inside the button\n- `success`: disables the button and shows a check mark inside the button; auto-triggers to change back to DEFAULT state after 1000 ms\n- `error`: shows the content of the button (in the context of application, this state is usually entered from a LOADING state. the application should show appropriate error message)", "fieldName": "loadingState" }, + { + "name": "disabled", + "type": { + "text": "boolean" + }, + "fieldName": "disabled", + "inheritedFrom": { + "name": "CdsBaseButton", + "module": "internal/base/button.base.js" + } + }, { "name": "pressed", "type": { @@ -3410,18 +3499,6 @@ "module": "internal/base/button.base.js" } }, - { - "name": "disabled", - "type": { - "text": "boolean" - }, - "default": "false", - "fieldName": "disabled", - "inheritedFrom": { - "name": "CdsBaseButton", - "module": "internal/base/button.base.js" - } - }, { "name": "popup", "type": { @@ -3568,15 +3645,14 @@ }, { "kind": "field", - "name": "_disabled", + "name": "disabled", "type": { "text": "boolean" }, - "privacy": "private", - "default": "false", + "attribute": "disabled", "inheritedFrom": { - "name": "CdsButton", - "module": "button/button.element.js" + "name": "CdsBaseButton", + "module": "internal/base/button.base.js" } }, { @@ -3606,8 +3682,21 @@ }, { "kind": "method", - "name": "enableButton", + "name": "restoreButton", + "privacy": "private", + "inheritedFrom": { + "name": "CdsButton", + "module": "button/button.element.js" + } + }, + { + "kind": "field", + "name": "_disabledExternally", + "type": { + "text": "boolean" + }, "privacy": "private", + "default": "false", "inheritedFrom": { "name": "CdsButton", "module": "button/button.element.js" @@ -3687,12 +3776,11 @@ }, { "kind": "field", - "name": "disabled", + "name": "popup", "type": { - "text": "boolean" + "text": "string" }, - "default": "false", - "attribute": "disabled", + "attribute": "popup", "inheritedFrom": { "name": "CdsBaseButton", "module": "internal/base/button.base.js" @@ -3700,11 +3788,12 @@ }, { "kind": "field", - "name": "popup", + "name": "_disabled", "type": { - "text": "string" + "text": "boolean" }, - "attribute": "popup", + "privacy": "private", + "default": "false", "inheritedFrom": { "name": "CdsBaseButton", "module": "internal/base/button.base.js" @@ -3782,6 +3871,17 @@ "module": "button/button.element.js" } }, + { + "name": "disabled", + "type": { + "text": "boolean" + }, + "fieldName": "disabled", + "inheritedFrom": { + "name": "CdsBaseButton", + "module": "internal/base/button.base.js" + } + }, { "name": "pressed", "type": { @@ -3848,18 +3948,6 @@ "module": "internal/base/button.base.js" } }, - { - "name": "disabled", - "type": { - "text": "boolean" - }, - "default": "false", - "fieldName": "disabled", - "inheritedFrom": { - "name": "CdsBaseButton", - "module": "internal/base/button.base.js" - } - }, { "name": "popup", "type": { @@ -8033,7 +8121,6 @@ "type": { "text": "boolean" }, - "default": "false", "attribute": "disabled", "inheritedFrom": { "name": "CdsBaseButton", @@ -8051,6 +8138,19 @@ "name": "CdsBaseButton", "module": "internal/base/button.base.js" } + }, + { + "kind": "field", + "name": "_disabled", + "type": { + "text": "boolean" + }, + "privacy": "private", + "default": "false", + "inheritedFrom": { + "name": "CdsBaseButton", + "module": "internal/base/button.base.js" + } } ], "attributes": [ @@ -8158,7 +8258,6 @@ "type": { "text": "boolean" }, - "default": "false", "fieldName": "disabled", "inheritedFrom": { "name": "CdsBaseButton", @@ -36298,7 +36397,6 @@ "type": { "text": "boolean" }, - "default": "false", "attribute": "disabled", "inheritedFrom": { "name": "CdsBaseButton", @@ -36316,6 +36414,19 @@ "name": "CdsBaseButton", "module": "internal/base/button.base.js" } + }, + { + "kind": "field", + "name": "_disabled", + "type": { + "text": "boolean" + }, + "privacy": "private", + "default": "false", + "inheritedFrom": { + "name": "CdsBaseButton", + "module": "internal/base/button.base.js" + } } ], "attributes": [ @@ -36424,7 +36535,6 @@ "type": { "text": "boolean" }, - "default": "false", "fieldName": "disabled", "inheritedFrom": { "name": "CdsBaseButton", @@ -37933,7 +38043,6 @@ "type": { "text": "boolean" }, - "default": "false", "attribute": "disabled" }, { @@ -37943,6 +38052,15 @@ "text": "string" }, "attribute": "popup" + }, + { + "kind": "field", + "name": "_disabled", + "type": { + "text": "boolean" + }, + "privacy": "private", + "default": "false" } ], "attributes": [ @@ -37993,7 +38111,6 @@ "type": { "text": "boolean" }, - "default": "false", "fieldName": "disabled" }, { @@ -50355,7 +50472,6 @@ "type": { "text": "boolean" }, - "default": "false", "attribute": "disabled", "inheritedFrom": { "name": "CdsBaseButton", @@ -50492,6 +50608,19 @@ "name": "CdsBaseButton", "module": "internal/base/button.base.js" } + }, + { + "kind": "field", + "name": "_disabled", + "type": { + "text": "boolean" + }, + "privacy": "private", + "default": "false", + "inheritedFrom": { + "name": "CdsBaseButton", + "module": "internal/base/button.base.js" + } } ], "attributes": [ @@ -50600,7 +50729,6 @@ "type": { "text": "boolean" }, - "default": "false", "fieldName": "disabled", "inheritedFrom": { "name": "CdsBaseButton", @@ -57347,7 +57475,6 @@ "type": { "text": "boolean" }, - "default": "false", "attribute": "disabled", "inheritedFrom": { "name": "CdsBaseButton", @@ -57365,6 +57492,19 @@ "name": "CdsBaseButton", "module": "internal/base/button.base.js" } + }, + { + "kind": "field", + "name": "_disabled", + "type": { + "text": "boolean" + }, + "privacy": "private", + "default": "false", + "inheritedFrom": { + "name": "CdsBaseButton", + "module": "internal/base/button.base.js" + } } ], "attributes": [ @@ -57465,7 +57605,6 @@ "type": { "text": "boolean" }, - "default": "false", "fieldName": "disabled", "inheritedFrom": { "name": "CdsBaseButton", diff --git a/projects/core/src/button/button.element.spec.ts b/projects/core/src/button/button.element.spec.ts index 09da8ac92..ed2f3c426 100644 --- a/projects/core/src/button/button.element.spec.ts +++ b/projects/core/src/button/button.element.spec.ts @@ -279,7 +279,7 @@ describe('button element', () => { expect(component.disabled).not.toBeTruthy(); }); - it('should stay disabled when loadingState changes to default', async () => { + it('should stay disabled when loadingState changes back to default', async () => { await componentIsStable(component); component.disabled = true; component.loadingState = ClrLoadingState.default; @@ -298,6 +298,25 @@ describe('button element', () => { expect(component.disabled).toBeTruthy(); }); + it('should return to disabled state when set in non-default loadingState status', async () => { + await componentIsStable(component); + component.loadingState = ClrLoadingState.default; + await componentIsStable(component); + + component.loadingState = ClrLoadingState.loading; + await componentIsStable(component); + expect(component.disabled).toBeTruthy(); + + component.loadingState = ClrLoadingState.success; + component.disabled = true; + await componentIsStable(component); + expect(component.disabled).toBeTruthy(); + + component.loadingState = ClrLoadingState.default; + await componentIsStable(component); + expect(component.disabled).toBeTruthy(); + }); + it('should default to spinner size to "18"', async () => { component.loadingState = ClrLoadingState.loading; await componentIsStable(component); @@ -310,6 +329,25 @@ describe('button element', () => { await componentIsStable(component); expect(component.shadowRoot.querySelector('cds-progress-circle').size).toBe('12'); }); + + it('should not have hardcoded width on initial render', async () => { + // the width is only hardcoded when switching from default (with button text) to a loading status (with icon) + const testElement2 = await createTestElement(html` +
+ + ${placeholderText} + +
+ `); + + const component2 = testElement2.querySelector('cds-button'); + await componentIsStable(component2); + + expect(component2.style.width).toEqual(''); + expect(component2.getBoundingClientRect().width).toBeGreaterThan(12); + + removeTestElement(testElement2); + }); }); }); diff --git a/projects/core/src/button/button.element.ts b/projects/core/src/button/button.element.ts index 2bd94e7bc..722417fd7 100644 --- a/projects/core/src/button/button.element.ts +++ b/projects/core/src/button/button.element.ts @@ -83,6 +83,16 @@ export class CdsButton extends CdsBaseButton { @property({ type: String }) loadingState: keyof typeof ClrLoadingState = ClrLoadingState.default; + @property({ type: Boolean }) + get disabled(): boolean { + return super.disabled; + } + + set disabled(value: boolean) { + this._disabledExternally = value; + super.disabled = value; + } + firstUpdated(props: PropertyValues) { super.firstUpdated(props); @@ -92,26 +102,17 @@ export class CdsButton extends CdsBaseButton { } update(props: PropertyValues) { - if (props.has('loadingState')) { - if (this.isDefaultLoadingState(props.get('loadingState') as string)) { - // track prior disabled state to set prior value after button is re-enabled from a loading state - this._disabled = this.disabled; - } - - if (props.get('loadingState') !== undefined) { - if (this.isDefaultLoadingState(this.loadingState)) { - this.enableButton(); - } else { - this.disableButton(); - } + if (props.has('loadingState') && props.get('loadingState') !== undefined) { + if (this.isDefaultLoadingState(this.loadingState)) { + this.restoreButton(); + } else { + this.disableButton(); } } super.update(props); } - private _disabled = false; - render() { return html`
${this.loadingState === ClrLoadingState.success @@ -135,11 +136,16 @@ export class CdsButton extends CdsBaseButton { private disableButton() { this.style.width = getElementWidth(this); - this.disabled = true; + super.disabled = true; } - private enableButton() { + private restoreButton() { this.style.removeProperty('width'); - this.disabled = this._disabled; + super.disabled = this._disabledExternally; } + + // when the loading state changes, the disabled state should be set to what the consumer manually set, if set + // the setter here should never be called in the component, call super instead + // https://github.com/vmware-clarity/core/issues/129 + private _disabledExternally = false; } diff --git a/projects/core/src/internal/base/button.base.ts b/projects/core/src/internal/base/button.base.ts index 43338181d..56929acb0 100644 --- a/projects/core/src/internal/base/button.base.ts +++ b/projects/core/src/internal/base/button.base.ts @@ -39,7 +39,18 @@ export class CdsBaseButton extends LitElement { @property({ type: String }) value: string; - @property({ type: Boolean }) disabled = false; + @property({ type: Boolean }) + get disabled(): boolean { + return this._disabled; + } + + set disabled(value: boolean) { + const oldValue = this._disabled; + this._disabled = value; + this.requestUpdate('disabled', oldValue); + } @property({ type: String }) popup: string; + + private _disabled = false; } diff --git a/projects/react/src/button/__snapshots__/index.test.tsx.snap b/projects/react/src/button/__snapshots__/index.test.tsx.snap index e6a3acabb..b617847c4 100644 --- a/projects/react/src/button/__snapshots__/index.test.tsx.snap +++ b/projects/react/src/button/__snapshots__/index.test.tsx.snap @@ -18,6 +18,7 @@ exports[`CdsButton snapshot 1`] = ` danger disabled diff --git a/projects/react/src/button/index.test.tsx b/projects/react/src/button/index.test.tsx index 3e6249ec4..b7a371f6c 100644 --- a/projects/react/src/button/index.test.tsx +++ b/projects/react/src/button/index.test.tsx @@ -17,7 +17,9 @@ describe('CdsButton', () => { expect(await screen.findByRole('button', { name: 'primary' })).toHaveAttribute('status', 'primary'); expect(await screen.findByRole('button', { name: 'success' })).toHaveAttribute('status', 'success'); - expect(await screen.findByRole('button', { name: 'disabled' })).toHaveAttribute('disabled', ''); + // There's a lit issue that is resulting in this being 'true' instead of '' + // https://github.com/lit/lit/issues/2799#issuecomment-1203178300 + expect(await screen.findByRole('button', { name: 'disabled' })).toHaveAttribute('disabled', 'true'); }); it('snapshot', () => { diff --git a/projects/react/src/pagination/__snapshots__/index.test.tsx.snap b/projects/react/src/pagination/__snapshots__/index.test.tsx.snap index 899025e81..8485daa0c 100644 --- a/projects/react/src/pagination/__snapshots__/index.test.tsx.snap +++ b/projects/react/src/pagination/__snapshots__/index.test.tsx.snap @@ -8,6 +8,7 @@ exports[`CdsPagination snapshot 1`] = `