Skip to content

Conversation

@cexbrayat
Copy link
Member

PR Checklist

Please check to confirm your PR fulfills the following requirements:

PR Type

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Other... Please describe:

What is the new behavior?

I think a few things could be improved in the new signal forms lesson:

  • Use error.kind for tracking in @for loops instead of $index
  • Use form submit event instead of button click for better semantics
  • Add type="submit" to submit buttons
  • Update modules 19 and 20 exercises to reflect best practices

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

@cexbrayat
Copy link
Member Author

Regarding the usage of [disabled]="loginForm().invalid()" to prevent submission of invalid forms, this pattern has some drawbacks (disabled buttons don't receive focus and may confuse screen reader users, users may not understand why they can't submit).

With signal forms, the submit() utility function already handles validation internally as it marks all fields as touched and only executes your callback if the form is valid. This means you can safely remove the [disabled] binding and let users attempt submission at any time. When they click submit on an invalid form, submit() will automatically mark all fields as touched, triggering the display of all validation errors at once.

I can fix this in the PR if you agree

Copy link

@MarkTechson MarkTechson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a nice addition, thank you @cexbrayat

LGTM.

@MarkTechson
Copy link

Regarding the usage of [disabled]="loginForm().invalid()" to prevent submission of invalid forms, this pattern has some drawbacks (disabled buttons don't receive focus and may confuse screen reader users, users may not understand why they can't submit).

With signal forms, the submit() utility function already handles validation internally as it marks all fields as touched and only executes your callback if the form is valid. This means you can safely remove the [disabled] binding and let users attempt submission at any time. When they click submit on an invalid form, submit() will automatically mark all fields as touched, triggering the display of all validation errors at once.

I can fix this in the PR if you agree

Yeah, I agree with you here actually. You can update the PR.

- Use error.kind for tracking in @for loops instead of $index
- Use form submit event instead of button click for better semantics
- Add type="submit" to submit buttons
- Update Module 19 and 20 exercises to reflect best practices
- Remove disabled button pattern
@cexbrayat cexbrayat force-pushed the fix/signal-forms-lesson branch from 3261f56 to c94a0a8 Compare December 12, 2025 19:17
@cexbrayat
Copy link
Member Author

@MarkTechson Done 👍

@clydin clydin added target: patch This PR is targeted for the next patch release target: minor This PR is targeted for the next minor release action: merge The PR is ready for merge by the caretaker and removed target: patch This PR is targeted for the next patch release labels Dec 12, 2025
@hybrist hybrist merged commit 032257a into angular:main Dec 13, 2025
64 of 65 checks passed
@hybrist
Copy link
Contributor

hybrist commented Dec 13, 2025

This PR was merged into the repository. The changes were merged into the following branches:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

action: merge The PR is ready for merge by the caretaker area: @angular/cli target: minor This PR is targeted for the next minor release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants