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

Deprecate acceptsKeyboardFocus #498

Closed
chrisglein opened this issue Jul 13, 2020 · 1 comment · Fixed by #655
Closed

Deprecate acceptsKeyboardFocus #498

chrisglein opened this issue Jul 13, 2020 · 1 comment · Fixed by #655
Labels
enhancement New feature or request help wanted Extra attention is needed Partner: Facebook

Comments

@chrisglein
Copy link
Member

chrisglein commented Jul 13, 2020

From Keyboarding in React Native Desktop document:

React Native core introduced focusable that does the same thing as acceptsKeyboardFocus in v0.63. Windows has added focusable and marked acceptsKeyboardFocus as deprecated in 0.63. macOS needs to do the same.

@ghost ghost added the Needs: Triage 🔍 label Jul 13, 2020
@chrisglein chrisglein added enhancement New feature or request and removed Needs: Triage 🔍 labels Aug 19, 2020
@harinikmsft harinikmsft added the help wanted Extra attention is needed label Aug 20, 2020
@NickGerleman
Copy link

Updating the original description, since the RNW deprecation ended up happening in 0.63. The version matrix basically looked like:

  • RNW 0.63 added focusable and runtime warnings if acceptsKeyboardFocus is used
  • RNW 0.64 will remove acceptsKeyboardFocus entirely (likely going to have a debug assert if VMs see them)
  • For Office, we need to support bundles built against N-1 versions of the react-native version we have in repo, so react-native-win32 also removed typings in 0.63

The TLDR: react-native-macos will need to add focusable by 0.63, and will be able to fully remove acceptsKeyboardFocus in 0.64. Deprecation warnings at runtime are useful since folks are using acceptsKeyboardFocus without typings.

NickGerleman added a commit to NickGerleman/react-native that referenced this issue Nov 18, 2020
Fixes microsoft#498

This change adds support for the "focusable" property present in upstream RN, along with react-native-windows and NetUI. We need to reconcile this against the existing acceptsKeyboardFocus property. This is tricky since its usage was untyped. We follow the same strategy as react-native-windows for this.

1. Usages of acceptsKeyboardFocus will continue to work, but will yellowbox
2. Components will prefer to use focusable, and Touchables with slightly different semantics will prefer focusable semantics when present

acceptsKeyboardFocus will redbox in RNW 0.64 instead of yellowbox, but this change should allow xplat code to use focusable and not be effected by that.

Validated setting a view to focusable is propagated, and that acceptsKeyboardFocus is propagated with a yellowbox.
NickGerleman added a commit that referenced this issue Nov 19, 2020
* Support focusable, Yellowbox on acceptsKeyboardFocus

Fixes #498

This change adds support for the "focusable" property present in upstream RN, along with react-native-windows and NetUI. We need to reconcile this against the existing acceptsKeyboardFocus property. This is tricky since its usage was untyped. We follow the same strategy as react-native-windows for this.

1. Usages of acceptsKeyboardFocus will continue to work, but will yellowbox
2. Components will prefer to use focusable, and Touchables with slightly different semantics will prefer focusable semantics when present

acceptsKeyboardFocus will redbox in RNW 0.64 instead of yellowbox, but this change should allow xplat code to use focusable and not be effected by that.

Validated setting a view to focusable is propagated, and that acceptsKeyboardFocus is propagated with a yellowbox.

* PR feedback

* Explicitly false

* Removed the wrong focusable...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed Partner: Facebook
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants