From e43c6b922c25732692b28408c345590bdc45073e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Inka=20Soko=C5=82owska?= Date: Fri, 11 Oct 2024 11:35:00 +0200 Subject: [PATCH] [native] Fix NUX Android problem Summary: issue: [[https://linear.app/comm/issue/ENG-9472/nux-appears-outside-of-safe-area-on-android | ENG-9472]] On some Androids, we cannot measure a button that is not visible on the screen. `undefined`s are returned. So we have to wait of the login screen to be dismissed, before we call `measure`. I wanted to pass the `measure` function itseft through conxtext, but that results in `Cannot read property '_nativeTag' of undefined` error. Not sure what that means, but I don't think passing the ref is much worse. `measure` asynchronously measures the component and then calls the callback. This is by I need states and an effect, instead of a memo. `buttonMeasured` flag ensures the tip doesn't jump after the `measure` is done. `measure` returns quickly and I don't think this slight delay is a problem. Test Plan: Tested on Android 11 physical device, and iOS 17.4 simulator. Tesdted by updating `NUXHandlerInner` to run the effect on every app reaload. Checked that on both Android and iOS the NUX tips are displayed in the correct position. Checked thare are no odd visual effects, like jumping (thanks to `buttonMeasured`) Reviewers: tomek, kamil Reviewed By: tomek Subscribers: ashoat Differential Revision: https://phab.comm.dev/D13697 --- native/chat/chat-tab-bar.react.js | 43 +++---- native/chat/chat.react.js | 36 +++--- native/components/nux-tips-context.react.js | 10 +- native/tooltip/nux-tips-overlay.react.js | 132 ++++++++++++-------- 4 files changed, 122 insertions(+), 99 deletions(-) diff --git a/native/chat/chat-tab-bar.react.js b/native/chat/chat-tab-bar.react.js index 6e183ea48d..c39a9cf15e 100644 --- a/native/chat/chat-tab-bar.react.js +++ b/native/chat/chat-tab-bar.react.js @@ -9,8 +9,11 @@ import { MaterialTopTabBar } from '@react-navigation/material-top-tabs'; import invariant from 'invariant'; import * as React from 'react'; import { View } from 'react-native'; +import type { MeasureOnSuccessCallback } from 'react-native/Libraries/Renderer/shims/ReactNativeTypes'; import { TabBarItem } from 'react-native-tab-view'; +import type { ReactRefSetter } from 'lib/types/react-types.js'; + import { nuxTip, NUXTipsContext, @@ -25,35 +28,29 @@ const ButtonTitleToTip = Object.freeze({ [HomeChatThreadListRouteName]: nuxTip.HOME, }); +const onLayout = () => {}; + function TabBarButton(props: TabBarItemProps>) { const tipsContext = React.useContext(NUXTipsContext); invariant(tipsContext, 'NUXTipsContext should be defined'); + const { registerTipButton } = tipsContext; - const viewRef = React.useRef>(); - const onLayout = React.useCallback(() => { - const button = viewRef.current; - if (!button) { - return; - } - - const tipType = ButtonTitleToTip[props.route.name]; - if (!tipType) { - return; - } - button.measure((x, y, width, height, pageX, pageY) => { - tipsContext.registerTipButton(tipType, { - x, - y, - width, - height, - pageX, - pageY, - }); - }); - }, [props.route.name, tipsContext]); + const registerRef: ReactRefSetter> = + React.useCallback( + element => { + const tipType = ButtonTitleToTip[props.route.name]; + if (!tipType) { + return; + } + const measure = (callback: MeasureOnSuccessCallback) => + element?.measure(callback); + registerTipButton(tipType, measure); + }, + [props.route.name, registerTipButton], + ); return ( - + ); diff --git a/native/chat/chat.react.js b/native/chat/chat.react.js index beb24ac487..25a58d77fa 100644 --- a/native/chat/chat.react.js +++ b/native/chat/chat.react.js @@ -25,12 +25,14 @@ import { StackView } from '@react-navigation/stack'; import invariant from 'invariant'; import * as React from 'react'; import { Platform, View, useWindowDimensions } from 'react-native'; +import type { MeasureOnSuccessCallback } from 'react-native/Libraries/Renderer/shims/ReactNativeTypes'; import MessageStorePruner from 'lib/components/message-store-pruner.react.js'; import ThreadDraftUpdater from 'lib/components/thread-draft-updater.react.js'; import { isLoggedIn } from 'lib/selectors/user-selectors.js'; import { threadSettingsNotificationsCopy } from 'lib/shared/thread-settings-notifications-utils.js'; import { threadIsPending, threadIsSidebar } from 'lib/shared/thread-utils.js'; +import type { ReactRefSetter } from 'lib/types/react-types.js'; import BackgroundChatThreadList from './background-chat-thread-list.react.js'; import ChatHeader from './chat-header.react.js'; @@ -362,6 +364,8 @@ const Chat = createChatNavigator< ChatNavigationHelpers, >(); +const communityDrawerButtonOnLayout = () => {}; + type Props = { +navigation: TabNavigationProp<'Chat'>, ... @@ -375,27 +379,21 @@ export default function ChatComponent(props: Props): React.Node { draftUpdater = ; } - const communityDrawerButtonRef = - React.useRef>(); - const tipsContext = React.useContext(NUXTipsContext); invariant(tipsContext, 'NUXTipsContext should be defined'); const { registerTipButton } = tipsContext; - const communityDrawerButtonOnLayout = React.useCallback(() => { - communityDrawerButtonRef.current?.measure( - (x, y, width, height, pageX, pageY) => { - registerTipButton(nuxTip.COMMUNITY_DRAWER, { - x, - y, - width, - height, - pageX, - pageY, - }); - }, - ); - }, [registerTipButton]); + const communityDrawerButtonRegisterRef: ReactRefSetter< + React.ElementRef, + > = React.useCallback( + element => { + const measure = (callback: MeasureOnSuccessCallback) => + element?.measure(callback); + + registerTipButton(nuxTip.COMMUNITY_DRAWER, measure); + }, + [registerTipButton], + ); const headerLeftButton = React.useCallback( (headerProps: StackHeaderLeftButtonProps) => { @@ -405,13 +403,13 @@ export default function ChatComponent(props: Props): React.Node { return ( ); }, - [communityDrawerButtonOnLayout, props.navigation], + [communityDrawerButtonRegisterRef, props.navigation], ); const messageEditingContext = React.useContext(MessageEditingContext); diff --git a/native/components/nux-tips-context.react.js b/native/components/nux-tips-context.react.js index 92f9986549..3f84388e08 100644 --- a/native/components/nux-tips-context.react.js +++ b/native/components/nux-tips-context.react.js @@ -1,6 +1,7 @@ // @flow import * as React from 'react'; +import type { MeasureOnSuccessCallback } from 'react-native/Libraries/Renderer/shims/ReactNativeTypes'; import { values } from 'lib/utils/objects.js'; @@ -61,14 +62,7 @@ function getNUXTipParams(currentTipKey: NUXTip): NUXTipParams { return nuxTipParams[currentTipKey]; } -type TipProps = { - +x: number, - +y: number, - +width: number, - +height: number, - +pageX: number, - +pageY: number, -}; +type TipProps = (callback: MeasureOnSuccessCallback) => void; export type NUXTipsContextType = { +registerTipButton: (type: NUXTip, tipProps: ?TipProps) => void, diff --git a/native/tooltip/nux-tips-overlay.react.js b/native/tooltip/nux-tips-overlay.react.js index b50b314459..c4c28a2f6b 100644 --- a/native/tooltip/nux-tips-overlay.react.js +++ b/native/tooltip/nux-tips-overlay.react.js @@ -121,23 +121,38 @@ function createNUXTipsOverlay( const dimensions = useSelector(state => state.dimensions); - const { initialCoordinates, verticalBounds } = React.useMemo(() => { + const [coordinates, setCoordinates] = React.useState(null); + + React.useEffect(() => { if (!ButtonComponent) { - const y = (dimensions.height * 2) / 5; - const x = dimensions.width / 2; - return { - initialCoordinates: { height: 0, width: 0, x, y }, - verticalBounds: { height: 0, y }, - }; + const yInitial = (dimensions.height * 2) / 5; + const xInitial = dimensions.width / 2; + setCoordinates({ + initialCoordinates: { height: 0, width: 0, x: xInitial, y: yInitial }, + verticalBounds: { height: 0, y: yInitial }, + }); + return; } - const tipProps = nuxTipContext?.tipsProps?.[route.params.tipKey]; - invariant(tipProps, 'button should be registered with nuxTipContext'); - const { pageX, pageY, width, height } = tipProps; - - return { - initialCoordinates: { height, width, x: pageX, y: pageY }, - verticalBounds: { height, y: pageY }, - }; + const measure = nuxTipContext?.tipsProps?.[route.params.tipKey]; + invariant(measure, 'button should be registered with nuxTipContext'); + + measure((x, y, width, height, pageX, pageY) => + setCoordinates({ + initialCoordinates: { height, width, x: pageX, y: pageY }, + verticalBounds: { height, y: pageY }, + }), + ); }, [dimensions, nuxTipContext?.tipsProps, route.params.tipKey]); const overlayContext = React.useContext(OverlayContext); @@ -149,38 +164,44 @@ function createNUXTipsOverlay( const styles = useStyles(unboundStyles); const contentContainerStyle = React.useMemo(() => { + if (!coordinates) { + return {}; + } const fullScreenHeight = dimensions.height; - const top = verticalBounds.y; + const top = coordinates.verticalBounds.y; const bottom = - fullScreenHeight - verticalBounds.y - verticalBounds.height; + fullScreenHeight - + coordinates.verticalBounds.y - + coordinates.verticalBounds.height; return { ...styles.contentContainer, marginTop: top, marginBottom: bottom, }; - }, [ - dimensions.height, - styles.contentContainer, - verticalBounds.height, - verticalBounds.y, - ]); + }, [dimensions.height, styles.contentContainer, coordinates]); const buttonStyle = React.useMemo(() => { - const { x, y, width, height } = initialCoordinates; + if (!coordinates) { + return {}; + } + const { x, y, width, height } = coordinates.initialCoordinates; return { width: Math.ceil(width), height: Math.ceil(height), - marginTop: y - verticalBounds.y, + marginTop: y - coordinates.verticalBounds.y, marginLeft: x, }; - }, [initialCoordinates, verticalBounds]); + }, [coordinates]); const tipHorizontalOffsetRef = React.useRef(new Value(0)); const tipHorizontalOffset = tipHorizontalOffsetRef.current; const onTipContainerLayout = React.useCallback( (event: LayoutEvent) => { - const { x, width } = initialCoordinates; + if (!coordinates) { + return; + } + const { x, width } = coordinates.initialCoordinates; const extraLeftSpace = x; const extraRightSpace = dimensions.width - width - x; @@ -194,13 +215,17 @@ function createNUXTipsOverlay( tipHorizontalOffset.setValue((actualWidth - minWidth) / 2); } }, - [dimensions.width, initialCoordinates, tipHorizontalOffset], + [coordinates, dimensions.width, tipHorizontalOffset], ); const tipParams = getNUXTipParams(route.params.tipKey); const { tooltipLocation } = tipParams; const baseTipContainerStyle = React.useMemo(() => { + if (!coordinates) { + return {}; + } + const { initialCoordinates, verticalBounds } = coordinates; const { y, x, height, width } = initialCoordinates; const style: WritableAnimatedStyleObj = { @@ -233,17 +258,13 @@ function createNUXTipsOverlay( } return style; - }, [ - dimensions.height, - dimensions.width, - initialCoordinates, - tooltipLocation, - verticalBounds.height, - verticalBounds.y, - ]); + }, [coordinates, dimensions.height, dimensions.width, tooltipLocation]); const triangleStyle = React.useMemo(() => { - const { x, width } = initialCoordinates; + if (!coordinates) { + return {}; + } + const { x, width } = coordinates.initialCoordinates; const extraLeftSpace = x; const extraRightSpace = dimensions.width - width - x; if (extraLeftSpace < extraRightSpace) { @@ -257,13 +278,20 @@ function createNUXTipsOverlay( right: extraRightSpace + (4 / 10) * width - marginHorizontal, }; } - }, [dimensions.width, initialCoordinates]); + }, [coordinates, dimensions.width]); // prettier-ignore const tipContainerEnteringAnimation = React.useCallback( (values/*: EntryAnimationsValues*/) => { 'worklet'; + if (!coordinates) { + return { + animations: {}, + initialValues:{}, + }; + } + if(tooltipLocation === 'absolute'){ return { animations: { @@ -283,8 +311,8 @@ function createNUXTipsOverlay( const initialX = (-values.targetWidth + - initialCoordinates.width + - initialCoordinates.x) / + coordinates.initialCoordinates.width + + coordinates.initialCoordinates.x) / 2; const initialY = tooltipLocation === 'below' @@ -310,7 +338,7 @@ function createNUXTipsOverlay( }, }; }, - [initialCoordinates.width, initialCoordinates.x, tooltipLocation], + [coordinates, tooltipLocation], ); // prettier-ignore @@ -318,6 +346,13 @@ function createNUXTipsOverlay( (values/*: ExitAnimationsValues*/) => { 'worklet'; + if (!coordinates) { + return { + animations: {}, + initialValues:{}, + }; + } + if (tooltipLocation === 'absolute') { return { animations: { @@ -336,8 +371,8 @@ function createNUXTipsOverlay( const toValueX = (-values.currentWidth + - initialCoordinates.width + - initialCoordinates.x) / + coordinates.initialCoordinates.width + + coordinates.initialCoordinates.x) / 2; const toValueY = tooltipLocation === 'below' @@ -368,12 +403,7 @@ function createNUXTipsOverlay( callback: onExitFinish, }; }, - [ - initialCoordinates.width, - initialCoordinates.x, - onExitFinish, - tooltipLocation, - ], + [coordinates, onExitFinish, tooltipLocation], ); let triangleDown = null; @@ -420,6 +450,10 @@ function createNUXTipsOverlay( [buttonStyle, contentContainerStyle, props.navigation, route], ); + if (!coordinates) { + return null; + } + return (