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

fix(MultiSelectProvider.tsx): double addition on mobile #817

Merged
merged 2 commits into from
Dec 11, 2024

Conversation

KishenKumarrrrr
Copy link
Contributor

Problem

This issue was first identified on ActiveSG where programme filters would be added twice when users performed the action on mobile. This issue does not exist outside mobile mode.

Refer to the following recording of the issue:

Screen.Recording.2024-12-11.at.11.54.22.AM.mov

Investigation

The root cause of the issue is still unclear.

@dextertanyj identified that itemHandleClick function in downshift is only called once, eliminating the hypothesis that there was a bug in the package that led to the event being fired twice.

@dextertanyj further identified that on mobile, two events onClick and onMouseMove are fired at the same time (thus causing two state transitions). The current hypothesis is that this bug arises due to the way React handles both these transitions happening at (almost) the same time.

This issue has been raised before, the conclusion is similar to ours: downshift-js/downshift#1534

Solution

Since the issue might be due to the internal implementation of React, we decided to instead just make the component idempotent. Thus, in onSelectedItemsChange, I use a set to remove duplicates.

@KishenKumarrrrr KishenKumarrrrr self-assigned this Dec 11, 2024
@KishenKumarrrrr KishenKumarrrrr merged commit dfe6aa8 into main Dec 11, 2024
12 checks passed
@KishenKumarrrrr KishenKumarrrrr deleted the fix/double-addition-in-MultiSelectProvider branch December 11, 2024 06:25
@karrui karrui mentioned this pull request Dec 12, 2024
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.

2 participants