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

feat: add ignoreInvalidForm config #11

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rafaelss95
Copy link
Contributor

PR Checklist

Please check if your PR fulfills the following requirements:

  • The commit message follows our guidelines: CONTRIBUTING.md#commit
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

[ ] Bugfix
[x] 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 current behavior?

There's no option to ignore invalid form.

Issue Number: Closes #8.

What is the new behavior?

A new option to ignore invalid form.

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

return this.group.get(def.path)!.valueChanges.pipe(
const control = this.group.get(def.path)!;
return control.valueChanges.pipe(
debounceTime(0),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had to use this to ensure group.valid is up to date at this point. For more info, you can check angular/angular#40649. Also let me know if you have a better solution than this.

@@ -3,6 +3,7 @@ import { QueryParamDef } from './QueryParamDef';
export type ParamDefType = 'boolean' | 'array' | 'number' | 'string' | 'object';

export type QueryParamParams<QueryParams = any> = {
ignoreInvalidForm?: boolean;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Please, feel free to suggest another name if needed :)


tick();

assertRouterCall(spectator, { withMinlengthValidator: 'with1' });
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@NetanelBasal I'm stucked in this test. I was hoping the router to be called with

{
  searchTerm: 'term',
  withMinlengthValidator: 'with1',
}

... but, as the filter excludes searchTerm as the group was invalid at that time, the buffer isn't being populated as expected. Would you mind to help me on how we can solve this problem?

@rafaelss95
Copy link
Contributor Author

@NetanelBasal please, note that this is still a draft. I want to get an initial feedback on names and how we can solve the problem I explained above.

After that, I'll try to work on docs and in the final implementation.

@@ -31,7 +31,20 @@ export class BindQueryParamsManager<T = any> {
this.updateControl(this.defs, { emitEvent: true }, (def) => def.strategy === 'twoWay');

const controls = this.defs.map((def) => {
return this.group.get(def.path)!.valueChanges.pipe(
const control = this.group.get(def.path)!;
return control.valueChanges.pipe(
Copy link
Member

Choose a reason for hiding this comment

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

Why we're checking the group here? I think the check should only be if the control is valid. If that's the case, we can update the address.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, that's interesting... if we didn't check the group validity, how can we prevent a router sync if any control is invalid?

Copy link
Member

Choose a reason for hiding this comment

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

The intention was not to sync the specific controls that are not valid. Why preventing other valid controls?

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.

Prevent the change of query params when a form is invalid
2 participants