feat(forms): Prevents marking fields as touched/dirty when state is hidden/readonly/disabled (#63633)

Ensures fields that are hidden, disabled, or readonly cannot be marked as touched or dirty, improving form state integrity

PR Close #63633

packages/forms/signals/src/field/state.ts

@@ -37,7 +37,6 @@ export class FieldNodeState {
    * Marks this specific field as touched.
    */
   markAsTouched(): void {
-    // TODO: should this be noop for fields that are hidden/disabled/readonly
     this.selfTouched.set(true);
   }
 
@@ -45,7 +44,6 @@ export class FieldNodeState {
    * Marks this specific field as dirty.
    */
   markAsDirty(): void {
-    // TODO: should this be noop for fields that are hidden/disabled/readonly
     this.selfDirty.set(true);
   }
 
@@ -72,13 +70,14 @@ export class FieldNodeState {
    * Whether this field is considered dirty.
    *
    * A field is considered dirty if one of the following is true:
-   *  - It was directly dirtied
+   *  - It was directly dirtied and is interactive
    *  - One of its children is considered dirty
    */
   readonly dirty: Signal<boolean> = computed(() => {
+    const selfDirtyValue = this.selfDirty() && !this.isNonInteractive();
     return reduceChildren(
       this.node,
-      this.selfDirty(),
+      selfDirtyValue,
       (child, value) => value || child.nodeState.dirty(),
       shortCircuitTrue,
     );
@@ -88,17 +87,18 @@ export class FieldNodeState {
    * Whether this field is considered touched.
    *
    * A field is considered touched if one of the following is true:
-   *  - It was directly touched
+   *  - It was directly touched and is interactive
    *  - One of its children is considered touched
    */
-  readonly touched: Signal<boolean> = computed(() =>
-    reduceChildren(
+  readonly touched: Signal<boolean> = computed(() => {
+    const selfTouchedValue = this.selfTouched() && !this.isNonInteractive();
+    return reduceChildren(
       this.node,
-      this.selfTouched(),
+      selfTouchedValue,
       (child, value) => value || child.nodeState.touched(),
       shortCircuitTrue,
-    ),
-  );
+    );
+  });
 
   /**
    * The reasons for this field's disablement. This includes disabled reasons for any parent field
@@ -156,4 +156,15 @@ export class FieldNodeState {
 
     return `${parent.name()}.${this.node.structure.keyInParent()}`;
   });
+
+  /** Whether this field is considered non-interactive.
+   *
+   * A field is considered non-interactive if one of the following is true:
+   * - It is hidden
+   * - It is disabled
+   * - It is readonly
+   */
+  private readonly isNonInteractive = computed(
+    () => this.hidden() || this.disabled() || this.readonly(),
+  );
 }

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

@@ -15,6 +15,7 @@ import {
   disabled,
   FieldPath,
   form,
+  hidden,
   readonly,
   required,
   REQUIRED,
@@ -177,6 +178,137 @@ describe('FieldNode', () => {
       expect(f().dirty()).toBe(false);
       expect(f.b).toBeUndefined();
     });
+
+    it('should not be marked as dirty when is readonly', () => {
+      const f = form(
+        signal({
+          a: 1,
+          b: 2,
+        }),
+        (p) => {
+          readonly(p);
+        },
+        {injector: TestBed.inject(Injector)},
+      );
+
+      expect(f().dirty()).toBe(false);
+
+      f().markAsDirty();
+      expect(f().dirty()).toBe(false);
+    });
+    it('should not be marked as dirty when is disabled', () => {
+      const f = form(
+        signal({
+          a: 1,
+          b: 2,
+        }),
+        (p) => {
+          disabled(p);
+        },
+        {injector: TestBed.inject(Injector)},
+      );
+
+      expect(f().dirty()).toBe(false);
+
+      f().markAsDirty();
+      expect(f().dirty()).toBe(false);
+    });
+
+    it('should not be marked as dirty when is hidden', () => {
+      const f = form(
+        signal({
+          a: 1,
+          b: 2,
+        }),
+        (p) => {
+          hidden(p, () => true);
+        },
+        {injector: TestBed.inject(Injector)},
+      );
+
+      expect(f().dirty()).toBe(false);
+
+      f().markAsDirty();
+
+      expect(f().dirty()).toBe(false);
+    });
+
+    it('should be marked as dirty when not readonly', () => {
+      const f = form(
+        signal({
+          a: 1,
+          b: 2,
+        }),
+        {injector: TestBed.inject(Injector)},
+      );
+
+      expect(f().dirty()).toBe(false);
+
+      f().markAsDirty();
+      expect(f().dirty()).toBe(true);
+    });
+
+    it('should be marked as dirty when not disabled', () => {
+      const f = form(
+        signal({
+          a: 1,
+          b: 2,
+        }),
+        {injector: TestBed.inject(Injector)},
+      );
+
+      expect(f().dirty()).toBe(false);
+
+      f().markAsDirty();
+      expect(f().dirty()).toBe(true);
+    });
+
+    it('should be marked as dirty when not hidden', () => {
+      const f = form(
+        signal({
+          a: 1,
+          b: 2,
+        }),
+        {injector: TestBed.inject(Injector)},
+      );
+
+      expect(f().dirty()).toBe(false);
+
+      f().markAsDirty();
+      expect(f().dirty()).toBe(true);
+    });
+
+    it('should become pristine when field becomes non-interactive after being marked dirty', () => {
+      const isReadonly = signal(false);
+      const f = form(
+        signal({
+          a: 1,
+          b: 2,
+        }),
+        (p) => {
+          readonly(p, isReadonly);
+        },
+        {injector: TestBed.inject(Injector)},
+      );
+
+      // Initially interactive and not dirty
+      expect(f().readonly()).toBe(false);
+      expect(f().dirty()).toBe(false);
+
+      // Mark as dirty while interactive
+      f().markAsDirty();
+      expect(f().dirty()).toBe(true);
+
+      // Make non-interactive, should become pristine
+      isReadonly.set(true);
+      expect(f().readonly()).toBe(true);
+      expect(f().dirty()).toBe(false);
+
+      // Make interactive again, should still be dirty
+      isReadonly.set(false);
+      expect(f().readonly()).toBe(false);
+      expect(f().dirty()).toBe(true);
+    });
   });
 
   describe('touched', () => {
@@ -255,6 +387,137 @@ describe('FieldNode', () => {
       f().reset();
       expect(f().touched()).toBe(false);
     });
+
+    it('should not be marked as touched when is readonly', () => {
+      const f = form(
+        signal({
+          a: 1,
+          b: 2,
+        }),
+        (p) => {
+          readonly(p);
+        },
+        {injector: TestBed.inject(Injector)},
+      );
+
+      expect(f().touched()).toBe(false);
+
+      f().markAsTouched();
+      expect(f().touched()).toBe(false);
+    });
+    it('should not be marked as touched when is disabled', () => {
+      const f = form(
+        signal({
+          a: 1,
+          b: 2,
+        }),
+        (p) => {
+          disabled(p);
+        },
+        {injector: TestBed.inject(Injector)},
+      );
+
+      expect(f().touched()).toBe(false);
+
+      f().markAsTouched();
+      expect(f().touched()).toBe(false);
+    });
+
+    it('should not be marked as touched when is hidden', () => {
+      const f = form(
+        signal({
+          a: 1,
+          b: 2,
+        }),
+        (p) => {
+          hidden(p, () => true);
+        },
+        {injector: TestBed.inject(Injector)},
+      );
+
+      expect(f().touched()).toBe(false);
+
+      f().markAsTouched();
+
+      expect(f().touched()).toBe(false);
+    });
+
+    it('should be marked as touched when not readonly', () => {
+      const f = form(
+        signal({
+          a: 1,
+          b: 2,
+        }),
+        {injector: TestBed.inject(Injector)},
+      );
+
+      expect(f().touched()).toBe(false);
+
+      f().markAsTouched();
+      expect(f().touched()).toBe(true);
+    });
+
+    it('should be marked as touched when not disabled', () => {
+      const f = form(
+        signal({
+          a: 1,
+          b: 2,
+        }),
+        {injector: TestBed.inject(Injector)},
+      );
+
+      expect(f().touched()).toBe(false);
+
+      f().markAsTouched();
+      expect(f().touched()).toBe(true);
+    });
+
+    it('should be marked as touched when not hidden', () => {
+      const f = form(
+        signal({
+          a: 1,
+          b: 2,
+        }),
+        {injector: TestBed.inject(Injector)},
+      );
+
+      expect(f().touched()).toBe(false);
+
+      f().markAsTouched();
+      expect(f().touched()).toBe(true);
+    });
+
+    it('should become untouched when field becomes non-interactive after being marked touched', () => {
+      const isHidden = signal(false);
+      const f = form(
+        signal({
+          a: 1,
+          b: 2,
+        }),
+        (p) => {
+          hidden(p, isHidden);
+        },
+        {injector: TestBed.inject(Injector)},
+      );
+
+      // Initially interactive and not touched
+      expect(f().hidden()).toBe(false);
+      expect(f().touched()).toBe(false);
+
+      // Mark as touched while interactive
+      f().markAsTouched();
+      expect(f().touched()).toBe(true);
+
+      // Make non-interactive, should become untouched
+      isHidden.set(true);
+      expect(f().hidden()).toBe(true);
+      expect(f().touched()).toBe(false);
+
+      // Make interactive again, should still be touched
+      isHidden.set(false);
+      expect(f().hidden()).toBe(false);
+      expect(f().touched()).toBe(true);
+    });
   });
 
   describe('arrays', () => {