Skip to content

Conversation

Saadnajmi
Copy link
Collaborator

@Saadnajmi Saadnajmi commented May 9, 2021

Please select one of the following

  • I am removing an existing difference between facebook/react-native and microsoft/react-native-macos 👍
  • I am cherry-picking a change from Facebook's react-native into microsoft/react-native-macos 👍
  • I am making a fix / change for the macOS implementation of react-native
  • I am making a change required for Microsoft usage of react-native

Summary

Note: Key view loops are finicky and I have written some of my findings into documentation at microsoft/apple-ux-guide#6 . Most of it is relevant to this PR, specifically the fact that if your window's autoRecalculatesKeyViewLoop property is set to true, then any custom nextKeyView properties you set are stamped out whenever a view is added or whenever you press tab.

This closes #768.

We would like to add support for setting the next key view of a component in react native, mirroring the underlying NSVIew property nextKeyView. We do this by creating a custom property on RCTView that takes in the reactTag of a nextKeyView, and uses the UIManager to look up the view and set it as the nextKeyView.

There was a catch: I had to set the prop in the onFocus method of the view, rather than just in the normal JSX initialization. This was the only way I found to get around the fact that RNTester's window automatically recalculates the key view loop. As it turns out, it seems to even recalculate the key view loop every time you tab into a new view. Thus, the only place I could set the prop where it would not get stamped out was right after focus, hence using the onFocus callback.

I considered setting window.autoRecalculatesKeyViewLoop = false in RNTester a non sequitur because that would require us creating a completely manual key view loop for every view in RNTester.. which is no fun and not an approach we would recommend.

Changelog

[macOS] [Added] - Added nextKeyView prop to View and other related components

Test Plan

I added a page to RNTester that demonstrates the use of this new prop.

Screen.Recording.2021-05-09.at.9.49.03.PM.mov

HeyImChris and others added 30 commits May 14, 2020 15:03
Merge upstream into fork
* For arrow keys, add "ArrowLeft", "ArrowRight", "ArrowUp", "ArrowDown",
*/
validKeysUp?: ?Array<string>,

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is left over from adding and removing the prop... but should probably be here so I'll keep it

@Saadnajmi Saadnajmi merged commit 14ce7dc into microsoft:master May 21, 2021
Saadnajmi added a commit to Saadnajmi/react-native-macos that referenced this pull request May 21, 2021
* Update RCTCxxBridge.mm

* Update RCTCxxBridge.mm

* initial props

* initial key loop standup

* update props

* podfile

* Add a few refs

* NextKeyView -> NextKeyViewTest

* More stuff

* commit local changes

* rip

* commit local WIP

* WIP

* Clean up code a but for debugging

* Key view loop works on a second pass

* Move code to componentDidMount, but I still need the console logs in onFocus...

* Latest local changes

* Taking Tomun's approach

* Clean up code

* More cleanup

* more clean up

* for chris

* wip

* wip

* WIP to see the diff

* Cleanup

* More cleanup

* I guess the onFocus hack is the way to go

* Cleanup

* Update tags

* Fix flow checks

* yarn lint check fix

* comment out flipper

* Uncomment Flipper. We should just fix the CI failure instead

* Clean up example fix ifdefs

* Remove prop from everyhing but View

* More comments

* Minor indent fix

* Revert podifle changes

* update podfile

* lint fix

Co-authored-by: Chris Hogan <chrishog@microsoft.com>
Co-authored-by: HeyImChris <48299693+HeyImChris@users.noreply.github.com>
Saadnajmi added a commit to Saadnajmi/react-native-macos that referenced this pull request May 21, 2021
* Update RCTCxxBridge.mm

* Update RCTCxxBridge.mm

* initial props

* initial key loop standup

* update props

* podfile

* Add a few refs

* NextKeyView -> NextKeyViewTest

* More stuff

* commit local changes

* rip

* commit local WIP

* WIP

* Clean up code a but for debugging

* Key view loop works on a second pass

* Move code to componentDidMount, but I still need the console logs in onFocus...

* Latest local changes

* Taking Tomun's approach

* Clean up code

* More cleanup

* more clean up

* for chris

* wip

* wip

* WIP to see the diff

* Cleanup

* More cleanup

* I guess the onFocus hack is the way to go

* Cleanup

* Update tags

* Fix flow checks

* yarn lint check fix

* comment out flipper

* Uncomment Flipper. We should just fix the CI failure instead

* Clean up example fix ifdefs

* Remove prop from everyhing but View

* More comments

* Minor indent fix

* Revert podifle changes

* update podfile

* lint fix

Co-authored-by: Chris Hogan <chrishog@microsoft.com>
Co-authored-by: HeyImChris <48299693+HeyImChris@users.noreply.github.com>
HeyImChris added a commit that referenced this pull request May 24, 2021
* Add support for modifying the key view loop (#769)

* Update RCTCxxBridge.mm

* Update RCTCxxBridge.mm

* initial props

* initial key loop standup

* update props

* podfile

* Add a few refs

* NextKeyView -> NextKeyViewTest

* More stuff

* commit local changes

* rip

* commit local WIP

* WIP

* Clean up code a but for debugging

* Key view loop works on a second pass

* Move code to componentDidMount, but I still need the console logs in onFocus...

* Latest local changes

* Taking Tomun's approach

* Clean up code

* More cleanup

* more clean up

* for chris

* wip

* wip

* WIP to see the diff

* Cleanup

* More cleanup

* I guess the onFocus hack is the way to go

* Cleanup

* Update tags

* Fix flow checks

* yarn lint check fix

* comment out flipper

* Uncomment Flipper. We should just fix the CI failure instead

* Clean up example fix ifdefs

* Remove prop from everyhing but View

* More comments

* Minor indent fix

* Revert podifle changes

* update podfile

* lint fix

Co-authored-by: Chris Hogan <chrishog@microsoft.com>
Co-authored-by: HeyImChris <48299693+HeyImChris@users.noreply.github.com>

* revert changes not part of cherry pick

* Another revert

Co-authored-by: Chris Hogan <chrishog@microsoft.com>
Co-authored-by: HeyImChris <48299693+HeyImChris@users.noreply.github.com>
HeyImChris added a commit that referenced this pull request May 24, 2021
* Update RCTCxxBridge.mm

* Update RCTCxxBridge.mm

* initial props

* initial key loop standup

* update props

* podfile

* Add a few refs

* NextKeyView -> NextKeyViewTest

* More stuff

* commit local changes

* rip

* commit local WIP

* WIP

* Clean up code a but for debugging

* Key view loop works on a second pass

* Move code to componentDidMount, but I still need the console logs in onFocus...

* Latest local changes

* Taking Tomun's approach

* Clean up code

* More cleanup

* more clean up

* for chris

* wip

* wip

* WIP to see the diff

* Cleanup

* More cleanup

* I guess the onFocus hack is the way to go

* Cleanup

* Update tags

* Fix flow checks

* yarn lint check fix

* comment out flipper

* Uncomment Flipper. We should just fix the CI failure instead

* Clean up example fix ifdefs

* Remove prop from everyhing but View

* More comments

* Minor indent fix

* Revert podifle changes

* update podfile

* lint fix

Co-authored-by: Chris Hogan <chrishog@microsoft.com>
Co-authored-by: HeyImChris <48299693+HeyImChris@users.noreply.github.com>

Co-authored-by: Chris Hogan <chrishog@microsoft.com>
Co-authored-by: HeyImChris <48299693+HeyImChris@users.noreply.github.com>
@Saadnajmi Saadnajmi deleted the keyloop branch October 13, 2021 22:23
@Saadnajmi Saadnajmi restored the keyloop branch January 5, 2023 23:46
@Saadnajmi Saadnajmi deleted the keyloop branch January 5, 2023 23:46
@Saadnajmi Saadnajmi restored the keyloop branch January 5, 2023 23:46
Saadnajmi added a commit to Saadnajmi/react-native-macos that referenced this pull request Jan 5, 2023
Saadnajmi added a commit to Saadnajmi/react-native-macos that referenced this pull request Jan 14, 2023
Saadnajmi added a commit to Saadnajmi/react-native-macos that referenced this pull request Jan 14, 2023
Saadnajmi added a commit to Saadnajmi/react-native-macos that referenced this pull request Jan 14, 2023
Saadnajmi added a commit to Saadnajmi/react-native-macos that referenced this pull request Jan 14, 2023
Saadnajmi added a commit that referenced this pull request Jan 14, 2023
Saadnajmi added a commit to shwanton/react-native-macos that referenced this pull request Jan 14, 2023
shwanton pushed a commit to shwanton/react-native-macos that referenced this pull request Feb 13, 2023
shwanton pushed a commit to shwanton/react-native-macos that referenced this pull request Feb 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Proposal: Add support for modifying the Key View Loop

3 participants