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

Create Support for Event argument functions #203

Closed
jechternach opened this issue May 25, 2022 · 6 comments
Closed

Create Support for Event argument functions #203

jechternach opened this issue May 25, 2022 · 6 comments
Labels
enhancement New feature or request

Comments

@jechternach
Copy link

Summary

For certain events like onKeyDown or OnContextRequested we would want to mark that event is handled.

Currently there isn't support for it beyond I think beyond opening context menus from the pointer position with the TryGetPosition() workaround with the MutableRefObject pattern

Motivation

Here are some patterns where we would like to mark events as handled so that other areas of the code would account for the event as handled.

Currently if we want to mark an event as handled, we have to traverse the xaml tree in native code, find the element in RNX, and mark each element as handled. This is very unstable and fragile for future work as it relies on us searching for element names in RNX that could change in the future

There are other instances like focus events that this could be helpful to have event function args on what is being passed https://docs.microsoft.com/en-us/uwp/api/windows.ui.xaml.input.gettingfocuseventargs?view=winrt-22621

Basic Example

onKeyDown={e=> {
        e.nativeEvent.args.handled(true);
      }}
}

Open Questions

I am not sure if this bug is related to this feature request: #163 (reference)

@jechternach jechternach added the enhancement New feature or request label May 25, 2022
@ghost ghost added the Needs: Triage 🔍 label May 25, 2022
@jdrage
Copy link

jdrage commented May 25, 2022

Thanks @jechternach. This came up in investigation of an Accessibility issue with keyboard navigation, around the capability to reach every item in the list via the up/down arrow keys. GridView/ListView does not currently implement the Grid pattern in UIA, it exposes a linear list, so not being able to get to every item via keyboard up/down is confusing to non-sighted users.

@asklar
Copy link
Member

asklar commented May 26, 2022

The way events work is that the XAML element triggers the event, we go into the event handler on the native side, we serialize whatever event arguments make sense, and pass the value off to be dispatched on the JS thread, asynchronously. Then we return from the native event handler.

So the event that gets delivered to JS, is asynchronous w.r.t. the XAML event handler, since the UI thread is separate from the JS thread. At the time that the JS event runs, we've already returned from the native side event handler and the value of Handled cannot be changed.

See for example: https://github.com/microsoft/react-native-xaml/blob/main/package/windows/ReactNativeXaml/Codegen/TypeEvents.g.h#L62

You'd want to sent args.Handled there, but the JS event won't have been delivered yet so either it needs to never be marked as handled, or always, or it has to be informed by something that is able to be determined statically

@chrisglein
Copy link
Member

Would the ability to statically specify an event as always handled work? Or do you need to dynamically decide? (former = easier, latter = impossible today)

@ghost
Copy link

ghost commented Jun 6, 2022

This issue has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 4 days. It will be closed if no further activity occurs within 3 days of this comment.

@jechternach
Copy link
Author

@jdrage is there a preference to your work to have the ability to statically mark an event as always handled? I think that would work for what we need in JS thus far but I am not sure about your work!

@asklar
Copy link
Member

asklar commented Jun 21, 2022

Closing this for now, please reopen with more details if/once you can come up with what you'd like to see :)

@asklar asklar closed this as not planned Won't fix, can't repro, duplicate, stale Jun 21, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Jul 22, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants