Skip to content

test: Simulate mouseover in browser#19567

Closed
eps1lon wants to merge 2 commits intofacebook:masterfrom
eps1lon:test/mouseenter-twice
Closed

test: Simulate mouseover in browser#19567
eps1lon wants to merge 2 commits intofacebook:masterfrom
eps1lon:test/mouseenter-twice

Conversation

@eps1lon
Copy link
Copy Markdown
Collaborator

@eps1lon eps1lon commented Aug 9, 2020

Summary

Failing test for #19562

@gaearon had the right idea in #19562 (comment)

Test Plan

See https://codesandbox.io/s/simulatemousemove-in-reactnext-hnhh3 for actual browser behavior. It first uses simulateMouseMove from this test. The same output is generated if you manually mouse over the first button.

@codesandbox-ci
Copy link
Copy Markdown

codesandbox-ci Bot commented Aug 9, 2020

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit a2325cf:

Sandbox Source
React Configuration
simulateMouseMove in react@next PR

@sizebot
Copy link
Copy Markdown

sizebot commented Aug 9, 2020

No significant bundle size changes to report.

Size changes (experimental)

Generated by 🚫 dangerJS against a2325cf

@sizebot
Copy link
Copy Markdown

sizebot commented Aug 9, 2020

No significant bundle size changes to report.

Size changes (stable)

Generated by 🚫 dangerJS against a2325cf

);

simulateMouseMove(target.parentElement, target);
expect(ops).toEqual(['enter']);
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently failing with

- Expected  - 0
+ Received  + 1

  Array [
    "enter",
+   "enter",
  ]

  1021 | 
  1022 |     simulateMouseMove(target.parentElement, target);
> 1023 |     expect(ops).toEqual(['enter']);
        |                 ^
  1024 |   });

container,
);

simulateMouseMove(target.parentElement, target);
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mouseout is also delivered to an element if the cursor enters a child element, because the child element obscures the visible area of the element.

-- https://developer.mozilla.org/en-US/docs/Web/API/Element/mouseout_event

it MUST be dispatched when the pointer device moves from an element onto the boundaries of one of its descendent elements.

-- https://w3c.github.io/uievents/#event-type-mouseout

);

simulateMouseMove(null, firstTarget);
simulateMouseMove(firstTarget.parentElement, firstTarget);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does this make sense? We haven't "entered" firstTarget.parentElement yet so I don't know if it's valid to leave it already.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. If you move from an element that is rendered by react to another then it doesn't fire twice i.e. in

<div onMouseEnter={logEventType}>
      <button
        onMouseEnter={logEventType}
        style={{ border: "2px solid black", margin: 50, padding: 50 }}
      >
        mouseEnter fires twice on me sometimes
      </button>
      <button
        onFocus={() => {}}
        onMouseEnter={logEventType}
        style={{ border: "2px solid black", margin: 50, padding: 50 }}
      >
        mouseEnter fires twice on me sometimes
      </button>
    </div>

only a single mouseenter is fired. But if you apply

-<div onMouseEnter={logEventType}>
+<React.Fragment>

then you'll observe firing twice. Probably because in

if (isOverEvent && (eventSystemFlags & IS_REPLAYED) === 0) {
const related =
(nativeEvent: any).relatedTarget || (nativeEvent: any).fromElement;
if (related) {
// Due to the fact we don't add listeners to the document with the
// modern event system and instead attach listeners to roots, we
// need to handle the over event case. To ensure this, we just need to
// make sure the node that we're coming from is managed by React.
const inst = getClosestInstanceFromNode(related);
if (inst !== null) {
return;
}
}
}

inst will be null since related points to an element that does not have a fiber? (since it's the rootElement in ReactDOM.render(<App />, rootElement).

@eps1lon
Copy link
Copy Markdown
Collaborator Author

eps1lon commented Aug 10, 2020

Included in #19571

@eps1lon eps1lon closed this Aug 10, 2020
@eps1lon eps1lon deleted the test/mouseenter-twice branch August 10, 2020 14:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants