From baa61ddc9c088c70456fd4c4561a43b02bdfc428 Mon Sep 17 00:00:00 2001 From: Jay Phelps Date: Mon, 11 Jun 2018 15:11:01 -0700 Subject: [PATCH] Trigger onFocus/onBlur instead of onPressIn/onPressOut (eventually, but for now just deprecate) (#18470) Summary: Currently on iOS and Android focus/blur events trigger onPressIn/onPressOut. Based on discussions with people are several companies who use react-native we're proposing instead triggering new events onFocus/onBlur. Initial discussion on Slack with some from the core team on Slack seemed positive. Couple reasons: * The current API behavior overloads onPressIn/onPressOut. That means on platforms like react-native-web, if focus/blur support was added (as we're hoping for), even though onPressIn/onPressOut would be useful as the name describes, you wouldn't be able to distinguish between it and browser element focus/blur events. * The names aren't as self-documenting/intuitive as onFocus/onBlur, especially for react-dom users. There aren't any current tests around this, but I intend to add them if we solidify the API. There's also an option question on the transition--do we deprecate the existing API with a warning? This PR just deprecates them, though it will on any TV platform when something becomes focused regardless of whether they use the API or not. This isn't ideal. It's not clear if there are alternatives or if just right away breaking the API for TV users is the correct solution, if we can get consensus between the few parties who are using it. *** I'm interested to hear counter points or prior discussions. Cc/ matthargett dlowder-salesforce rozele Closes https://github.com/facebook/react-native/pull/18470 Differential Revision: D8368109 Pulled By: hramos fbshipit-source-id: 22587b82e091645e748b6c2d721fdff06d54837f --- Libraries/Components/Touchable/Touchable.js | 31 +++++++++++++++++-- .../Components/Touchable/TouchableBounce.js | 1 - .../Touchable/TouchableWithoutFeedback.js | 12 +++++++ 3 files changed, 41 insertions(+), 3 deletions(-) diff --git a/Libraries/Components/Touchable/Touchable.js b/Libraries/Components/Touchable/Touchable.js index ce449d6d99..1af0887d46 100644 --- a/Libraries/Components/Touchable/Touchable.js +++ b/Libraries/Components/Touchable/Touchable.js @@ -325,11 +325,17 @@ const TouchableMixin = { evt.dispatchConfig = {}; if (myTag === evt.tag) { if (evt.eventType === 'focus') { - cmp.touchableHandleActivePressIn && + if (cmp.touchableHandleActivePressIn) { + console.warn('Using `touchableHandleActivePressIn` or `onPressIn` to listen for focus events is deprecated. Use `touchableHandleFocus` or `onFocus` instead.'); cmp.touchableHandleActivePressIn(evt); + } + cmp.touchableHandleFocus(evt); } else if (evt.eventType === 'blur') { - cmp.touchableHandleActivePressOut && + if (cmp.touchableHandleActivePressOut) { + console.warn('Using `touchableHandleActivePressOut` or `onPressOut` to listen for blur events is deprecated. Use `touchableHandleBlur` or `onBlur` instead.'); cmp.touchableHandleActivePressOut(evt); + } + cmp.touchableHandleBlur(evt); } else if (evt.eventType === 'select') { cmp.touchableHandlePress && !cmp.props.disabled && @@ -528,6 +534,27 @@ const TouchableMixin = { } }, + /** + * Invoked when the item receives focus. Mixers might override this to + * visually distinguish the `VisualRect` so that the user knows that it + * currently has the focus. Most platforms only support a single element being + * focused at a time, in which case there may have been a previously focused + * element that was blurred just prior to this. + */ + touchableHandleFocus: function (e: Event) { + this.props.onFocus && this.props.onFocus(e); + }, + + /** + * Invoked when the item loses focus. Mixers might override this to + * visually distinguish the `VisualRect` so that the user knows that it + * no longer has focus. Most platforms only support a single element being + * focused at a time, in which case the focus may have moved to another. + */ + touchableHandleBlur: function (e: Event) { + this.props.onBlur && this.props.onBlur(e); + }, + // ==== Abstract Application Callbacks ==== /** diff --git a/Libraries/Components/Touchable/TouchableBounce.js b/Libraries/Components/Touchable/TouchableBounce.js index 4a61ef0bc8..c899748b44 100644 --- a/Libraries/Components/Touchable/TouchableBounce.js +++ b/Libraries/Components/Touchable/TouchableBounce.js @@ -56,7 +56,6 @@ const TouchableBounce = ((createReactClass({ propTypes: { ...TouchableWithoutFeedback.propTypes, - // The function passed takes a callback to start the animation which should // be run after this onPress handler is done. You can use this (for example) // to update UI before starting the animation. diff --git a/Libraries/Components/Touchable/TouchableWithoutFeedback.js b/Libraries/Components/Touchable/TouchableWithoutFeedback.js index 2cd249cb5b..9228d271a9 100755 --- a/Libraries/Components/Touchable/TouchableWithoutFeedback.js +++ b/Libraries/Components/Touchable/TouchableWithoutFeedback.js @@ -80,6 +80,18 @@ const TouchableWithoutFeedback = ((createReactClass({ PropTypes.oneOf(AccessibilityTraits), PropTypes.arrayOf(PropTypes.oneOf(AccessibilityTraits)), ]), + /** + * When `accessible` is true (which is the default) this may be called when + * the OS-specific concept of "focus" occurs. Some platforms may not have + * the concept of focus. + */ + onFocus: PropTypes.func, + /** + * When `accessible` is true (which is the default) this may be called when + * the OS-specific concept of "blur" occurs, meaning the element lost focus. + * Some platforms may not have the concept of blur. + */ + onBlur: PropTypes.func, /** * If true, disable all interactions for this component. */