From e10134e7baa1b1f2a6b7db38be425aefb52359ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Kwa=C5=9Bniewski?= Date: Fri, 23 Feb 2024 15:26:33 +0100 Subject: [PATCH] chore: remove hover effect & add cursor: pointer (#122) * chore: remove hover effect * fix: set cursor pointer by default, update tests * feat(iOS): Implement cursor style prop # Conflicts: # packages/react-native/Libraries/StyleSheet/StyleSheetTypes.d.ts --------- Co-authored-by: Saad Najmi # Conflicts: # packages/react-native/Libraries/Components/Pressable/__tests__/__snapshots__/Pressable-test.js.snap # packages/react-native/Libraries/Components/Touchable/TouchableHighlight.js # packages/react-native/Libraries/Components/Touchable/TouchableOpacity.js --- .../Components/Pressable/Pressable.js | 27 +++--- .../__snapshots__/Pressable-test.js.snap | 80 +++++++++++++++++ .../Touchable/TouchableHighlight.js | 24 ++--- .../Touchable/TouchableOpacity.d.ts | 7 +- .../Components/Touchable/TouchableOpacity.js | 22 ++--- .../TouchableHighlight-test.js.snap | 49 ++++++++++- .../TouchableOpacity-test.js.snap | 3 + .../View/ReactNativeStyleAttributes.js | 1 + .../Components/View/ViewPropTypes.d.ts | 6 -- .../Components/View/ViewPropTypes.js | 7 -- .../__snapshots__/Button-test.js.snap | 9 ++ .../Libraries/StyleSheet/StyleSheetTypes.d.ts | 4 + .../Libraries/StyleSheet/StyleSheetTypes.js | 3 + .../__snapshots__/public-api-test.js.snap | 2 + packages/react-native/React/Base/RCTConvert.h | 3 + packages/react-native/React/Base/RCTConvert.m | 9 ++ .../View/RCTViewComponentView.mm | 73 +++++++-------- packages/react-native/React/Views/RCTCursor.h | 14 +++ packages/react-native/React/Views/RCTView.h | 8 +- packages/react-native/React/Views/RCTView.m | 67 ++++++-------- .../react-native/React/Views/RCTViewManager.m | 3 +- .../components/view/BaseViewProps.cpp | 23 ++--- .../renderer/components/view/BaseViewProps.h | 6 +- .../renderer/components/view/conversions.h | 22 +++++ .../renderer/components/view/primitives.h | 2 + .../js/examples/Cursor/CursorExample.js | 88 +++++++++++++++++++ .../rn-tester/js/utils/RNTesterList.ios.js | 4 + 27 files changed, 402 insertions(+), 164 deletions(-) create mode 100644 packages/react-native/React/Views/RCTCursor.h create mode 100644 packages/rn-tester/js/examples/Cursor/CursorExample.js diff --git a/packages/react-native/Libraries/Components/Pressable/Pressable.js b/packages/react-native/Libraries/Components/Pressable/Pressable.js index 7453c325ea748b..15e850f49c2d97 100644 --- a/packages/react-native/Libraries/Components/Pressable/Pressable.js +++ b/packages/react-native/Libraries/Components/Pressable/Pressable.js @@ -20,11 +20,11 @@ import type { AccessibilityState, AccessibilityValue, } from '../View/ViewAccessibility'; -import type {HoverEffect} from '../View/ViewPropTypes'; import {PressabilityDebugView} from '../../Pressability/PressabilityDebug'; import usePressability from '../../Pressability/usePressability'; import {type RectOrSize} from '../../StyleSheet/Rect'; +import StyleSheet from '../../StyleSheet/StyleSheet'; import useMergeRefs from '../../Utilities/useMergeRefs'; import View from '../View/View'; import useAndroidRippleForView, { @@ -33,18 +33,12 @@ import useAndroidRippleForView, { import * as React from 'react'; import {useImperativeHandle, useMemo, useRef, useState} from 'react'; -const defaultHoverEffect: HoverEffect = 'highlight'; - type ViewStyleProp = $ElementType, 'style'>; export type StateCallbackType = $ReadOnly<{| pressed: boolean, |}>; -type VisionOSProps = $ReadOnly<{| - visionos_hoverEffect?: ?HoverEffect, -|}>; - type Props = $ReadOnly<{| /** * Accessibility. @@ -200,10 +194,6 @@ type Props = $ReadOnly<{| * https://github.com/facebook/react-native/issues/34424 */ 'aria-label'?: ?string, - /** - * Props needed for visionOS. - */ - ...VisionOSProps, |}>; /** @@ -243,7 +233,6 @@ function Pressable(props: Props, forwardedRef): React.Node { style, testOnly_pressed, unstable_pressDelay, - visionos_hoverEffect = defaultHoverEffect, ...restProps } = props; @@ -352,15 +341,23 @@ function Pressable(props: Props, forwardedRef): React.Node { {...restPropsWithDefaults} {...eventHandlers} ref={mergedRef} - style={typeof style === 'function' ? style({pressed}) : style} - collapsable={false} - visionos_hoverEffect={visionos_hoverEffect}> + style={[ + styles.pressable, + typeof style === 'function' ? style({pressed}) : style, + ]} + collapsable={false}> {typeof children === 'function' ? children({pressed}) : children} {__DEV__ ? : null} ); } +const styles = StyleSheet.create({ + pressable: { + cursor: 'pointer', + }, +}); + function usePressState(forcePressed: boolean): [boolean, (boolean) => void] { const [pressed, setPressed] = useState(false); return [pressed || forcePressed, setPressed]; diff --git a/packages/react-native/Libraries/Components/Pressable/__tests__/__snapshots__/Pressable-test.js.snap b/packages/react-native/Libraries/Components/Pressable/__tests__/__snapshots__/Pressable-test.js.snap index e348e02347e52b..2f9286b98987d6 100644 --- a/packages/react-native/Libraries/Components/Pressable/__tests__/__snapshots__/Pressable-test.js.snap +++ b/packages/react-native/Libraries/Components/Pressable/__tests__/__snapshots__/Pressable-test.js.snap @@ -31,6 +31,14 @@ exports[` should render as expected: should deep render when mocked onResponderTerminate={[Function]} onResponderTerminationRequest={[Function]} onStartShouldSetResponder={[Function]} + style={ + Array [ + Object { + "cursor": "pointer", + }, + undefined, + ] + } > @@ -67,6 +75,14 @@ exports[` should render as expected: should deep render when not mo onResponderTerminate={[Function]} onResponderTerminationRequest={[Function]} onStartShouldSetResponder={[Function]} + style={ + Array [ + Object { + "cursor": "pointer", + }, + undefined, + ] + } > @@ -115,6 +131,14 @@ exports[` should be disabled when disabled is true: onResponderTerminate={[Function]} onResponderTerminationRequest={[Function]} onStartShouldSetResponder={[Function]} + style={ + Array [ + Object { + "cursor": "pointer", + }, + undefined, + ] + } > @@ -151,6 +175,14 @@ exports[` should be disabled when disabled is true: onResponderTerminate={[Function]} onResponderTerminationRequest={[Function]} onStartShouldSetResponder={[Function]} + style={ + Array [ + Object { + "cursor": "pointer", + }, + undefined, + ] + } > @@ -203,6 +235,14 @@ exports[` should be disable onResponderTerminate={[Function]} onResponderTerminationRequest={[Function]} onStartShouldSetResponder={[Function]} + style={ + Array [ + Object { + "cursor": "pointer", + }, + undefined, + ] + } > @@ -239,6 +279,14 @@ exports[` should be disable onResponderTerminate={[Function]} onResponderTerminationRequest={[Function]} onStartShouldSetResponder={[Function]} + style={ + Array [ + Object { + "cursor": "pointer", + }, + undefined, + ] + } > @@ -293,6 +341,14 @@ exports[` shou onResponderTerminate={[Function]} onResponderTerminationRequest={[Function]} onStartShouldSetResponder={[Function]} + style={ + Array [ + Object { + "cursor": "pointer", + }, + undefined, + ] + } > @@ -329,6 +385,14 @@ exports[` shou onResponderTerminate={[Function]} onResponderTerminationRequest={[Function]} onStartShouldSetResponder={[Function]} + style={ + Array [ + Object { + "cursor": "pointer", + }, + undefined, + ] + } > @@ -391,6 +455,14 @@ exports[` sh onResponderTerminate={[Function]} onResponderTerminationRequest={[Function]} onStartShouldSetResponder={[Function]} + style={ + Array [ + Object { + "cursor": "pointer", + }, + undefined, + ] + } > @@ -427,6 +499,14 @@ exports[` sh onResponderTerminate={[Function]} onResponderTerminationRequest={[Function]} onStartShouldSetResponder={[Function]} + style={ + Array [ + Object { + "cursor": "pointer", + }, + undefined, + ] + } > diff --git a/packages/react-native/Libraries/Components/Touchable/TouchableHighlight.js b/packages/react-native/Libraries/Components/Touchable/TouchableHighlight.js index 6365cd2482f333..e9cca3342bfc64 100644 --- a/packages/react-native/Libraries/Components/Touchable/TouchableHighlight.js +++ b/packages/react-native/Libraries/Components/Touchable/TouchableHighlight.js @@ -9,7 +9,6 @@ */ import type {ColorValue} from '../../StyleSheet/StyleSheet'; -import type {HoverEffect} from '../View/ViewPropTypes'; import typeof TouchableWithoutFeedback from './TouchableWithoutFeedback'; import View from '../../Components/View/View'; @@ -33,15 +32,10 @@ type IOSProps = $ReadOnly<{| hasTVPreferredFocus?: ?boolean, |}>; -type VisionOSProps = $ReadOnly<{| - hoverEffect?: ?HoverEffect, -|}>; - type Props = $ReadOnly<{| ...React.ElementConfig, ...AndroidProps, ...IOSProps, - ...VisionOSProps, activeOpacity?: ?number, underlayColor?: ?ColorValue, @@ -335,10 +329,13 @@ class TouchableHighlight extends React.Component { accessibilityElementsHidden={ this.props['aria-hidden'] ?? this.props.accessibilityElementsHidden } - style={StyleSheet.compose( - this.props.style, - this.state.extraStyles?.underlay, - )} + style={[ + styles.touchable, + StyleSheet.compose( + this.props.style, + this.state.extraStyles?.underlay, + ), + ]} onLayout={this.props.onLayout} hitSlop={this.props.hitSlop} hasTVPreferredFocus={this.props.hasTVPreferredFocus} @@ -347,7 +344,6 @@ class TouchableHighlight extends React.Component { nextFocusLeft={this.props.nextFocusLeft} nextFocusRight={this.props.nextFocusRight} nextFocusUp={this.props.nextFocusUp} - visionos_hoverEffect={this.props.hoverEffect} focusable={ this.props.focusable !== false && this.props.onPress !== undefined } @@ -386,6 +382,12 @@ class TouchableHighlight extends React.Component { } } +const styles = StyleSheet.create({ + touchable: { + cursor: 'pointer', + }, +}); + const Touchable = (React.forwardRef((props, hostRef) => ( )): React.AbstractComponent< diff --git a/packages/react-native/Libraries/Components/Touchable/TouchableOpacity.d.ts b/packages/react-native/Libraries/Components/Touchable/TouchableOpacity.d.ts index f9b2e6a07f0df4..37c4ce6df43e4c 100644 --- a/packages/react-native/Libraries/Components/Touchable/TouchableOpacity.d.ts +++ b/packages/react-native/Libraries/Components/Touchable/TouchableOpacity.d.ts @@ -11,7 +11,7 @@ import type * as React from 'react'; import {Constructor} from '../../../types/private/Utilities'; import {TimerMixin} from '../../../types/private/TimerMixin'; import {NativeMethods} from '../../../types/public/ReactNativeTypes'; -import {HoverEffect, TVParallaxProperties} from '../View/ViewPropTypes'; +import {TVParallaxProperties} from '../View/ViewPropTypes'; import {TouchableMixin} from './Touchable'; import {TouchableWithoutFeedbackProps} from './TouchableWithoutFeedback'; @@ -86,11 +86,6 @@ export interface TouchableOpacityProps * @platform android */ tvParallaxProperties?: TVParallaxProperties | undefined; - - /** - * Hover style to apply to the view. Only supported on visionOS. - */ - visionos_hoverEffect?: HoverEffect | undefined; } /** diff --git a/packages/react-native/Libraries/Components/Touchable/TouchableOpacity.js b/packages/react-native/Libraries/Components/Touchable/TouchableOpacity.js index 6214a6817945ec..78f344488f7bea 100644 --- a/packages/react-native/Libraries/Components/Touchable/TouchableOpacity.js +++ b/packages/react-native/Libraries/Components/Touchable/TouchableOpacity.js @@ -9,7 +9,6 @@ */ import type {ViewStyleProp} from '../../StyleSheet/StyleSheet'; -import type {HoverEffect} from '../View/ViewPropTypes'; import typeof TouchableWithoutFeedback from './TouchableWithoutFeedback'; import Animated from '../../Animated/Animated'; @@ -19,11 +18,10 @@ import Pressability, { } from '../../Pressability/Pressability'; import {PressabilityDebugView} from '../../Pressability/PressabilityDebug'; import flattenStyle from '../../StyleSheet/flattenStyle'; +import StyleSheet from '../../StyleSheet/StyleSheet'; import Platform from '../../Utilities/Platform'; import * as React from 'react'; -const defaultHoverEffect: HoverEffect = 'highlight'; - type TVProps = $ReadOnly<{| hasTVPreferredFocus?: ?boolean, nextFocusDown?: ?number, @@ -33,14 +31,9 @@ type TVProps = $ReadOnly<{| nextFocusUp?: ?number, |}>; -type VisionOSProps = $ReadOnly<{| - visionos_hoverEffect?: ?HoverEffect, -|}>; - type Props = $ReadOnly<{| ...React.ElementConfig, ...TVProps, - ...VisionOSProps, activeOpacity?: ?number, style?: ?ViewStyleProp, @@ -138,10 +131,6 @@ type State = $ReadOnly<{| * */ class TouchableOpacity extends React.Component { - static defaultProps: {|visionos_hoverEffect: HoverEffect|} = { - visionos_hoverEffect: defaultHoverEffect, - }; - state: State = { anim: new Animated.Value(this._getChildStyleOpacityWithDefault()), pressability: new Pressability(this._createPressabilityConfig()), @@ -287,7 +276,7 @@ class TouchableOpacity extends React.Component { accessibilityElementsHidden={ this.props['aria-hidden'] ?? this.props.accessibilityElementsHidden } - style={[this.props.style, {opacity: this.state.anim}]} + style={[styles.touchable, this.props.style, {opacity: this.state.anim}]} nativeID={this.props.id ?? this.props.nativeID} testID={this.props.testID} onLayout={this.props.onLayout} @@ -298,7 +287,6 @@ class TouchableOpacity extends React.Component { nextFocusUp={this.props.nextFocusUp} hasTVPreferredFocus={this.props.hasTVPreferredFocus} hitSlop={this.props.hitSlop} - visionos_hoverEffect={this.props.visionos_hoverEffect} focusable={ this.props.focusable !== false && this.props.onPress !== undefined } @@ -336,6 +324,12 @@ class TouchableOpacity extends React.Component { } } +const styles = StyleSheet.create({ + touchable: { + cursor: 'pointer', + }, +}); + const Touchable = (React.forwardRef((props, ref) => ( )): React.AbstractComponent>); diff --git a/packages/react-native/Libraries/Components/Touchable/__tests__/__snapshots__/TouchableHighlight-test.js.snap b/packages/react-native/Libraries/Components/Touchable/__tests__/__snapshots__/TouchableHighlight-test.js.snap index 35c845e97493fc..f26560d45d738a 100644 --- a/packages/react-native/Libraries/Components/Touchable/__tests__/__snapshots__/TouchableHighlight-test.js.snap +++ b/packages/react-native/Libraries/Components/Touchable/__tests__/__snapshots__/TouchableHighlight-test.js.snap @@ -19,7 +19,14 @@ exports[`TouchableHighlight renders correctly 1`] = ` onResponderTerminate={[Function]} onResponderTerminationRequest={[Function]} onStartShouldSetResponder={[Function]} - style={Object {}} + style={ + Array [ + Object { + "cursor": "pointer", + }, + Object {}, + ] + } > Touchable @@ -51,6 +58,14 @@ exports[`TouchableHighlight with disabled state should be disabled when disabled onResponderTerminate={[Function]} onResponderTerminationRequest={[Function]} onStartShouldSetResponder={[Function]} + style={ + Array [ + Object { + "cursor": "pointer", + }, + undefined, + ] + } > @@ -80,6 +95,14 @@ exports[`TouchableHighlight with disabled state should be disabled when disabled onResponderTerminate={[Function]} onResponderTerminationRequest={[Function]} onStartShouldSetResponder={[Function]} + style={ + Array [ + Object { + "cursor": "pointer", + }, + undefined, + ] + } > @@ -109,6 +132,14 @@ exports[`TouchableHighlight with disabled state should disable button when acces onResponderTerminate={[Function]} onResponderTerminationRequest={[Function]} onStartShouldSetResponder={[Function]} + style={ + Array [ + Object { + "cursor": "pointer", + }, + undefined, + ] + } > @@ -139,6 +170,14 @@ exports[`TouchableHighlight with disabled state should keep accessibilityState w onResponderTerminate={[Function]} onResponderTerminationRequest={[Function]} onStartShouldSetResponder={[Function]} + style={ + Array [ + Object { + "cursor": "pointer", + }, + undefined, + ] + } > @@ -168,6 +207,14 @@ exports[`TouchableHighlight with disabled state should overwrite accessibilitySt onResponderTerminate={[Function]} onResponderTerminationRequest={[Function]} onStartShouldSetResponder={[Function]} + style={ + Array [ + Object { + "cursor": "pointer", + }, + undefined, + ] + } > diff --git a/packages/react-native/Libraries/Components/Touchable/__tests__/__snapshots__/TouchableOpacity-test.js.snap b/packages/react-native/Libraries/Components/Touchable/__tests__/__snapshots__/TouchableOpacity-test.js.snap index 17f2e7f6f764e0..ca634909ca5afc 100644 --- a/packages/react-native/Libraries/Components/Touchable/__tests__/__snapshots__/TouchableOpacity-test.js.snap +++ b/packages/react-native/Libraries/Components/Touchable/__tests__/__snapshots__/TouchableOpacity-test.js.snap @@ -31,6 +31,7 @@ exports[`TouchableOpacity renders correctly 1`] = ` onStartShouldSetResponder={[Function]} style={ Object { + "cursor": "pointer", "opacity": 1, } } @@ -72,6 +73,7 @@ exports[`TouchableOpacity renders in disabled state when a disabled prop is pass onStartShouldSetResponder={[Function]} style={ Object { + "cursor": "pointer", "opacity": 1, } } @@ -113,6 +115,7 @@ exports[`TouchableOpacity renders in disabled state when a key disabled in acces onStartShouldSetResponder={[Function]} style={ Object { + "cursor": "pointer", "opacity": 1, } } diff --git a/packages/react-native/Libraries/Components/View/ReactNativeStyleAttributes.js b/packages/react-native/Libraries/Components/View/ReactNativeStyleAttributes.js index 79dbeddb741e39..de77b65aaba7f5 100644 --- a/packages/react-native/Libraries/Components/View/ReactNativeStyleAttributes.js +++ b/packages/react-native/Libraries/Components/View/ReactNativeStyleAttributes.js @@ -144,6 +144,7 @@ const ReactNativeStyleAttributes: {[string]: AnyAttributeType, ...} = { borderTopLeftRadius: true, borderTopRightRadius: true, borderTopStartRadius: true, + cursor: true, opacity: true, pointerEvents: true, diff --git a/packages/react-native/Libraries/Components/View/ViewPropTypes.d.ts b/packages/react-native/Libraries/Components/View/ViewPropTypes.d.ts index 1e700fa925c99c..53f5e82c34149b 100644 --- a/packages/react-native/Libraries/Components/View/ViewPropTypes.d.ts +++ b/packages/react-native/Libraries/Components/View/ViewPropTypes.d.ts @@ -16,8 +16,6 @@ import {LayoutChangeEvent, PointerEvents} from '../../Types/CoreEventTypes'; import {Touchable} from '../Touchable/Touchable'; import {AccessibilityProps} from './ViewAccessibility'; -export type HoverEffect = 'lift' | 'highlight'; - export type TVParallaxProperties = { /** * If true, parallax effects are enabled. Defaults to true. @@ -124,10 +122,6 @@ export interface ViewPropsIOS extends TVViewPropsIOS { * Test and measure when using this property. */ shouldRasterizeIOS?: boolean | undefined; - /** - * Hover style to apply to the view. Only supported on visionOS. - */ - visionos_hoverEffect?: HoverEffect | undefined; } export interface ViewPropsAndroid { diff --git a/packages/react-native/Libraries/Components/View/ViewPropTypes.js b/packages/react-native/Libraries/Components/View/ViewPropTypes.js index e3f0229ee877d4..1ead9ba053a51a 100644 --- a/packages/react-native/Libraries/Components/View/ViewPropTypes.js +++ b/packages/react-native/Libraries/Components/View/ViewPropTypes.js @@ -263,8 +263,6 @@ type AndroidDrawableRipple = $ReadOnly<{| rippleRadius?: ?number, |}>; -export type HoverEffect = 'lift' | 'highlight'; - type AndroidDrawable = AndroidDrawableThemeAttr | AndroidDrawableRipple; type AndroidViewProps = $ReadOnly<{| @@ -438,11 +436,6 @@ type IOSViewProps = $ReadOnly<{| * See https://reactnative.dev/docs/view#shouldrasterizeios */ shouldRasterizeIOS?: ?boolean, - - /** - * Hover style to apply to the view. Only supported on visionOS. - */ - visionos_hoverEffect?: ?HoverEffect, |}>; export type ViewProps = $ReadOnly<{| diff --git a/packages/react-native/Libraries/Components/__tests__/__snapshots__/Button-test.js.snap b/packages/react-native/Libraries/Components/__tests__/__snapshots__/Button-test.js.snap index 5b4294e0e8c850..a3efb80e477bfc 100644 --- a/packages/react-native/Libraries/Components/__tests__/__snapshots__/Button-test.js.snap +++ b/packages/react-native/Libraries/Components/__tests__/__snapshots__/Button-test.js.snap @@ -32,6 +32,7 @@ exports[`