Uh oh!
There was an error while loading. Please reload this page.
[DRAFT] fix(fabric, view): support a bunch of macOS only view props - #2674
Closed
Saad Najmi (Saadnajmi) wants to merge 14 commits into
Closed
[DRAFT] fix(fabric, view): support a bunch of macOS only view props#2674Saad Najmi (Saadnajmi) wants to merge 14 commits into
Saad Najmi (Saadnajmi) wants to merge 14 commits into
Conversation
Summary: **Context** - Our Fabric ViewComponentView supports the focus prop but it is not being sent via native props **Change** - Add the native prop to `ReactCommon/.../ViewProps.h` - Updated `focusable` property on `RCTViewComponentView` Test Plan: |RNTester - Fabric| | https://pxl.cl/3wLwF | |RNTester - Paper| || Reviewers: lefever, #rn-desktop Differential Revision: https://phabricator.intern.facebook.com/D49998664 [upstream][fabric] Add enableFocusRing for Fabric ViewComponentView Summary: **Context** - `focusable` prop was added to Fabric ViewComponent in D49998664 - `enableFocusRing` is available on Paper view **Change** - Add the native prop to `ReactCommon/.../ViewProps.h` - Updated `enableFocusView` property on RCTViewComponentView Test Plan: NOTE: Since views are flattened, the focusable view w/ enable focus ring won't be visible till D50102747 |Fabric| | https://pxl.cl/3z9Wm| Reviewers: lefever, #rn-desktop Reviewed By: lefever Differential Revision: https://phabricator.intern.facebook.com/D50102547
Summary: **Context** - Focusable props were added in D49998664 and D50102547 - The ViewComponentView is flattened so when the component had children, the focus view would be flattened away. **Change** - Do not flatten the view if `focusable` or `enableFocusView` are props on the Fabric View Test Plan: |Fabric| | https://pxl.cl/3z9X4| Reviewers: lefever, #rn-desktop Reviewed By: lefever Differential Revision: https://phabricator.intern.facebook.com/D50102747
Summary: This diff updates `ViewProps` with a reference to the `HostPlatformViewProps`, allowing to move macOS specific properties in a separate class without having to change `ViewProps`. Test Plan: Tested later in this stack. Reviewers: shawndempsey, #rn-desktop Reviewed By: shawndempsey Differential Revision: https://phabricator.intern.facebook.com/D53241732 Tasks: T157889406, T154618477
Summary: This diff updates the `ViewEventEmitter` with a reference to the `HostPlatformViewEventEmitter`, allowing to move macOS specific event emitters in a separate file without having to change `ViewEventEmitter`. Test Plan: Tested later in this stack. Reviewers: shawndempsey, #rn-desktop Reviewed By: shawndempsey Differential Revision: https://phabricator.intern.facebook.com/D53241735 Tasks: T157889406, T154618477
Summary: Moving the macOS specific props from ViewProps to the macOS specific HostPlatformViewProps extension. Test Plan: Tested later in this stack. Reviewers: shawndempsey, #rn-desktop Reviewed By: shawndempsey Differential Revision: https://phabricator.intern.facebook.com/D53241734 Tasks: T157889406, T154618477
Summary: This diff adds the `validKeyDown` and `validKeyUp` props to the View component to provide a list of key event matchers used to filter out key event that have to be handled manually by the JS side. These properties were added through the HostPlatformViewProps class to keep the macOS specific properties in a separate file from the iOS ViewProps. Test Plan: Tested later in this stack. Reviewers: shawndempsey, #rn-desktop Reviewed By: shawndempsey Differential Revision: https://phabricator.intern.facebook.com/D53241733 Tasks: T157889406, T154618477
Summary: This diff adds the event emitters that will be used by the keyboard handler of the View component. The `KeyEvent` struct provides the same fields that were previously submitted by Paper. An equals operator between `KeyEvent` and `HandledKey` allows to check if a key event matches the criteria defined by the handled key description. The event emitters were added using the HostPlatformEventEmitter so that the macOS specific changes would be kept separate from the iOS version. Test Plan: Tested later in this stack. Reviewers: shawndempsey, #rn-desktop Reviewed By: shawndempsey Differential Revision: https://phabricator.intern.facebook.com/D53241730 Tasks: T157889406, T154618477
Summary: This diff adds key down/up handling to the View component. The key events are checked against the provided list of key strokes that should be manually handled. If a match is found, the corresponding keyDown/Up event emitter is provided with the captured key even. This will allow the View component to receive keyboard events and respond accordingly. Test Plan: Tested later in this stack. Reviewers: shawndempsey, #rn-desktop Reviewed By: shawndempsey Differential Revision: https://phabricator.intern.facebook.com/D53241731 Tasks: T157889406, T154618477
Summary: This diff adds the `onMouseEnter` and `onMouseLeave` event handler props and event emitters to the View component in Fabric. The mouse event data is the same that was provided by the Paper implementation in `RCTView`. These changes were made in the macOS specific extension of the View component props. Test Plan: Tested later in this stack. Reviewers: shawndempsey, #rn-desktop Reviewed By: shawndempsey Differential Revision: https://phabricator.intern.facebook.com/D53529016 Tasks: T154617556
Summary: This diff implements mouse enter/leave tracking for the area covered by the View component. The tracking is handled by configuring a tracking area on the NSView when either of the handlers is set on the view. This enables the NSResponder `mouseEntered:` and `mouseExited:` notifications which are translated to mouse events. Test Plan: * Run Zeratul with Fabric enabled. * Move the cursor over controls that have hover-dependent styling. https://pxl.cl/4jSw5 Reviewers: shawndempsey, #rn-desktop Reviewed By: shawndempsey Differential Revision: https://phabricator.intern.facebook.com/D53529015 Tasks: T154617556
Summary: Views having mouse event handlers assigned to them should not be flattened so that the mouse tracking for the area they cover would work at all times. This diff disabled view flattening if either of the mouse event handlers is set. Test Plan: Tested later in this stack. Reviewers: shawndempsey, #rn-desktop Reviewed By: shawndempsey Differential Revision: https://phabricator.intern.facebook.com/D53529017 Tasks: T154617556
Summary: This diff adds a bounds change observer to the wrapping clip view, allowing to be notified when the view position is changing because a parent scroll view is being scrolled. This allows the view to evaluate the new cursor position and check if mouse enter/leave events have to be emitted. Test Plan: * Run Zeratul with Fabric enabled. * Scroll the messages view without moving the cursor position and check that enter/leave tracking is working as expected. https://pxl.cl/4jSxh Reviewers: shawndempsey, #rn-desktop Reviewed By: shawndempsey Differential Revision: https://phabricator.intern.facebook.com/D53529018 Tasks: T154617556
…ents Summary: This diff adds null checks to avoid sending key and/or mouse events before the event emitter was assigned to the component. Test Plan: * Run Zeratul with Fabric enabled * Hover over buttons * Use up/down arrows in composer on user tag selector https://pxl.cl/4jVCb Reviewers: shawndempsey, #rn-desktop Reviewed By: shawndempsey Differential Revision: https://phabricator.intern.facebook.com/D53555261
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary:
Cherry pick a bunch of the commits in #2117 around adding support for macOS only view props like
focusableTest Plan:
todo