Skip to content

Add pointerenter/leave to nonBubblingEvents #43542

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

Merged
merged 3 commits into from
Sep 2, 2022

Conversation

zHaytam
Copy link
Contributor

@zHaytam zHaytam commented Aug 25, 2022

(This is @SteveSandersonMS hijacking the PR description to add our servicing template so it can be considered for .NET 7 RC2)

Add pointerenter/leave to nonBubblingEvents

Blazor supports all pointer events except for pointerenter and pointerleave. This adds support for those too.

Description

The only reason why pointerenter and pointerleave don't already work is that they are nonbubbling events, and we haven't included them in the list of nonbubbling events. Blazor needs to know about which events are nonbubbling because the way they are captured and dispatched into .NET is slightly different than for bubbling events.

This PR just adds those events to the nonbubbling list so they start working.

Fixes #43129

Customer Impact

Without this fix, developers can't use the pointerenter and pointerleave events. They have to use the older mouseover and mouseout events which don't support such a wide range of devices and in many cases don't have the desired modern semantics for overlapping elements.

Regression?

  • Yes
  • No

[If yes, specify the version the behavior has regressed from]

Risk

  • High
  • Medium
  • Low

Low because it's just adding two new event names to the list of nonbubbling events. It should only affect the behavior for those two events, which was already broken in the past and now should stop being broken. No other behaviors/events should be affected.

Verification

  • Manual (required)
  • Automated

Packaging changes reviewed?

  • Yes
  • No
  • N/A

Original PR description by @zHaytam:

The events pointerenter and pointerleave are the only two pointer events that don't work. This is because they don't bubble and as such, we need to set useCapture to true, which is done when nonBubblingEvents contains the event. Basically, they need to behave the same way as mouseenter and mouseleave.

Fixes #43129

@zHaytam zHaytam requested a review from a team as a code owner August 25, 2022 16:24
@ghost ghost added area-blazor Includes: Blazor, Razor Components community-contribution Indicates that the PR has been added by a community member labels Aug 25, 2022
@ghost
Copy link

ghost commented Aug 25, 2022

Thanks for your PR, @zHaytam. Someone from the team will get assigned to your PR shortly and we'll get it reviewed.

@SteveSandersonMS
Copy link
Member

Thanks for this, @zHaytam!

I think we'd be happy to accept this PR if it also contained the relevant end-to-end (E2E) test cases for the new functionality. You could add E2E tests for pointerenter and pointerleave following exactly the same pattern that is used for mouseenter and mouseleave tests in MouseEventComponent.razor and EventTest.cs.

Would you be interested in doing that?

@SteveSandersonMS SteveSandersonMS added the pr: pending author input For automation. Specifically separate from Needs: Author Feedback label Sep 1, 2022
@zHaytam
Copy link
Contributor Author

zHaytam commented Sep 1, 2022

Yes, I can do that. I'll have that ready as soon as I can.

@zHaytam
Copy link
Contributor Author

zHaytam commented Sep 1, 2022

I have added the test, but for some reason I can't run it on my machine:

../../../../../node_modules/@types/vfile/index.d.ts(11,31): error TS2792: Cannot find module 'vfile-message'. Did you mean to set the 'moduleResolution' option to 'node', or to add aliases to the 'paths' opti
on? [D:\Projects\aspnetcore\src\JSInterop\Microsoft.JSInterop.JS\src\Microsoft.JSInterop.JS.npmproj]
../../../../../node_modules/@types/webpack-sources/index.d.ts(10,62): error TS2792: Cannot find module 'source-map'. Did you mean to set the 'moduleResolution' option to 'node', or to add aliases to the 'path
s' option? [D:\Projects\aspnetcore\src\JSInterop\Microsoft.JSInterop.JS\src\Microsoft.JSInterop.JS.npmproj]
D:\Projects\aspnetcore\eng\targets\Npm.Common.targets(82,5): error : Command failed with exit code 1. [D:\Projects\aspnetcore\src\JSInterop\Microsoft.JSInterop.JS\src\Microsoft.JSInterop.JS.npmproj]

@SteveSandersonMS
Copy link
Member

@Pilchie Can we consider this one for RC2?

@SteveSandersonMS SteveSandersonMS removed the pr: pending author input For automation. Specifically separate from Needs: Author Feedback label Sep 1, 2022
@Pilchie
Copy link
Member

Pilchie commented Sep 1, 2022

Approved for .NET 7 RC2. Thanks for the contribution @zHaytam!

Copy link
Member

@SteveSandersonMS SteveSandersonMS left a comment

Choose a reason for hiding this comment

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

Actually I need to make sure blazor.server.js and blazor.webview.js are updated before merging this.

@zHaytam No further action is needed from you - I'll sort this out. Thanks for the contribution!

@zHaytam
Copy link
Contributor Author

zHaytam commented Sep 1, 2022

I'm glad this got approved but did the unit tests run? I still can't manage to run them locally so I would hate to contribute something not working...

@SteveSandersonMS
Copy link
Member

@zHaytam Yes, the CI system did run your new PointerEnterAndPointerLeave_CanTrigger test, and it passes on both WebAssembly and Server. You can search by test name at https://dev.azure.com/dnceng-public/public/_build/results?buildId=1245&view=ms.vss-test-web.build-test-results-tab after changing the filter to include "passed" tests.

@SteveSandersonMS SteveSandersonMS self-requested a review September 2, 2022 10:33
@SteveSandersonMS
Copy link
Member

@Pilchie @mkArtakMSFT Could either of you merge this, assuming the build passes and the most recent commit in this PR is still 5fc4102? That's the commit I've approved. The change was approved for RC2 by Kevin above.

I don't have permission to merge to release/7.0 but if I'm getting my info from the latest email thread, that's where RC2 changes should go now.

@Pilchie
Copy link
Member

Pilchie commented Sep 2, 2022

Done - this is the right branch, it just has our default release branch protection rules.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-blazor Includes: Blazor, Razor Components community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Blazor] the pointerenter and pointerleave events not getting fired
3 participants