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

Clarity controls causes extra valueChanges event to be sent on Angular 14 #337

Closed
2 of 5 tasks
rbirkgit opened this issue Oct 7, 2022 · 5 comments · Fixed by #356
Closed
2 of 5 tasks

Clarity controls causes extra valueChanges event to be sent on Angular 14 #337

rbirkgit opened this issue Oct 7, 2022 · 5 comments · Fixed by #356
Assignees
Labels

Comments

@rbirkgit
Copy link

rbirkgit commented Oct 7, 2022

Describe the bug

When a clarity control loses focus, there is code to check if the control has been touched. If not, Clarity sets the touched state and updates the validity and value of the control. If this control subscribes to valueChanges event, it gets one initial erroneous one which can cause existing reactive form code to break.

How to reproduce

Update to Angular 14. Have a reactive form. Subscribe to valueChanges event and focus and blur the control.

https://stackblitz.com/edit/clarity-light-theme-clr13-cds6-sksq78

Expected behavior

To behave like it did with Angular 13. No extra call to valueChanges event.

Versions

Clarity version:

  • v14.x

Framework:

  • Angular
  • React
  • Vue
  • Other:

Framework version:
ie: Angular 11

Additional notes

A workaround is to put the ClarityModule after the Angular forms module in the modules imports. If it's located in front, the blur events seem to now happen in different order than in Angular 13. Clarity get its event first and therefor the control's touched state has not yet been set. If changing the module import order, now the control is touched by the time clarity's blur event happens.

I wonder if we still need the code that checks for touched in Clarity wrapped-control.ts file. It seems it was for a workaround for numbers in certain cases.

@kevinbuhmann
Copy link
Member

I'm not sure if we can change this without breaking apps that use Angular 13. Maybe try control.valueChanges.pipe(distinctUntilChanged())?

@rbirkgit
Copy link
Author

rbirkgit commented Oct 7, 2022

We cannot use distinctUntilChanged() as the issue is the first value that comes in.

I think there a good chance we don't need the test for touched in the Clarity blur event. But I'm not familiar with the history of it. Without the workaround we cannot use Angular 14 and Clarity. So far it seems promising though. Hopefully it lasts.

@Jinnie
Copy link
Contributor

Jinnie commented Oct 10, 2022

This was documented in code to have been done for only one case - input[type=number] not updating its touched state.
From the tests I did, I could not reproduce this initial problem. Removing the fix looks safe. We'll still need some time to test it properly, though.

@Jinnie Jinnie self-assigned this Oct 17, 2022
Jinnie added a commit to Jinnie/ng-clarity that referenced this issue Oct 20, 2022
This was a bug fix for old issue with input type=number that is not reproducible anymore.
It causes extra valueChanged events on blur after upgrade to Angular v14.

closes vmware-clarity#337

Signed-off-by: Ivan Donchev <idonchev@vmware.com>
Jinnie added a commit to Jinnie/ng-clarity that referenced this issue Oct 25, 2022
This was a bug fix for old issue with input type=number that is not reproducible anymore.
It causes extra valueChanged events on blur after upgrade to Angular v14.

closes vmware-clarity#337

Signed-off-by: Ivan Donchev <idonchev@vmware.com>
Jinnie added a commit that referenced this issue Oct 31, 2022
This was a bug fix for old issue with input type=number that is not reproducible anymore.
It causes extra valueChanged events on blur after upgrade to Angular v14.

closes #337

Signed-off-by: Ivan Donchev <idonchev@vmware.com>
@github-actions
Copy link
Contributor

🎉 This issue has been resolved in version 13.9.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@github-actions
Copy link
Contributor

Hi there 👋, this is an automated message. To help Clarity keep track of discussions, we automatically lock closed issues after 14 days. Please look for another open issue or open a new issue with updated details and reference this one as necessary.

@github-actions github-actions bot locked and limited conversation to collaborators Nov 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants