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

hoverEnd does not fire on button move/resize under cursor #3522

Closed
xav-ie opened this issue Sep 15, 2022 · 7 comments
Closed

hoverEnd does not fire on button move/resize under cursor #3522

xav-ie opened this issue Sep 15, 2022 · 7 comments
Labels
bug Something isn't working

Comments

@xav-ie
Copy link

xav-ie commented Sep 15, 2022

🐛 Bug Report

If a user clicks a button and the button resizes or moves position on the page so that it is no longer under the cursor, the onHoverEnd event does not fire and isHovered remains true.

Steps to reproduce:

  1. Make button with react-aria's useButton and useHover
  2. Create effect that reacts to isHovered and isPressed and moves/resizes the button onPress so it is no longer under the cursor
  3. Notice that onHoverEnd did not fire and isHovered is still true.

Code sandbox link: https://codesandbox.io/s/hoverendnotfiring-25h7dh?file=/src/App.js

🤔 Expected Behavior

The isHovered variable should become false and onHoverEnd should be fired.

😯 Current Behavior

hoverEnd event does not fire and isHovered remains true!

Here is code sandbox link: https://codesandbox.io/s/hoverendnotfiring-25h7dh?file=/src/App.js

💁 Possible Solution

I think adding a delayed check to see if the button is still under cursor after onPressEnd will fix this problem. Or, we can work on making an all-in-one hook/solution for buttons that want hover effects. Right now, it feels a little weird to have useButton to also need useHover.

🔦 Context

Being inspired by Sam Selikoff's video, How to build an Animated Button with React Aria and Framer Motion, I set out to make my own "perfect" button with hover states.

Screen Shot 2022-09-15 at 6 40 27 PM

In the image above, the buttons circles are the problematic buttons that move/change shape on click, creating this issue of retaining their hover states.

💻 Code Sample

https://codesandbox.io/s/hoverendnotfiring-25h7dh?file=/src/App.js

🌍 Your Environment

Software Version(s)
react-aria 3.19.0
Browser Tested on Chromium, Firefox, and Safari and behavior is same
Operating System MacOS

This issue does not happen on mobile because there are no hovers on mobile (normally, lol).

I also use Framer motion to fix the active state not appearing, but that has no effect on this issue as I have a button in the code sandbox without framer-motion experiencing the same issue.

@xav-ie
Copy link
Author

xav-ie commented Sep 15, 2022

I have managed to boil it down to only useHover and checked that useButton is not affecting the useHover hook. Here is the code:

import { useRef, useState } from "react";
import { useHover } from "react-aria";
import { defaultState, hoverState } from "./Constants";

export default function ButtonEventQueue() {
  const ref = useRef(null);
  const [expanded, setExpanded] = useState(true);
  const { hoverProps, isHovered } = useHover({});

  return (
    <button
      {...hoverProps}
      ref={ref}
      onClick={() => setExpanded((e) => !e)}
      style={{
        background: isHovered ? hoverState.background : defaultState.background
      }}
    >
      Don't click this part (hovering: {isHovered ? "t" : "f"})
      {expanded && <span> | Tap this part</span>}
    </button>
  );
}

See https://codesandbox.io/s/hoverendnotfiring-25h7dh?file=/src/OnlyUseHover.jsx for live example.

@snowystinger
Copy link
Member

Seems like a browser bug, but I can't find concrete support for that idea in the spec https://www.w3.org/TR/pointerevents/#the-pointerout-event and https://www.w3.org/TR/pointerevents/#the-pointerleave-event
which both refer to what happens when the pointer is moved, not when the target changes dimensions

We could possibly add logic to track pointer move, but seems like it's a really heavy handed way to handle it. Same thing if we attached a ResizeObserver to the hovered element.

I'll bring it up with our team in our next grooming meeting.

@xav-ie
Copy link
Author

xav-ie commented Sep 16, 2022

Yes, I also came to this conclusion. The pointerout/leave events do not fire for these edge cases, but I think that these are important for when you are trying to style elements with useHover. I think the best way to approach this (or at least how I am trying to get around it is to set a timeout [100ms sounds good I guess??] check for onpointerup and compare the target boundingClientRect contains e.clientX and e.clientY). Here is my jerryrigged "perfect" button: https://codesandbox.io/s/perfectbutton-ve6j75?file=/src/PerfectButton.jsx

@snowystinger
Copy link
Member

But that button isn't working, it still shows hovered as true. Also, if you hover it a second time, the state is stale and it won't trigger an on hover.

This is something we'd need to build into useHover. I'm just not sure the best way to do that without incurring performance hits like I mentioned.

I'll respond back here after I've had a chance to discuss it with the team.

@LFDanLu LFDanLu added the bug Something isn't working label Sep 21, 2022
@LFDanLu LFDanLu moved this to ✏️ To Groom in RSP Component Milestones Sep 21, 2022
@devongovett
Copy link
Member

Tested a few browsers, seems pretty inconsistent: https://codepen.io/devongovett/pen/PoeKmwB

  1. Chrome - fires onpointerleave and removes :hover state immediately when the button moves (on click).
  2. Firefox - removes :hover immediately on click, fires onpointerleave after moving the mouse by at least 1px.
  3. Safari - does nothing on click, removes :hover and fires onpointerleave after moving the mouse.

But since onpointerleave does eventually get fired in all browsers, it should work.

@snowystinger
Copy link
Member

snowystinger commented Sep 21, 2022

Spent some more time looking at this after Devon's analysis

I've changed your default button in the codesandbox, and that now shows the correct behavior

https://codesandbox.io/s/hoverendnotfiring-forked-x83147?file=/src/RegularButton.jsx

The big thing to note is this change. When you spread them both independently, whichever one is spread last is going to overwrite any from the previous that have the same name. In this case, both button props and hover props define onPointerLeave functions. So they need to be merged.

{...hoverProps}
{...buttonProps}
// became
{...mergeProps(hoverProps, buttonProps)}

I'm closing for now, if you find another issue, please feel free to reopen

Repository owner moved this from ✏️ To Groom to ✅ Done in RSP Component Milestones Sep 21, 2022
@xav-ie
Copy link
Author

xav-ie commented Sep 21, 2022

Thank you all for helping me so much with this! I am so happy everything is working now. One last thing to point out is that adding removing whole elements does not fire onpointerleave properly (but not a fault of react-aria!). The code:

{expanded && <span>Tap this part</span>}

inside a button will not fire onpointerleave when the button is not expanded and not showing the span.

However, the fix to this is to either do:

{expanded && "Tap this part"}

so the text inside the button fires onpointerleave properly.
Or you can do:

<span>{expanded && "Tap this part"}</span>

If you need a span inside your button.

See https://codesandbox.io/s/weird-span-issue-lgxfxs?file=/src/App.js for examples. @snowystinger 's sandbox above still does not work for this reason. So, moving a button and changing text inside a button so it resizes fires onpointerleave eventually (thank you for finding this out, @devongovett !). However, adding/removing elements inside a button so it changes size does not fire onpointerleave.

I hope the sandbox above helps someone who runs into the same issue. I was confounding the mergeProps mistake, the weird <span> issue, and the weird way of when browsers decide to fire onpointerleave.

Same behavior found in Chromium, Firefox, and Safari.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Archived in project
Development

No branches or pull requests

4 participants