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

Treat 'pen' pointerType like 'mouse' in useHover hook #2016

Merged
merged 3 commits into from
Dec 8, 2022

Conversation

Nantris
Copy link
Contributor

@Nantris Nantris commented Dec 8, 2022

I made these changes in the github.dev editor, so apologies if any linter requirement might not be met.

I made these changes to the dist/floating-ui.react-dom-interactions.esm.js locally, tested them, and then copied the changes to the source file - so there's an exceedingly small chance there was a transcription error on my part - but I believe this code to be functional and ready to go.

@atomiks

@rollingversions
Copy link

rollingversions bot commented Dec 8, 2022

@floating-ui/react-dom-interactions (0.13.2 → 0.13.3)

Bug Fixes

  • fix(useHover): pointerType is sometimes returned as pen on Linux Chromium even while using a mouse, breaking mouseOnly: true and also delays (which are always 0 for non-mouse input).

    There is also better fallback handling, whereby hover will forcefully work if pointerType can't reliably be determined by the browser.

  • fix(useFocus): pointerType ref getting unset during onPointerLeave with touch emulation, preventing keyboardOnly prop from reliably working.

Packages With No Changes

The following packages have no user facing changes, so won't be released:

  • @floating-ui/core
  • @floating-ui/dom
  • @floating-ui/react-dom
  • @floating-ui/react-native

Edit changelogs

@netlify
Copy link

netlify bot commented Dec 8, 2022

Deploy Preview for vibrant-gates-22c214 canceled.

Name Link
🔨 Latest commit 0e309eb
🔍 Latest deploy log https://app.netlify.com/sites/vibrant-gates-22c214/deploys/639138dbdb48be0008b81c0e

@atomiks
Copy link
Collaborator

atomiks commented Dec 8, 2022

The first one has a spelling mistake Link not Like.

Also, maybe need to add undefined to the array of "mouse-like" types. I guess it's better to fallback to mouse behavior if pointer types aren't supported by the browser, idk though.

Also can you add this comment above the array:

// On some Linux machines with Chromium, mouse inputs return a `pointerType` of
// "pen": https://github.com/floating-ui/floating-ui/issues/2015

@Nantris
Copy link
Contributor Author

Nantris commented Dec 8, 2022

Good catch @atomiks! I accidentally made that typo and "fixed it" but I must have hit Ctrl+Z once too often and undid it.

@Nantris
Copy link
Contributor Author

Nantris commented Dec 8, 2022

@atomiks I'm not sure if it would be undefined or an empty string. Do you still want me to stick undefined in there, or should we handle that as a separate issue if there's some uncertainty.

Edit Or we could just include [undefined, '']

I can't find any definitive answer on the fallback value for pointerType

If the device type cannot be detected by the browser, the value can be an empty string ("")

This says "can be" but not "will be."

@atomiks
Copy link
Collaborator

atomiks commented Dec 8, 2022

pointerTypeRef.current can be undefined, which is why we need it (if onPointerEnter/onPointerDown are never called). As for empty string, yeah I guess add that as well. I think it's better to fallback to showing the floating element than not showing it when it can't be determined. Hopefully all touch devices reliably send back touch though...

@Nantris
Copy link
Contributor Author

Nantris commented Dec 8, 2022

Alright now we have both undefined and '' in the array!

Hopefully all touch devices reliably send back touch though...

One can never be certain, but I think it's a safe assumption. I think it's much less likely for touch devices to have any weirdness like my edge case involving pen vs mouse.

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.

None yet

2 participants