refactor: Cleanup hit test coercion #17961
Conversation
github-actions
bot
added
platform/wasm 🌐
|
🤖 Your Docs stage site is ready! Visit it here: https://unodocsprstaging.z13.web.core.windows.net/pr-17961/index.html |
|
🤖 Your WebAssembly Sample App stage site is ready! Visit it here: https://unowasmprstaging.z20.web.core.windows.net/pr-17961/index.html |
Youssef1313
force-pushed
the
cleanup-hittest-coercion2
branch
from
de444e9 to
4420403
Compare
|
🤖 Your WebAssembly Sample App stage site is ready! Visit it here: https://unowasmprstaging.z20.web.core.windows.net/pr-17961/index.html |
|
🤖 Your Docs stage site is ready! Visit it here: https://unodocsprstaging.z13.web.core.windows.net/pr-17961/index.html |
Youssef1313
force-pushed
the
cleanup-hittest-coercion2
branch
from
4420403 to
4a9f084
Compare
|
🤖 Your WebAssembly Sample App stage site is ready! Visit it here: https://unowasmprstaging.z20.web.core.windows.net/pr-17961/index.html |
|
🤖 Your Docs stage site is ready! Visit it here: https://unodocsprstaging.z13.web.core.windows.net/pr-17961/index.html |
| return HitTestability.Collapsed; | ||
| } | ||
|
|
||
| element = GetParentUIElement(element); |
There was a problem hiding this comment.
I don't see any interest to walk the tree up instead of relying on a pre-computed property (that is pushed to native elements on wasm for instance). This will be perform for each pointer so I would assume this change would have a negative perf impact.
There was a problem hiding this comment.
@dr1rrb It shouldn't happen for each pointer event, and will hopefully be rare (only when something that changes hit testability changes) and we previously propagated that down the tree, so I assume the parent chain is actually less of work?
There was a problem hiding this comment.
It should not: the down propagation is computed only when an element is being added into the tree or something that changes testability is updated...
Even if this approach is closer to winUI, we need to pre-compute the testability for native elements, so I don't see any interest to try to change the behavior.
| @@ -116,7 +102,7 @@ public IAsyncOperation<DataPackageOperation> DropAsync(CoreDragInfo dragInfo) | |||
| var target = VisualTreeHelper.HitTest( | |||
| dragInfo.Position, | |||
| xamlRoot, | |||
| getTestability: GetDropHitTestability, | |||
| isForDrop: true, | |||
There was a problem hiding this comment.
This reduce the flexibilty of the HitTest method, not sure about the gain?
There was a problem hiding this comment.
@dr1rrb I can try to refactor it back to gain a bit of flexibility back
GitHub Issue (If applicable): closes #
PR Type
What kind of change does this PR introduce?
What is the current behavior?
What is the new behavior?
PR Checklist
Please check if your PR fulfills the following requirements:
Screenshots Compare Test Runresults.Other information
Internal Issue (If applicable):