8000 fix(forms): only touch visible, interactive fields on submit · angular/angular@9f99b14 · GitHub
[go: up one dir, main page]

Skip to content

Commit 9f99b14

Browse files
leonsenftatscott
authored andcommitted
fix(forms): only touch visible, interactive fields on submit
Don't touch hidden, disabled, or readonly fields on submit, since they don't contribute to form validity. This also prevents errors from appearing immediately if they're later made interactive. Fix #66344 (cherry picked from commit e682e53)
1 parent 3de87e1 commit 9f99b14

File tree

2 files changed

+90
-3
lines changed

2 files changed

+90
-3
lines changed

packages/forms/signals/src/api/structure.ts

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
* found in the LICENSE file at https://angular.dev/license
77
*/
88

9-
import {inject, Injector, runInInjectionContext, WritableSignal} from '@angular/core';
9+
import {inject, Injector, runInInjectionContext, untracked, WritableSignal} from '@angular/core';
1010

1111
import {BasicFieldAdapter, FieldAdapter} from '../field/field_adapter';
1212
import {FormFieldManager} from '../field/manager';
@@ -369,10 +369,13 @@ export async function submit<TModel>(
369369
action: (form: FieldTree<TModel>) => Promise<TreeValidationResult>,
370370
) {
371371
const node = form() as unknown as FieldNode;
372-
markAllAsTouched(node);
372+
const invalid = untracked(() => {
373+
markAllAsTouched(node);
374+
return node.invalid();
375+
});
373376

374377
// Fail fast if the form is already invalid.
375-
if (node.invalid()) {
378+
if (invalid) {
376379
return;
377380
}
378381

@@ -429,6 +432,12 @@ export function schema<TValue>(fn: SchemaFn<TValue>): Schema<TValue> {
429432

430433
/** Marks a {@link node} and its descendants as touched. */
431434
function markAllAsTouched(node: FieldNode) {
435+
// Don't mark hidden, disabled, or readonly fields as touched since they don't contribute to the
436+
// form's validity. This also prevents errors from appearing immediately if they're later made
437+
// interactive.
438+
if (node.validationState.shouldSkipValidation()) {
439+
return;
440+
}
432441
node.markAsTouched();
433442
for (const child of node.structure.children()) {
434443
markAllAsTouched(child);

packages/forms/signals/test/node/submit.spec.ts

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,15 @@
99
import {ApplicationRef, Injector, resource, signal} from '@angular/core';
1010
import {TestBed} from '@angular/core/testing';
1111
import {
12+
disabled,
1213
form,
1314
required,
1415
requiredError,
1516
submit,
1617
validateAsync,
1718
ValidationError,
19+
hidden,
20+
readonly,
1821
} from '../../public_api';
1922

2023
describe('submit', () => {
@@ -361,6 +364,81 @@ describe('submit', () => {
361364
expect(f.first().errors()).toEqual([]);
362365
expect(f.last().errors()).toEqual([{kind: 'submit', fieldTree: f.last}]);
363366
});
367+
368+
it('does not mark disabled fields as touched', async () => {
369+
const data = signal({first: '', last: ''});
370+
const f = form(
371+
data,
372+
(name) => {
373+
// Disable first name when last name is empty.
374+
disabled(name.first, ({valueOf}) => valueOf(name.last) === '');
375+
},
376+
{injector: TestBed.inject(Injector)},
377+
);
378+
379+
expect(f.first().disabled()).toBe(true);
380+
expect(f.first().touched()).toBe(false);
381+
expect(f.last().touched()).toBe(false);
382+
383+
await submit(f, async () => []);
384+
expect(f.first().touched()).toBe(false);
385+
expect(f.last().touched()).toBe(true);
386+
387+
// Set last name to make first name enabled.
388+
f.last().value.set('Doe');
389+
expect(f.first().disabled()).toBe(false);
390+
expect(f.first().touched()).toBe(false);
391+
});
392+
393+
it('does not mark hidden fields as touched', async () => {
394+
const data = signal({first: '', last: ''});
395+
const f = form(
396+
data,
397+
(name) => {
398+
// Hide first name when last name is empty.
399+
hidden(name.first, ({valueOf}) => valueOf(name.last) === '');
400+
},
401+
{injector: TestBed.inject(Injector)},
402+
);
403+
404+
expect(f.first().hidden()).toBe(true);
405+
expect(f.first().touched()).toBe(false);
406+
expect(f.last().touched()).toBe(false);
407+
408+
await submit(f, async () => []);
409+
expect(f.first().touched()).toBe(false);
410+
expect(f.last().touched()).toBe(true);
411+
412+
// Set last name to make first name visible.
413+
f.last().value.set('Doe');
414+
expect(f.first().hidden()).toBe(false);
415+
expect(f.first().touched()).toBe(false);
416+
});
417+
418+
it('does not mark readonly fields as touched', async () => {
419+
const data = signal({first: '', last: ''});
420+
const f = form(
421+
data,
422+
(name) => {
423+
// Make first name readonly when last name is empty.
424+
readonly(name.first, ({valueOf}) => valueOf(name.last) === '');
425+
},
426+
{injector: TestBed.inject(Injector)},
427+
);
428+
429+
expect(f.first().readonly()).toBe(true);
430+
expect(f.first().touched()).toBe(false);
431+
expect(f.last().touched()).toBe(false);
432+
433+
await submit(f, async () => []);
434+
expect(f.first().touched()).toBe(false);
435+
expect(f.last().touched()).toBe(true);
436+
437+
// Set last name to make first name enabled.
438+
f.last().value.set('Doe');
439+
expect(f.first().readonly()).toBe(false);
440+
expect(f.first().touched()).toBe(false);
441+
});
364442
});
365443

366444
/**

0 commit comments

Comments
 (0)
0