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 "tipsy-ness" level selector on mobile #32

Merged
merged 2 commits into from
Mar 24, 2021
Merged

Conversation

Quaffel
Copy link
Owner

@Quaffel Quaffel commented Mar 24, 2021

Fix adds the previously missing "touchmove" event listener to the selector's container. As the default input handling must be suppressed (Event#preventDefault), regular DOM event listeners are registered manually as React always uses passive event listeners (which are not allowed to invoke this function) for handling these events (PR).

If there's a more elegant/state-of-the-art solution, please let me know.

Resolves #16.

@Quaffel Quaffel requested a review from Jonasdoubleyou March 24, 2021 21:30
@Quaffel Quaffel linked an issue Mar 24, 2021 that may be closed by this pull request
@Jonasdoubleyou
Copy link
Collaborator

Seems reasonable as long as facebook/react#6436 stays open.

@Quaffel Quaffel requested a review from Jonasdoubleyou March 24, 2021 23:05
@Quaffel Quaffel merged commit 8dc1516 into main Mar 24, 2021
@Quaffel Quaffel deleted the fix/tipsy-selector-mobile branch March 24, 2021 23:07
@Jonasdoubleyou
Copy link
Collaborator

Jonasdoubleyou commented Mar 24, 2021

Assuming that setValue is stable, omitting onMove from dependencies is okay :) (another option would be to wrap onMove in a useCallback, but yeah, works this way too ...)

@Quaffel
Copy link
Owner Author

Quaffel commented Mar 24, 2021

Haven't thought of this, thank you for the heads-up! The assumption that setValue is stable is quite fair to make, but using useCallback would have been better; you're right. I won't open a separate pull request for this though.

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.

Fix "tipsy-ness" level selector on mobile
2 participants