Skip to content
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

refactor: Handle getting value from disabled form controls in form-events util #499

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

michael-small
Copy link
Contributor

About

In all my efforts to get this util all together, I overlooked one of the most important things when it comes to getting form values from reactive forms (for what I argue in the issue is what most people want): getting the value of the form and each control even if the form is disabled.

Closes #491: "Handle getting value from disabled form controls in form-events util"

Notes

  • naming of form-events and its functions is outstanding.
  • Two existing tests were misphrased and one was not accounting for both of the utils. I threw in changes to those since I was in the test suite anyways.
  • @eneajaho about our conversation on naming the signal util: thoughts on toFormState? I feel like it is the compliment to the proposed observable util name of fromFormEvents.

@nartc
Copy link
Collaborator

nartc commented Oct 8, 2024

Can you rebase from latest main real quick to see if there's anything wrong with Nx 20 and latest Angular minor?

@msmallest
Copy link

msmallest commented Oct 8, 2024

The tests as well as a quick manual test of the test app seemed to work fine.

I am terribad at git stuff that isn't commit/push/pull. Anything beyond those, like rebase. I may have screwed it up trying to just rebase following a guide. Idk if it doubling those commits up and the merge was intended from what I think I understand about rebases. I can try to revert this stuff, or is it fine as is? I'm pretty sure you are going to squash this anyways.

edit: idk if this PR also being cosigned by my alt acct is a problem or not, but I am away from my main acct computer at the moment and wanted to get this change out

@msmallest msmallest force-pushed the refactor/getRawValue-form-events branch from 6eb5e0a to fcd218e Compare November 1, 2024 14:45
@msmallest msmallest force-pushed the refactor/getRawValue-form-events branch from fcd218e to 7e0faf3 Compare November 22, 2024 16:36
…EventsObservable`

Renaming of the functions outstanding beyond this issue.

Addresses ngxtension#491 - Handle getting value from disabled form controls in `form-events` util
… case

While I was making a test for fixing Issue ngxtension#491: Handle getting value from disabled form controls in form-events util, I realized the tests were misphrased, in that they didn't handle exactly what was said.
@msmallest msmallest force-pushed the refactor/getRawValue-form-events branch from 7e0faf3 to 5bf0c38 Compare November 26, 2024 15:34
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.

Handle getting value from disabled form controls in form-events util
3 participants