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

onMouseEnter/Leave do not get called when layout changes what is under cursor #5671

Closed
jackerghan opened this issue Aug 8, 2020 · 9 comments
Assignees
Labels
Area: Mouse bug Needs WinUI 3 Indicates that feature can only be done in WinUI 3.0 or beyond. Partner: Facebook
Milestone

Comments

@jackerghan
Copy link
Contributor

jackerghan commented Aug 8, 2020

Environment

Run the following in your terminal and copy the results here.

  1. npx react-native --version: 4.9.0
  2. npx react-native run-windows --info:
    System:
    OS: Windows 10 10.0.19041
    CPU: (40) x64 Intel(R) Xeon(R) Gold 6248 CPU @ 2.50GHz
    Memory: 154.88 GB / 190.60 GB
    Binaries:
    Node: 12.15.0 - C:\open\fbsource\xplat\third-party\node\bin\node.BAT
    Yarn: 1.17.0-20190429.1820 - C:\open\fbsource\xplat\third-party\yarn\yarn.BAT
    npm: 6.10.2 - C:\Program Files\nodejs\npm.CMD
    Watchman: 20200430.030054 - C:\tools\watchman\watchman.EXE
    Installed UWP SDKs:
    10.0.17763.0
    10.0.18362.0
  3. reg query "HKLM\SOFTWARE\Microsoft\Windows\CurrentVersion\AppModelUnlock"
    HKEY_LOCAL_MACHINE\SOFTWARE\Microsoft\Windows\CurrentVersion\AppModelUnlock
    AllowDevelopmentWithoutDevLicense REG_DWORD 0x1

Steps To Reproduce

Provide a detailed list of steps that reproduce the issue.

  1. For the sample below we have a rect that moves every couple seconds. It will turn red on mouse enter and blue on mouse leave.
  2. Move your cursor onto the rect and it will become red.

Expected Results

When the rect moves away by itself, the rect should turn blue. It stays red.
If you move your mouse to where rect would appear, it should turn red when under the cursor.

This is how onMouseEnter/onMouseLeave works on the web (including the RNW renderer for the snack below).
It is also how Xaml works with PointerEntered/Exited events since it was fixed in Windows 8.1
https://docs.microsoft.com/en-us/uwp/api/windows.ui.xaml.uielement.pointerexited?view=winrt-19041#windows8-behavior
Mac RN behaves this way as well, i.e. if a layout change causes what is under the cursor to change, appropriate mouse enter/leaveevents will be raised...

Snack, code example, screenshot, or link to a repository:

https://snack.expo.io/@cenkergan/3302f6

You can run the following sample in playground-win32 by overriding one of the other Samples*.tsx files:

import React, {useState, useEffect} from 'react';
import {AppRegistry, View} from 'react-native';

export default function Bootstrap() {
  const [mbgcl, setBgcl] = useState('green');
  const [left, setLeft] = useState(100);
  useEffect(() => {
    setTimeout(() => {
      setLeft((left + 100) % 500);
    }, 2000);
  }, [left]);

  return (
    <View
      style={{flex:1}}>
      <View
        style={{
          backgroundColor: mbgcl,
          width: 60,
          height: 60,
          position: 'absolute',
          left,
        }}
        onMouseEnter={() => {
          setBgcl('red');
        }}
        onMouseLeave={() => {
          setBgcl('blue');
        }}
      />
    </View>
  );
}

AppRegistry.registerComponent('Bootstrap', () => Bootstrap);

--- For Xaml you can create a new blank UWP CS project, change Grid to Canvas, add a MyRect rect with a white background and paste this code to MainPage.xaml.cs
    public sealed partial class MainPage : Page
    {
        DispatcherTimer dispatcherTimer = new DispatcherTimer();
        public MainPage() {
            this.InitializeComponent();
            dispatcherTimer.Interval = new TimeSpan(0, 0, 1);
            dispatcherTimer.Tick += dispatcherTimer_Tick;
            dispatcherTimer.Start();
        }
        void dispatcherTimer_Tick(object sender, object e) {
            Canvas.SetLeft(MyRect, (Canvas.GetLeft(MyRect) + 100) % 500);
        }
        private void MyRect_PointerEntered(object sender, PointerRoutedEventArgs e) {
            MyRect.Fill = new SolidColorBrush(Windows.UI.Colors.DarkRed);
        }
        private void MyRect_PointerExited(object sender, PointerRoutedEventArgs e) {
            MyRect.Fill = new SolidColorBrush(Windows.UI.Colors.Blue);
        }
    }
@jackerghan jackerghan added the bug label Aug 8, 2020
@ghost ghost added Needs: Triage 🔍 New issue that needs to be reviewed by the issue management team (label applied by bot) Partner: Facebook labels Aug 8, 2020
@chrisglein
Copy link
Member

This is supposed to work with XAML via "pointer replay", but something about what RNW is doing on top must be getting in the way.

@chrisglein chrisglein added Area: Mouse and removed Needs: Triage 🔍 New issue that needs to be reviewed by the issue management team (label applied by bot) labels Aug 10, 2020
@chrisglein chrisglein added this to the 0.64 milestone Aug 10, 2020
@kmelmon
Copy link
Contributor

kmelmon commented Aug 11, 2020

The min repro JS seems to work correctly in a normal UWP scenario (ie Playground). Will try islands in playground-win32 next.

@kmelmon
Copy link
Contributor

kmelmon commented Aug 11, 2020

Indeed, the bug repros in playground-win32. I noticed that sometimes the background color does change after a long delay.

It's a bit strange that the bug does not repro in playground. Next step is to debug XAML and see if the pointer replay is actually firing in both scenarios.

@kmelmon
Copy link
Contributor

kmelmon commented Aug 12, 2020

Bug understood. The pointer replay feature doesn't work in XAML Islands, due to this check in WinBrowserHost.cpp:

bool CXcpBrowserHost::IsPreviousPointerUpdateMessageValid(UINT32 previousPointerUpdateMsgId)
{
ASSERT(m_spPreviousPointerUpdateMsg != nullptr);

// Match the id that was requested against the id of the current cached pointer update message. If they don't match,
// that means we received a newer pointer update message and did a new hit test walk already, so we ignore the request
// and no-op.
if (m_previousPointerUpdateMsgId == previousPointerUpdateMsgId)
{
    // Check if the PointerId of the latest WM_POINTERUPDATE message is still valid. Delete the saved message if it is not.
    POINTER_INFO pointerInfo = {};
    if (::GetPointerInfo(m_spPreviousPointerUpdateMsg->m_pointerInfo.m_pointerId, &pointerInfo))
    {
        // Check the cached hwnd associated with the pointer. It might be a popup window that has since closed, in which case we
        // shouldn't replay the pointer update.
        if (m_previousPointerUpdateHwnd == m_pSite->GetXcpControlWindow()     

The "if (m_previousPointerUpdateHwnd == m_pSite->GetXcpControlWindow()" check isn't islands-aware. It's comparing the HWND of the last pointer message to the core's notion of its window, which in the case of islands is a "fake CoreWindow" and certainly isn't going to pass this check. Thus this function always returns false in islands mode, and XAML refuses to fire the pointer replay event.

@kmelmon
Copy link
Contributor

kmelmon commented Aug 12, 2020

Filed a bug in the WinUI repo to track fixing in XAML:
microsoft/microsoft-ui-xaml#3102

In the mean-time we'll have to think about how to fix this outside of XAML. We may need to implement the equivalent of pointer replay in the RNW code. If we did that, it would have to only be done conditionally, only when in islands mode, because pointer replay does work in UWP mode and having this in two places would cause pointer replay to "double fire" in UWP mode.

@kmelmon
Copy link
Contributor

kmelmon commented Aug 28, 2020

Got confirmation that facebook folks are not blocked on this. Moving to the backlog, most likely outcome will be to wait for this to be fixed in WinUI 3

@kmelmon kmelmon modified the milestones: 0.64, Backlog Aug 28, 2020
@kmelmon
Copy link
Contributor

kmelmon commented Sep 4, 2020

This bug was just fixed in WinUI 3. See:
https://microsoft.visualstudio.com/WinUI/_git/microsoft-ui-xaml-lift/pullrequest/5127704
We currently do not need to service this fix back into system XAML, as @jackerghan has found a workaround.
Will keep this bug for tracking, will most likely resolve when RNW re-targets to use WinUI 3.
@asklar FYI

@asklar asklar added the Needs WinUI 3 Indicates that feature can only be done in WinUI 3.0 or beyond. label Sep 4, 2020
@kmelmon
Copy link
Contributor

kmelmon commented Sep 4, 2020

Update: George tells me pointer replay will still be broken on islands, as the check above was replace by another check that fails on islands. Hence we are still without a fix in WinUI 3.

@kmelmon
Copy link
Contributor

kmelmon commented Oct 8, 2020

Update: George is amazing! He found a fix for WinUI 3, which is now checked in. I think it's safe to close this as there's no specific action we need to take on this issue, it will come when we re-target to WinUI 3.

@kmelmon kmelmon closed this as completed Oct 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Mouse bug Needs WinUI 3 Indicates that feature can only be done in WinUI 3.0 or beyond. Partner: Facebook
Projects
None yet
Development

No branches or pull requests

4 participants