Skip to content

Commit e729e4e

Browse files
authored
fix(web-components): various field interaction bugs and storybook inconsistencies (#34555)
1 parent 911ee44 commit e729e4e

File tree

13 files changed

+159
-188
lines changed

13 files changed

+159
-188
lines changed
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"type": "prerelease",
3+
"comment": "fix: various field interaction bugs",
4+
"packageName": "@fluentui/web-components",
5+
"email": "[email protected]",
6+
"dependentChangeType": "patch"
7+
}

packages/web-components/docs/web-components.api.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -673,6 +673,10 @@ export class BaseField extends FASTElement {
673673
changeHandler(e: Event): boolean | void;
674674
// @internal
675675
clickHandler(e: MouseEvent): boolean | void;
676+
// (undocumented)
677+
connectedCallback(): void;
678+
// (undocumented)
679+
disconnectedCallback(): void;
676680
// @internal
677681
elementInternals: ElementInternals;
678682
// @internal

packages/web-components/src/button/button.base.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -395,7 +395,7 @@ export class BaseButton extends FASTElement {
395395
!this.name &&
396396
!this.formAction &&
397397
!this.formEnctype &&
398-
!this.form &&
398+
!this.formAttribute &&
399399
!this.formMethod &&
400400
!this.formNoValidate &&
401401
!this.formTarget

packages/web-components/src/checkbox/checkbox.base.ts

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,15 @@ export class BaseCheckbox extends FASTElement {
131131
this._value = next;
132132
}
133133

134+
/**
135+
* Tracks whether the space key was pressed down while the checkbox was focused.
136+
* This is used to prevent inadvertently checking a required, unchecked checkbox when the space key is pressed on a
137+
* submit button and field validation is triggered.
138+
*
139+
* @internal
140+
*/
141+
private _keydownPressed: boolean = false;
142+
134143
/**
135144
* The name of the element. This element's value will be surfaced during form submission under the provided name.
136145
*
@@ -352,7 +361,8 @@ export class BaseCheckbox extends FASTElement {
352361
}
353362

354363
/**
355-
* Prevents scrolling when the user presses the space key.
364+
* Prevents scrolling when the user presses the space key, and sets a flag to indicate that the space key was pressed
365+
* down while the checkbox was focused.
356366
*
357367
* @param e - the event object
358368
* @internal
@@ -361,6 +371,8 @@ export class BaseCheckbox extends FASTElement {
361371
if (e.key !== ' ') {
362372
return true;
363373
}
374+
375+
this._keydownPressed = true;
364376
}
365377

366378
/**
@@ -370,10 +382,11 @@ export class BaseCheckbox extends FASTElement {
370382
* @internal
371383
*/
372384
public keyupHandler(e: KeyboardEvent): boolean | void {
373-
if (e.key !== ' ') {
385+
if (!this._keydownPressed || e.key !== ' ') {
374386
return true;
375387
}
376388

389+
this._keydownPressed = false;
377390
this.click();
378391
}
379392

packages/web-components/src/checkbox/checkbox.spec.ts

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { CheckboxShape, CheckboxSize } from './checkbox.options.js';
55
test.describe('Checkbox', () => {
66
test.use({
77
tagName: 'fluent-checkbox',
8+
waitFor: ['fluent-button'],
89
});
910

1011
test('should have a role of `checkbox`', async ({ fastPage }) => {
@@ -350,4 +351,33 @@ test.describe('Checkbox', () => {
350351

351352
await expect(page).toHaveURL(/\?checkbox=foo&checkbox=bar/);
352353
});
354+
355+
test('should not change the checkedness when the form is submitted and the checkbox is invalid (required and unchecked)', async ({
356+
fastPage,
357+
page,
358+
}) => {
359+
const { element } = fastPage;
360+
const submitButton = page.locator('fluent-button');
361+
362+
await fastPage.setTemplate(/* html */ `
363+
<form>
364+
<label for="checkbox">Checkbox</label>
365+
<fluent-checkbox required name="checkbox" id="checkbox"></fluent-checkbox>
366+
<fluent-button type="submit">submit</fluent-button>
367+
</form>
368+
`);
369+
370+
await expect(element).toHaveJSProperty('checked', false);
371+
await expect(element).toHaveJSProperty('validity.valueMissing', true);
372+
373+
await submitButton.focus();
374+
375+
await expect(submitButton).toBeFocused();
376+
377+
await page.keyboard.down(' ');
378+
379+
await page.keyboard.up(' ');
380+
381+
await expect(element).toHaveJSProperty('checked', false);
382+
});
353383
});

packages/web-components/src/checkbox/checkbox.stories.ts

Lines changed: 22 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
1-
import { html, repeat } from '@microsoft/fast-element';
1+
import { html, ref, repeat } from '@microsoft/fast-element';
22
import { uniqueId } from '@microsoft/fast-web-utilities';
3-
import { LabelPosition, ValidationFlags } from '../field/field.options.js';
3+
import { LabelPosition } from '../field/field.options.js';
44
import { type Meta, renderComponent, type StoryArgs, type StoryObj } from '../helpers.stories.js';
55
import type { Checkbox as FluentCheckbox } from './checkbox.js';
66
import { CheckboxShape, CheckboxSize } from './checkbox.options.js';
@@ -18,6 +18,7 @@ const storyTemplate = html<StoryArgs<FluentCheckbox>>`
1818
shape="${story => story.shape}"
1919
size="${story => story.size}"
2020
slot="${story => story.slot}"
21+
${ref('checkbox')}
2122
>
2223
${story => story.checkedIndicatorContent?.()} ${story => story.indeterminateIndicatorContent?.()}
2324
</fluent-checkbox>
@@ -123,7 +124,7 @@ export default {
123124
export const Default: Story = {};
124125

125126
export const Checkbox: Story = {
126-
render: renderComponent(storyTemplate).bind({}),
127+
render: renderComponent(storyTemplate),
127128
args: {
128129
id: uniqueId('checkbox-'),
129130
},
@@ -132,7 +133,7 @@ export const Checkbox: Story = {
132133
export const Checked: Story = {
133134
render: renderComponent(html<StoryArgs<FluentCheckbox>>`
134135
${repeat(story => story.storyContent, html<StoryArgs<FluentCheckbox>>`${fieldStoryTemplate}<br />`)}
135-
`).bind({}),
136+
`),
136137
args: {
137138
storyContent: [
138139
{
@@ -159,7 +160,7 @@ export const Checked: Story = {
159160
export const Indeterminate: Story = {
160161
render: renderComponent(html<StoryArgs<FluentCheckbox>>`
161162
${repeat(story => story.storyContent, html<StoryArgs<FluentCheckbox>>`${fieldStoryTemplate}<br />`)}
162-
`).bind({}),
163+
`),
163164
args: {
164165
storyContent: [
165166
{
@@ -186,7 +187,7 @@ export const Indeterminate: Story = {
186187
export const Disabled: Story = {
187188
render: renderComponent(html<StoryArgs<FluentCheckbox>>`
188189
${repeat(story => story.storyContent, html<StoryArgs<FluentCheckbox>>`${fieldStoryTemplate}<br />`)}
189-
`).bind({}),
190+
`),
190191
args: {
191192
storyContent: [
192193
{
@@ -250,30 +251,33 @@ export const Disabled: Story = {
250251

251252
export const Required: Story = {
252253
render: renderComponent(html<StoryArgs<FluentCheckbox>>`
253-
<form style="display: inline-flex; gap: 1em; align-items: baseline">
254+
<form
255+
@reset="${story => story.successMessage.toggleAttribute('hidden', true)}"
256+
@submit="${story => story.checkbox.checkValidity() && story.successMessage.toggleAttribute('hidden', false)}"
257+
style="display: inline-flex; flex-direction: column; gap: 1rem; max-width: 400px;"
258+
>
259+
${fieldStoryTemplate}
254260
<div>
255-
<fluent-checkbox id="required-fluent-checkbox" required></fluent-checkbox>
256-
<label for="required-fluent-checkbox">Required</label>
261+
<fluent-button type="submit">Submit</fluent-button>
262+
<fluent-button type="reset">Reset</fluent-button>
257263
</div>
258-
${fieldStoryTemplate}
259-
<fluent-button type="submit">Submit</fluent-button>
264+
<fluent-text ${ref('successMessage')} hidden>Form submitted successfully!</fluent-text>
260265
</form>
261-
`).bind({}),
266+
`),
262267
args: {
263268
storyContent: storyTemplate,
264269
slot: 'input',
265270
labelPosition: LabelPosition.after,
266271
id: uniqueId('checkbox-'),
267272
required: true,
268-
label: 'Required',
269-
messages: [{ message: 'This field is required', flag: ValidationFlags.valueMissing }],
273+
label: 'Check this to submit the form',
270274
},
271275
};
272276

273277
export const Large: Story = {
274278
render: renderComponent(html<StoryArgs<FluentCheckbox>>`
275279
${repeat(story => story.storyContent, html<StoryArgs<FluentCheckbox>>`${fieldStoryTemplate}<br />`)}
276-
`).bind({}),
280+
`),
277281
args: {
278282
storyContent: [
279283
{
@@ -317,7 +321,7 @@ export const Large: Story = {
317321
};
318322

319323
export const LabelBefore: Story = {
320-
render: renderComponent(fieldStoryTemplate).bind({}),
324+
render: renderComponent(fieldStoryTemplate),
321325
args: {
322326
storyContent: storyTemplate,
323327
id: uniqueId('checkbox-'),
@@ -328,7 +332,7 @@ export const LabelBefore: Story = {
328332
};
329333

330334
export const LabelWrapping: Story = {
331-
render: renderComponent(fieldStoryTemplate).bind({}),
335+
render: renderComponent(fieldStoryTemplate),
332336
args: {
333337
storyContent: storyTemplate,
334338
id: uniqueId('checkbox-'),
@@ -348,7 +352,7 @@ export const LabelWrapping: Story = {
348352
export const Circular: Story = {
349353
render: renderComponent(html<StoryArgs<FluentCheckbox>>`
350354
${repeat(story => story.storyContent, html<StoryArgs<FluentCheckbox>>`${fieldStoryTemplate}<br />`)}
351-
`).bind({}),
355+
`),
352356
args: {
353357
storyContent: [
354358
{

packages/web-components/src/field/field.base.ts

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -47,13 +47,6 @@ export class BaseField extends FASTElement {
4747
*/
4848
public messageSlotChanged(prev: Element[], next: Element[]) {
4949
toggleState(this.elementInternals, 'has-message', !!next.length);
50-
51-
if (!next.length) {
52-
this.removeEventListener('invalid', this.invalidHandler, { capture: true });
53-
return;
54-
}
55-
56-
this.addEventListener('invalid', this.invalidHandler, { capture: true });
5750
}
5851

5952
/**
@@ -140,6 +133,16 @@ export class BaseField extends FASTElement {
140133
this.elementInternals.role = 'presentation';
141134
}
142135

136+
connectedCallback(): void {
137+
super.connectedCallback();
138+
this.addEventListener('invalid', this.invalidHandler, { capture: true });
139+
}
140+
141+
disconnectedCallback(): void {
142+
this.removeEventListener('invalid', this.invalidHandler, { capture: true });
143+
super.disconnectedCallback();
144+
}
145+
143146
/**
144147
* Applies the `focus-visible` state to the element when the slotted input receives visible focus.
145148
*

0 commit comments

Comments
 (0)