Skip to content

Commit d56f4d3

Browse files
asynclizcopybara-github
authored andcommitted
fix(button,iconbutton): use form-associated mixin behavior
PiperOrigin-RevId: 876381068
1 parent 4142a69 commit d56f4d3

File tree

4 files changed

+36
-79
lines changed

4 files changed

+36
-79
lines changed

button/internal/button.ts

Lines changed: 4 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -16,24 +16,19 @@ import {
1616
dispatchActivationClick,
1717
isActivationClick,
1818
} from '../../internal/events/form-label-activation.js';
19-
import {
20-
internals,
21-
mixinElementInternals,
22-
} from '../../labs/behaviors/element-internals.js';
19+
import {mixinElementInternals} from '../../labs/behaviors/element-internals.js';
20+
import {mixinFormAssociated} from '../../labs/behaviors/form-associated.js';
2321
import {mixinFormSubmitter} from '../../labs/behaviors/form-submitter.js';
2422

2523
// Separate variable needed for closure.
2624
const buttonBaseClass = mixinDelegatesAria(
27-
mixinFormSubmitter(mixinElementInternals(LitElement)),
25+
mixinFormSubmitter(mixinFormAssociated(mixinElementInternals(LitElement))),
2826
);
2927

3028
/**
3129
* A button component.
3230
*/
3331
export abstract class Button extends buttonBaseClass {
34-
/** @nocollapse */
35-
static readonly formAssociated = true;
36-
3732
/** @nocollapse */
3833
static override shadowRootOptions: ShadowRootInit = {
3934
mode: 'open',
@@ -43,7 +38,7 @@ export abstract class Button extends buttonBaseClass {
4338
/**
4439
* Whether or not the button is disabled.
4540
*/
46-
@property({type: Boolean, reflect: true}) disabled = false;
41+
declare disabled: boolean; // for jsdoc until lit-analyzer is updated
4742

4843
/**
4944
* Whether or not the button is "soft-disabled" (disabled but still
@@ -89,13 +84,6 @@ export abstract class Button extends buttonBaseClass {
8984
@property({type: Boolean, attribute: 'has-icon', reflect: true}) hasIcon =
9085
false;
9186

92-
/**
93-
* The associated form element with which this element's value will submit.
94-
*/
95-
get form() {
96-
return this[internals].form;
97-
}
98-
9987
@query('.button') private readonly buttonElement!: HTMLElement | null;
10088

10189
@queryAssignedElements({slot: 'icon', flatten: true})

iconbutton/internal/icon-button.ts

Lines changed: 4 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -19,17 +19,15 @@ import {
1919
afterDispatch,
2020
setupDispatchHooks,
2121
} from '../../internal/events/dispatch-hooks.js';
22-
import {
23-
internals,
24-
mixinElementInternals,
25-
} from '../../labs/behaviors/element-internals.js';
22+
import {mixinElementInternals} from '../../labs/behaviors/element-internals.js';
23+
import {mixinFormAssociated} from '../../labs/behaviors/form-associated.js';
2624
import {mixinFormSubmitter} from '../../labs/behaviors/form-submitter.js';
2725

2826
type LinkTarget = '_blank' | '_parent' | '_self' | '_top';
2927

3028
// Separate variable needed for closure.
3129
const iconButtonBaseClass = mixinDelegatesAria(
32-
mixinFormSubmitter(mixinElementInternals(LitElement)),
30+
mixinFormSubmitter(mixinFormAssociated(mixinElementInternals(LitElement))),
3331
);
3432

3533
/**
@@ -40,9 +38,6 @@ const iconButtonBaseClass = mixinDelegatesAria(
4038
* @fires change {Event} Dispatched when a toggle button toggles --bubbles
4139
*/
4240
export class IconButton extends iconButtonBaseClass {
43-
/** @nocollapse */
44-
static readonly formAssociated = true;
45-
4641
/** @nocollapse */
4742
static override shadowRootOptions: ShadowRootInit = {
4843
mode: 'open',
@@ -52,7 +47,7 @@ export class IconButton extends iconButtonBaseClass {
5247
/**
5348
* Disables the icon button and makes it non-interactive.
5449
*/
55-
@property({type: Boolean, reflect: true}) disabled = false;
50+
declare disabled: boolean; // for jsdoc until lit-analyzer is updated
5651

5752
/**
5853
* "Soft-disables" the icon button (disabled but still focusable).
@@ -105,20 +100,6 @@ export class IconButton extends iconButtonBaseClass {
105100
*/
106101
@property({type: Boolean, reflect: true}) selected = false;
107102

108-
/**
109-
* The associated form element with which this element's value will submit.
110-
*/
111-
get form() {
112-
return this[internals].form;
113-
}
114-
115-
/**
116-
* The labels this element is associated with.
117-
*/
118-
get labels() {
119-
return this[internals].labels;
120-
}
121-
122103
@state() private flipIcon = isRtl(this, this.flipIconInRtl);
123104

124105
constructor() {

labs/behaviors/form-associated.ts

Lines changed: 14 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -46,15 +46,16 @@ export interface FormAssociated {
4646
disabled: boolean;
4747

4848
/**
49-
* Gets the current form value of a component.
49+
* Gets the current form value of a component. Defaults to the component's
50+
* `value` attribute.
5051
*
5152
* @return The current form value.
5253
*/
5354
[getFormValue](): FormValue | null;
5455

5556
/**
5657
* Gets the current form state of a component. Defaults to the component's
57-
* `[formValue]`.
58+
* `[getFormValue]()` result.
5859
*
5960
* Use this when the state of an element is different from its value, such as
6061
* checkboxes (internal boolean state and a user string value).
@@ -72,25 +73,26 @@ export interface FormAssociated {
7273
formDisabledCallback(disabled: boolean): void;
7374

7475
/**
75-
* A callback for when the form requests to reset its value. Typically, the
76-
* default value that is reset is represented in the attribute of an element.
76+
* An optional callback for when the form requests to reset its value.
77+
* Typically, the default value that is reset is represented in the attribute
78+
* of an element.
7779
*
7880
* This means the attribute used for the value should not update as the value
7981
* changes. For example, a checkbox should not change its default `checked`
8082
* attribute when selected. Ensure form values do not reflect.
8183
*/
82-
formResetCallback(): void;
84+
formResetCallback?(): void;
8385

8486
/**
85-
* A callback for when the form restores the state of a component. For
86-
* example, when a page is reloaded or forms are autofilled.
87+
* An optional callback for when the form restores the state of a component.
88+
* For example, when a page is reloaded or forms are autofilled.
8789
*
8890
* @param state The state to restore, or null to reset the form control's
8991
* value.
9092
* @param reason The reason state was restored, either `'restore'` or
9193
* `'autocomplete'`.
9294
*/
93-
formStateRestoreCallback(
95+
formStateRestoreCallback?(
9496
state: FormRestoreState | null,
9597
reason: FormRestoreReason,
9698
): void;
@@ -238,7 +240,9 @@ export function mixinFormAssociated<
238240
return this.hasAttribute('disabled');
239241
}
240242
set disabled(disabled: boolean) {
241-
this.toggleAttribute('disabled', disabled);
243+
// Coerce `disabled` in `Boolean()` to ensure that setting to `null` or
244+
// `undefined` sets the attribute to `false`.
245+
this.toggleAttribute('disabled', Boolean(disabled));
242246
// We don't need to call `requestUpdate()` since it's called synchronously
243247
// in `attributeChangedCallback()`.
244248
}
@@ -282,9 +286,7 @@ export function mixinFormAssociated<
282286
}
283287

284288
[getFormValue](): FormValue | null {
285-
// Closure does not allow abstract symbol members, so a default
286-
// implementation is needed.
287-
throw new Error('Implement [getFormValue]');
289+
return this.getAttribute('value');
288290
}
289291

290292
[getFormState](): FormValue | null {
@@ -294,13 +296,6 @@ export function mixinFormAssociated<
294296
formDisabledCallback(disabled: boolean) {
295297
this.disabled = disabled;
296298
}
297-
298-
abstract formResetCallback(): void;
299-
300-
abstract formStateRestoreCallback(
301-
state: FormRestoreState | null,
302-
reason: FormRestoreReason,
303-
): void;
304299
}
305300

306301
return FormAssociatedElement;

labs/behaviors/form-associated_test.ts

Lines changed: 14 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -217,27 +217,20 @@ describe('mixinFormAssociated()', () => {
217217
});
218218

219219
describe('[getFormValue]()', () => {
220-
it('should throw an error if not implemented', () => {
221-
expect(() => {
222-
@customElement('test-bad-form-associated')
223-
class TestBadFormAssociated extends mixinFormAssociated(
224-
mixinElementInternals(LitElement),
225-
) {
226-
override requestUpdate(
227-
...args: Parameters<LitElement['requestUpdate']>
228-
) {
229-
// Suppress errors that will occur async when the element is
230-
// initialized. This is harder to test in jasmine, so we explicitly
231-
// call the getFormValue function to test the error.
232-
try {
233-
super.requestUpdate(...args);
234-
} catch {}
235-
}
236-
}
237-
238-
const element = new TestBadFormAssociated();
239-
element[getFormValue]();
240-
}).toThrowError(/getFormValue/);
220+
it('should return the value attribute by default', () => {
221+
@customElement('test-default-value-form-associated')
222+
class TestDefaultValueFormAssociated extends mixinFormAssociated(
223+
mixinElementInternals(LitElement),
224+
) {}
225+
226+
const element = new TestDefaultValueFormAssociated();
227+
expect(element[getFormValue]())
228+
.withContext('[getFormValue]() return with no value attribute')
229+
.toBeNull();
230+
element.setAttribute('value', 'value');
231+
expect(element[getFormValue]())
232+
.withContext('[getFormValue]() return with value attribute')
233+
.toBe('value');
241234
});
242235

243236
it('should not add form data without a name', () => {

0 commit comments

Comments
 (0)