refactor(switch): signal migration #468
Conversation
| @@ -58,5 +58,5 @@ export function rxHostPressedListener() { | |||
| }), | |||
| filter(Boolean), | |||
| ), | |||
| ).pipe(debounceTime(0)); | |||
There was a problem hiding this comment.
This was causing several issues. The individual listeners attached in the lines above already have a delay, the problem is that this delay is added after the takeUntilDestroyed, which means that if the checkbox is destroyed during this debounce we get an error thrown. This was resulting if us having to add if statements in the code to see if the checkbox exists and was causing issues in some tests too. Removing this fixes the issues.
There was a problem hiding this comment.
How are you feeling about rxHostPressedListener? That was a super early implementation and not sure if it's worth the upkeep!
There was a problem hiding this comment.
Yeah, I agree, personally I think it's easier and more obvious just to add a two hostlisteners. Saves jumping between files and it's not immediately obvious what this functions purpose is. I don't really see the benefit in it myself.
There was a problem hiding this comment.
It was supposed to be advanced into something like usePress from React Aria, which is great for mobile apps. But I never got around to that. Happy to deprecate/remove before v1
There was a problem hiding this comment.
Makes sense. If that is ever something you want to bring in, I ported all those interactions from react aria to angular for angular primitives, so they could easily be carried over to Spartan.
There was a problem hiding this comment.
I have now removed the rxHostListeners from all components. Wasn't used in many places.
PR Checklist
Please check if your PR fulfills the following requirements:
guidelines: https://github.com/goetzrobin/spartan/blob/main/CONTRIBUTING.md#-commit-message-guidelines
PR Type
What kind of change does this PR introduce?
Which package are you modifying?
What is the current behavior?
Closes #438
What is the new behavior?
Does this PR introduce a breaking change?
Other information