prototype(View): add isTabStop prop to focus
#1386
Conversation
Allow, e.g., View buttons to be focusable. This is the first step, the next step is to add a JS native event listener that tracks the currently focused element to re-route keyboard input. Towards microsoft#886
Codecov Report
@@ Coverage Diff @@
## master #1386 +/- ##
=========================================
- Coverage 31.65% 31.25% -0.4%
=========================================
Files 259 260 +1
Lines 19194 19436 +242
Branches 1622 1633 +11
=========================================
Hits 6075 6075
- Misses 12966 13208 +242
Partials 153 153
Continue to review full report at Codecov.
|
| if (isTabStop) | ||
| { | ||
| view.GotFocus += OnGotFocus; | ||
| view.LostFocus += OnLostFocus; |
There was a problem hiding this comment.
Should we unregister these event handlers if isTabStop is false?
| /// </summary> | ||
| public class BorderedCanvas : Canvas | ||
| public class ReactViewControl : UserControl |
There was a problem hiding this comment.
UserControl [](start = 36, length = 11)
A while ago, every View mapped to a Border nested in a Canvas.
To noticeably improve performance, things were changed so Borders got created lazily and most Views mapped to just a Canvas.
With this change, each View will map to a UserControl which contains a Canvas. Will this regress performance?
There was a problem hiding this comment.
Yes, this will likely regress performance. A UserControl actually already has a built in border, but it doesn't have a CornerRadius prop, so the performance is even worse than it was originally. We may want to override View and check for the isTabStop property to decide which views we convert to controls, but this prototype is reasonable for a functionality MVP.
| Content = _canvas = new Canvas | ||
| { | ||
| HorizontalAlignment = HorizontalAlignment.Stretch, | ||
| VerticalAlignment = VerticalAlignment.Stretch, |
There was a problem hiding this comment.
Why do we need to set this. It wasn't being set before was it?
There was a problem hiding this comment.
If these values aren't set, the Canvas Width/Height will never be set. We could add property bindings similar to @antonkh's approach for the lazy Border, but these values also guarantee that the Canvas fills the parent user control.
| /// </summary> | ||
| public class BorderedCanvas : Canvas | ||
| public class ReactViewControl : UserControl |
There was a problem hiding this comment.
UserControl [](start = 36, length = 11)
How did you decide to subclass UserControl instead of ContentControl or some other Control subclass? I don't have in-depth knowledge of XAML so I'm curious.
There was a problem hiding this comment.
Good question, honestly, I had originally created ReactViewControl from the Visual Studio User Control template, which is the only XAML template other than the Blank Page template, so that's what I went for (i.e., there's no ContentControl Add New Item template in Visual Studio). One thing that's intriguing about UserControl is that it sets itself as the routed event source for events, so it may be a slight optimization that clicks that occur on the inner Canvas will actually be "sourced" from the control (which is the "unit" that React cares about)
|
How would a developer explicitly specify tab order? Since flexbox can be finicky about ordering of declarations for desired layouts, implicitly ordering tab stops by declaration order may not work. Could this prop be upstreamed to Android and/or Web as well? |
This is just a prototype as well. Leveraging the native focus stack to manage `onKeyDown` and `onKeyUp` events. Also adding `handledKeyUpKeys` and `handledKeyDownKeys` to declaratively specify which keys should be handled by the views.
|
@matthargett I need to test this, but I think that there is a correlation between the declaration of children in a React component and the resulting view hierarchy. I.e., even if you specified That being said, it's trivial to expose the already existing |
This is an example implementation of Button that allows you to tab focus the button and use the return key to trigger the `onPress`. We actually need to integrate more tightly with the Touchable mixin than this to properly support long press and other Touchable props using the return key, as the title implies, this is just a demo. To quickly see this in action, run the RNTester Button example.
|
Putting a pin in this prototype. In general, it's not a good idea for all Views to be controls. Hopefully #1411 will be a better approach in general. |
Allow, e.g., View buttons to be focusable. This is the first step, the next step is to add a JS native event listener that tracks the currently focused element to re-route keyboard input.
Towards #886