refactor(tap-click): use pointer events api#29192
Merged
liamdebeasi merged 2 commits intofeature-8.0from Mar 21, 2024
Merged
Conversation
2 tasks
averyjohnston
approved these changes
Mar 21, 2024
Contributor
averyjohnston
left a comment
There was a problem hiding this comment.
Verified on desktop plus my actual Android and iOS devices, looks good!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue number: Internal
What is the current behavior?
Amanda pointed out that the ripple effect for the Button inside of InputPasswordToggle was not working. I found out that calling
ev.preventDefaultonpointerdowncausesmouseupto not get fired. On desktop, we rely onmouseupto know when to add the ripple effect. (touchendis not impacted)Interestingly, calling
ev.preventDefaultonpointerdowndoes not preventpointerupfrom being fired. The idea here is that if we migrate the tap click utility to use the PointerEvents API instead of separate mouse/touch listeners we can keep the existing tap click behavior while also fixing the bug that Amanda noted.What is the new behavior?
Impact to developers is fairly minimal. There should be no behavior change (other than the bug I noted being fixed). There should be a very small perf boost because this util now only adds 4 event listeners on the document instead of 7 previously.
Does this introduce a breaking change?
Other information
Reviewers: Please manually test this on desktop devices as well as iOS and Android devices (not Chrome Dev Tools. iOS simulators are fine). Test that components such as
ion-buttoncorrectly add the activated state (or ripple effect for MD). Also verify that the activated state is not added when tapping the button and then scrolling. For desktop, check that right clicking does not add the activated state.