From 73c5a8ec1becc5be323acda88cabb2da881954cf Mon Sep 17 00:00:00 2001 From: Sam Mathias Weggersen Date: Fri, 21 Jun 2019 03:04:00 -0700 Subject: [PATCH] Keyboard accessibility follow up (#25274) Summary: This is a follow up PR to https://github.com/facebook/react-native/issues/24359. There's a good thread in the mentioned PR for more background for why I'm doing this change. Essentially `focusable` makes more sense since it is about whether a view can receive user-initiated focus from a pointer or keyboard. Pull Request resolved: https://github.com/facebook/react-native/pull/25274 Differential Revision: D15873739 Pulled By: cpojer fbshipit-source-id: 0f526bb99ecdc68131dfc10200a5d44c2ef75b33 --- Libraries/Components/Touchable/TouchableBounce.js | 4 ++-- Libraries/Components/Touchable/TouchableHighlight.js | 4 ++-- .../Touchable/TouchableNativeFeedback.android.js | 4 ++-- Libraries/Components/Touchable/TouchableOpacity.js | 4 ++-- Libraries/Components/Touchable/TouchableWithoutFeedback.js | 4 ++-- .../__snapshots__/TouchableHighlight-test.js.snap | 2 +- Libraries/Components/View/ViewPropTypes.js | 4 ++-- .../com/facebook/react/views/view/ReactViewManager.java | 7 ++++--- 8 files changed, 17 insertions(+), 16 deletions(-) diff --git a/Libraries/Components/Touchable/TouchableBounce.js b/Libraries/Components/Touchable/TouchableBounce.js index e9937dfd70a..51b31b78e43 100644 --- a/Libraries/Components/Touchable/TouchableBounce.js +++ b/Libraries/Components/Touchable/TouchableBounce.js @@ -185,8 +185,8 @@ const TouchableBounce = ((createReactClass({ nativeID={this.props.nativeID} testID={this.props.testID} hitSlop={this.props.hitSlop} - clickable={ - this.props.clickable !== false && + focusable={ + this.props.focusable !== false && this.props.onPress !== undefined && !this.props.disabled } diff --git a/Libraries/Components/Touchable/TouchableHighlight.js b/Libraries/Components/Touchable/TouchableHighlight.js index 5d733edb0a2..e5be44311f4 100644 --- a/Libraries/Components/Touchable/TouchableHighlight.js +++ b/Libraries/Components/Touchable/TouchableHighlight.js @@ -423,8 +423,8 @@ const TouchableHighlight = ((createReactClass({ nextFocusLeft={this.props.nextFocusLeft} nextFocusRight={this.props.nextFocusRight} nextFocusUp={this.props.nextFocusUp} - clickable={ - this.props.clickable !== false && this.props.onPress !== undefined + focusable={ + this.props.focusable !== false && this.props.onPress !== undefined } onClick={this.touchableHandlePress} onStartShouldSetResponder={this.touchableHandleStartShouldSetResponder} diff --git a/Libraries/Components/Touchable/TouchableNativeFeedback.android.js b/Libraries/Components/Touchable/TouchableNativeFeedback.android.js index 1f10ff412bd..8e652d084d3 100644 --- a/Libraries/Components/Touchable/TouchableNativeFeedback.android.js +++ b/Libraries/Components/Touchable/TouchableNativeFeedback.android.js @@ -326,8 +326,8 @@ const TouchableNativeFeedback = createReactClass({ nextFocusRight: this.props.nextFocusRight, nextFocusUp: this.props.nextFocusUp, hasTVPreferredFocus: this.props.hasTVPreferredFocus, - clickable: - this.props.clickable !== false && + focusable: + this.props.focusable !== false && this.props.onPress !== undefined && !this.props.disabled, onClick: this.touchableHandlePress, diff --git a/Libraries/Components/Touchable/TouchableOpacity.js b/Libraries/Components/Touchable/TouchableOpacity.js index adc6439584a..751d90842db 100644 --- a/Libraries/Components/Touchable/TouchableOpacity.js +++ b/Libraries/Components/Touchable/TouchableOpacity.js @@ -325,8 +325,8 @@ const TouchableOpacity = ((createReactClass({ hasTVPreferredFocus={this.props.hasTVPreferredFocus} tvParallaxProperties={this.props.tvParallaxProperties} hitSlop={this.props.hitSlop} - clickable={ - this.props.clickable !== false && this.props.onPress !== undefined + focusable={ + this.props.focusable !== false && this.props.onPress !== undefined } onClick={this.touchableHandlePress} onStartShouldSetResponder={this.touchableHandleStartShouldSetResponder} diff --git a/Libraries/Components/Touchable/TouchableWithoutFeedback.js b/Libraries/Components/Touchable/TouchableWithoutFeedback.js index 6323cee0ed9..ddf4570a083 100755 --- a/Libraries/Components/Touchable/TouchableWithoutFeedback.js +++ b/Libraries/Components/Touchable/TouchableWithoutFeedback.js @@ -262,8 +262,8 @@ const TouchableWithoutFeedback = ((createReactClass({ return (React: any).cloneElement(child, { ...overrides, accessible: this.props.accessible !== false, - clickable: - this.props.clickable !== false && this.props.onPress !== undefined, + focusable: + this.props.focusable !== false && this.props.onPress !== undefined, onClick: this.touchableHandlePress, onStartShouldSetResponder: this.touchableHandleStartShouldSetResponder, onResponderTerminationRequest: this diff --git a/Libraries/Components/Touchable/__tests__/__snapshots__/TouchableHighlight-test.js.snap b/Libraries/Components/Touchable/__tests__/__snapshots__/TouchableHighlight-test.js.snap index 00b32eed85e..3bbbc05d27c 100644 --- a/Libraries/Components/Touchable/__tests__/__snapshots__/TouchableHighlight-test.js.snap +++ b/Libraries/Components/Touchable/__tests__/__snapshots__/TouchableHighlight-test.js.snap @@ -3,7 +3,7 @@ exports[`TouchableHighlight renders correctly 1`] = ` { // handled in NativeViewHierarchyOptimizer } - @ReactProp(name = "clickable") - public void setClickable(final ReactViewGroup view, boolean clickable) { - if (clickable) { + @ReactProp(name = "focusable") + public void setFocusable(final ReactViewGroup view, boolean focusable) { + if (focusable) { view.setOnClickListener( new View.OnClickListener() { @Override @@ -245,6 +245,7 @@ public class ReactViewManager extends ViewGroupManager { else { view.setOnClickListener(null); view.setClickable(false); + // Don't set view.setFocusable(false) because we might still want it to be focusable for accessibiliy reasons } }