Skip to content

fix(login): make username and password required #1701

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 3, 2025

Conversation

shadowbas
Copy link
Contributor

Remove the empty redundant ngModel directive.

@shadowbas shadowbas requested a review from castaway June 16, 2025 14:40
Copy link
Contributor

@castaway castaway left a comment

Choose a reason for hiding this comment

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

It looks like validation is supported in template-driven forms, and as we don't need to capture valuechanges or similar, I think this would be better in KISS style by just adding "required" to the input fields, rather than switching to Reactive forms: https://v16.angular.io/guide/form-validation

@shadowbas
Copy link
Contributor Author

shadowbas commented Jun 17, 2025

It looks like validation is supported in template-driven forms, and as we don't need to capture valuechanges or similar, I think this would be better in KISS style by just adding "required" to the input fields, rather than switching to Reactive forms: https://v16.angular.io/guide/form-validation

@castaway, Required using angular material does not behave like native inputs. It behaves as if required is not defined.

@shadowbas
Copy link
Contributor Author

I guess, what I said is not completely correct. The main issue really is that one has to render the errors themselves using mat-error component. The reactive approach has the rendering of errors baked in.

@shadowbas shadowbas force-pushed the shadowbas/fix-login-inputs-should-be-required branch from 6b973f5 to 4b6b98d Compare June 17, 2025 11:30
@shadowbas
Copy link
Contributor Author

shadowbas commented Jun 17, 2025

@castaway , you are correct that template driven forms offer the ability to perform required validation. I read that forms with validation should use reactive forms and went with that.

@shadowbas shadowbas requested a review from castaway June 17, 2025 11:37
Copy link
Contributor

@castaway castaway left a comment

Choose a reason for hiding this comment

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

lgtm

Disable the submit button when the required fields
are not filled in.
@shadowbas shadowbas force-pushed the shadowbas/fix-login-inputs-should-be-required branch from 9749a70 to 67c0a11 Compare July 1, 2025 10:57
@shadowbas
Copy link
Contributor Author

@castaway, rebased and ready to merge.

@castaway castaway merged commit d99052b into master Jul 3, 2025
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants