From 41c105d09e07bfcd4dd6e5a39cd04ff69909ce2e Mon Sep 17 00:00:00 2001 From: Sibtain Ali Date: Sun, 12 May 2024 21:25:52 +0500 Subject: [PATCH 1/4] fix: re-add finetunings --- src/CONST.ts | 9 ++- .../config.native.ts | 5 -- src/hooks/useAnimatedHighlightStyle/config.ts | 8 --- src/hooks/useAnimatedHighlightStyle/index.ts | 69 ++++++++++++++----- src/libs/actions/Policy.ts | 45 ++++-------- 5 files changed, 73 insertions(+), 63 deletions(-) delete mode 100644 src/hooks/useAnimatedHighlightStyle/config.native.ts delete mode 100644 src/hooks/useAnimatedHighlightStyle/config.ts diff --git a/src/CONST.ts b/src/CONST.ts index edceb11edd85..38d8525ceaa6 100755 --- a/src/CONST.ts +++ b/src/CONST.ts @@ -79,8 +79,13 @@ const CONST = { // Note: Group and Self-DM excluded as these are not tied to a Workspace WORKSPACE_ROOM_TYPES: [chatTypes.POLICY_ADMINS, chatTypes.POLICY_ANNOUNCE, chatTypes.DOMAIN_ALL, chatTypes.POLICY_ROOM, chatTypes.POLICY_EXPENSE_CHAT], ANDROID_PACKAGE_NAME, - ANIMATED_HIGHLIGHT_DELAY: 500, - ANIMATED_HIGHLIGHT_DURATION: 500, + WORKSPACE_ENABLE_FEATURE_REDIRECT_DELAY: 100, + ANIMATED_HIGHLIGHT_ENTRY_DELAY: 50, + ANIMATED_HIGHLIGHT_ENTRY_DURATION: 300, + ANIMATED_HIGHLIGHT_START_DELAY: 10, + ANIMATED_HIGHLIGHT_START_DURATION: 300, + ANIMATED_HIGHLIGHT_END_DELAY: 800, + ANIMATED_HIGHLIGHT_END_DURATION: 2000, ANIMATED_TRANSITION: 300, ANIMATED_TRANSITION_FROM_VALUE: 100, ANIMATION_IN_TIMING: 100, diff --git a/src/hooks/useAnimatedHighlightStyle/config.native.ts b/src/hooks/useAnimatedHighlightStyle/config.native.ts deleted file mode 100644 index a62d3a33039e..000000000000 --- a/src/hooks/useAnimatedHighlightStyle/config.native.ts +++ /dev/null @@ -1,5 +0,0 @@ -const DELAY_FACTOR = 1.85; - -export default {}; - -export {DELAY_FACTOR}; diff --git a/src/hooks/useAnimatedHighlightStyle/config.ts b/src/hooks/useAnimatedHighlightStyle/config.ts deleted file mode 100644 index 6010c8c33aa7..000000000000 --- a/src/hooks/useAnimatedHighlightStyle/config.ts +++ /dev/null @@ -1,8 +0,0 @@ -import {isMobile} from '@libs/Browser'; - -// It takes varying amount of time to navigate to a new page on mobile and desktop -// This variable takes that into account -const DELAY_FACTOR = isMobile() ? 1 : 0.2; -export default {}; - -export {DELAY_FACTOR}; diff --git a/src/hooks/useAnimatedHighlightStyle/index.ts b/src/hooks/useAnimatedHighlightStyle/index.ts index e438bd2473fa..4f934fee7652 100644 --- a/src/hooks/useAnimatedHighlightStyle/index.ts +++ b/src/hooks/useAnimatedHighlightStyle/index.ts @@ -1,9 +1,9 @@ import React from 'react'; import {InteractionManager} from 'react-native'; import {Easing, interpolate, interpolateColor, runOnJS, useAnimatedStyle, useSharedValue, withDelay, withSequence, withTiming} from 'react-native-reanimated'; +import useScreenWrapperTranstionStatus from '@hooks/useScreenWrapperTransitionStatus'; import useTheme from '@hooks/useTheme'; import CONST from '@src/CONST'; -import {DELAY_FACTOR} from './config'; type Props = { /** Border radius of the wrapper */ @@ -12,14 +12,26 @@ type Props = { /** Height of the item that is to be faded */ height: number; - /** Whether the item should be highlighted */ - shouldHighlight: boolean; + /** Delay before the highlighted item enters */ + itemEnterDelay?: number; + + /** Duration in which the item enters */ + itemEnterDuration?: number; + + /** Delay before the item starts to get highlighted */ + highlightStartDelay?: number; + + /** Duration in which the item gets fully highlighted */ + highlightStartDuration?: number; - /** Duration of the highlight animation */ - highlightDuration?: number; + /** Delay before the item starts to get un-highlighted */ + highlightEndDelay?: number; - /** Delay before the highlight animation starts */ - delay?: number; + /** Duration in which the item gets fully un-highlighted */ + highlightEndDuration?: number; + + /** Whether the item should be highlighted */ + shouldHighlight: boolean; }; /** @@ -28,37 +40,60 @@ type Props = { export default function useAnimatedHighlightStyle({ borderRadius, shouldHighlight, - highlightDuration = CONST.ANIMATED_HIGHLIGHT_DURATION, - delay = CONST.ANIMATED_HIGHLIGHT_DELAY, + itemEnterDelay = CONST.ANIMATED_HIGHLIGHT_ENTRY_DELAY, + itemEnterDuration = CONST.ANIMATED_HIGHLIGHT_ENTRY_DURATION, + highlightStartDelay = CONST.ANIMATED_HIGHLIGHT_START_DELAY, + highlightStartDuration = CONST.ANIMATED_HIGHLIGHT_START_DURATION, + highlightEndDelay = CONST.ANIMATED_HIGHLIGHT_END_DELAY, + highlightEndDuration = CONST.ANIMATED_HIGHLIGHT_END_DURATION, height, }: Props) { - const actualDelay = delay * DELAY_FACTOR; const repeatableProgress = useSharedValue(0); const nonRepeatableProgress = useSharedValue(shouldHighlight ? 0 : 1); + const {didScreenTransitionEnd} = useScreenWrapperTranstionStatus(); const theme = useTheme(); const highlightBackgroundStyle = useAnimatedStyle(() => ({ - backgroundColor: interpolateColor(repeatableProgress.value, [0, 1], ['rgba(0, 0, 0, 0)', theme.border]), + backgroundColor: interpolateColor(repeatableProgress.value, [0, 1], [theme.appBG, theme.border]), height: interpolate(nonRepeatableProgress.value, [0, 1], [0, height]), opacity: interpolate(nonRepeatableProgress.value, [0, 1], [0, 1]), borderRadius, })); React.useEffect(() => { - if (!shouldHighlight) { + if (!shouldHighlight || !didScreenTransitionEnd) { return; } InteractionManager.runAfterInteractions(() => { runOnJS(() => { - nonRepeatableProgress.value = withDelay(actualDelay, withTiming(1, {duration: highlightDuration, easing: Easing.inOut(Easing.ease)})); - repeatableProgress.value = withSequence( - withDelay(actualDelay, withTiming(1, {duration: highlightDuration, easing: Easing.inOut(Easing.ease)})), - withDelay(actualDelay, withTiming(0, {duration: highlightDuration, easing: Easing.inOut(Easing.ease)})), + nonRepeatableProgress.value = withDelay( + itemEnterDelay, + withTiming(1, {duration: itemEnterDuration, easing: Easing.inOut(Easing.ease)}, (finished) => { + if (!finished) { + return; + } + + repeatableProgress.value = withSequence( + withDelay(highlightStartDelay, withTiming(1, {duration: highlightStartDuration, easing: Easing.inOut(Easing.ease)})), + withDelay(highlightEndDelay, withTiming(0, {duration: highlightEndDuration, easing: Easing.inOut(Easing.ease)})), + ); + }), ); })(); }); - }, [shouldHighlight, highlightDuration, actualDelay, repeatableProgress, nonRepeatableProgress]); + }, [ + didScreenTransitionEnd, + shouldHighlight, + itemEnterDelay, + itemEnterDuration, + highlightStartDelay, + highlightStartDuration, + highlightEndDelay, + highlightEndDuration, + repeatableProgress, + nonRepeatableProgress, + ]); return highlightBackgroundStyle; } diff --git a/src/libs/actions/Policy.ts b/src/libs/actions/Policy.ts index 3332932f97c7..3c2c0c36dd4d 100644 --- a/src/libs/actions/Policy.ts +++ b/src/libs/actions/Policy.ts @@ -72,7 +72,6 @@ import * as TransactionUtils from '@libs/TransactionUtils'; import type {PolicySelector} from '@pages/home/sidebar/SidebarScreen/FloatingActionButtonAndPopover'; import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; -import type {Route} from '@src/ROUTES'; import ROUTES from '@src/ROUTES'; import type { InvitedEmailsToAccountIDs, @@ -4003,26 +4002,10 @@ function openPolicyDistanceRatesPage(policyID?: string) { API.read(READ_COMMANDS.OPEN_POLICY_DISTANCE_RATES_PAGE, params); } -function navigateWhenEnableFeature(policyID: string, featureRoute: Route) { - const isNarrowLayout = getIsNarrowLayout(); - if (isNarrowLayout) { - setTimeout(() => { - Navigation.navigate(ROUTES.WORKSPACE_INITIAL.getRoute(policyID)); - }, 1000); - return; - } - - /** - * The app needs to set a navigation action to the microtask queue, it guarantees to execute Onyx.update first, then the navigation action. - * More details - https://github.com/Expensify/App/issues/37785#issuecomment-1989056726. - */ - new Promise((resolve) => { - resolve(); - }).then(() => { - requestAnimationFrame(() => { - Navigation.navigate(featureRoute); - }); - }); +function navigateWhenEnableFeature(policyID: string) { + setTimeout(() => { + Navigation.navigate(ROUTES.WORKSPACE_INITIAL.getRoute(policyID)); + }, CONST.WORKSPACE_ENABLE_FEATURE_REDIRECT_DELAY); } function enablePolicyCategories(policyID: string, enabled: boolean) { @@ -4068,8 +4051,8 @@ function enablePolicyCategories(policyID: string, enabled: boolean) { API.write(WRITE_COMMANDS.ENABLE_POLICY_CATEGORIES, parameters, onyxData); - if (enabled) { - navigateWhenEnableFeature(policyID, ROUTES.WORKSPACE_CATEGORIES.getRoute(policyID)); + if (enabled && getIsNarrowLayout()) { + navigateWhenEnableFeature(policyID); } } @@ -4160,8 +4143,8 @@ function enablePolicyDistanceRates(policyID: string, enabled: boolean) { API.write(WRITE_COMMANDS.ENABLE_POLICY_DISTANCE_RATES, parameters, onyxData); - if (enabled) { - navigateWhenEnableFeature(policyID, ROUTES.WORKSPACE_DISTANCE_RATES.getRoute(policyID)); + if (enabled && getIsNarrowLayout()) { + navigateWhenEnableFeature(policyID); } } @@ -4252,8 +4235,8 @@ function enablePolicyTags(policyID: string, enabled: boolean) { API.write(WRITE_COMMANDS.ENABLE_POLICY_TAGS, parameters, onyxData); - if (enabled) { - navigateWhenEnableFeature(policyID, ROUTES.WORKSPACE_TAGS.getRoute(policyID)); + if (enabled && getIsNarrowLayout()) { + navigateWhenEnableFeature(policyID); } } @@ -4365,8 +4348,8 @@ function enablePolicyTaxes(policyID: string, enabled: boolean) { } API.write(WRITE_COMMANDS.ENABLE_POLICY_TAXES, parameters, onyxData); - if (enabled) { - navigateWhenEnableFeature(policyID, ROUTES.WORKSPACE_TAXES.getRoute(policyID)); + if (enabled && getIsNarrowLayout()) { + navigateWhenEnableFeature(policyID); } } @@ -4456,8 +4439,8 @@ function enablePolicyWorkflows(policyID: string, enabled: boolean) { API.write(WRITE_COMMANDS.ENABLE_POLICY_WORKFLOWS, parameters, onyxData); - if (enabled) { - navigateWhenEnableFeature(policyID, ROUTES.WORKSPACE_WORKFLOWS.getRoute(policyID)); + if (enabled && getIsNarrowLayout()) { + navigateWhenEnableFeature(policyID); } } From b72a7ac30371edaf200b5597022534d75acc1c59 Mon Sep 17 00:00:00 2001 From: Sibtain Ali Date: Mon, 13 May 2024 01:14:05 +0500 Subject: [PATCH 2/4] fix: add timeout --- ios/Podfile.lock | 2 +- src/CONST.ts | 1 + src/components/ScreenWrapper.tsx | 8 +++++++- src/hooks/useAnimatedHighlightStyle/index.ts | 14 +++++++++++--- 4 files changed, 20 insertions(+), 5 deletions(-) diff --git a/ios/Podfile.lock b/ios/Podfile.lock index d0155051fc3b..90d69e768d52 100644 --- a/ios/Podfile.lock +++ b/ios/Podfile.lock @@ -2577,4 +2577,4 @@ SPEC CHECKSUMS: PODFILE CHECKSUM: a25a81f2b50270f0c0bd0aff2e2ebe4d0b4ec06d -COCOAPODS: 1.13.0 +COCOAPODS: 1.14.2 diff --git a/src/CONST.ts b/src/CONST.ts index 38d8525ceaa6..3da1c015f7df 100755 --- a/src/CONST.ts +++ b/src/CONST.ts @@ -96,6 +96,7 @@ const CONST = { // Multiplier for gyroscope animation in order to make it a bit more subtle ANIMATION_GYROSCOPE_VALUE: 0.4, BACKGROUND_IMAGE_TRANSITION_DURATION: 1000, + SCREEN_TRANSITION_END_TIMEOUT: 1000, ARROW_HIDE_DELAY: 3000, API_ATTACHMENT_VALIDATIONS: { diff --git a/src/components/ScreenWrapper.tsx b/src/components/ScreenWrapper.tsx index e53823860ce0..ec259e313507 100644 --- a/src/components/ScreenWrapper.tsx +++ b/src/components/ScreenWrapper.tsx @@ -169,12 +169,17 @@ function ScreenWrapper( ).current; useEffect(() => { + const timeout = setTimeout(() => { + setDidScreenTransitionEnd(true); + onEntryTransitionEnd?.(); + }, CONST.SCREEN_TRANSITION_END_TIMEOUT); + const unsubscribeTransitionEnd = navigation.addListener('transitionEnd', (event) => { // Prevent firing the prop callback when user is exiting the page. if (event?.data?.closing) { return; } - + clearTimeout(timeout); setDidScreenTransitionEnd(true); onEntryTransitionEnd?.(); }); @@ -192,6 +197,7 @@ function ScreenWrapper( : undefined; return () => { + clearTimeout(timeout); unsubscribeTransitionEnd(); if (beforeRemoveSubscription) { diff --git a/src/hooks/useAnimatedHighlightStyle/index.ts b/src/hooks/useAnimatedHighlightStyle/index.ts index 4f934fee7652..bbae790f23a2 100644 --- a/src/hooks/useAnimatedHighlightStyle/index.ts +++ b/src/hooks/useAnimatedHighlightStyle/index.ts @@ -1,4 +1,4 @@ -import React from 'react'; +import React, {useState} from 'react'; import {InteractionManager} from 'react-native'; import {Easing, interpolate, interpolateColor, runOnJS, useAnimatedStyle, useSharedValue, withDelay, withSequence, withTiming} from 'react-native-reanimated'; import useScreenWrapperTranstionStatus from '@hooks/useScreenWrapperTransitionStatus'; @@ -48,6 +48,7 @@ export default function useAnimatedHighlightStyle({ highlightEndDuration = CONST.ANIMATED_HIGHLIGHT_END_DURATION, height, }: Props) { + const [startHighlight, setStartHighlight] = useState(false); const repeatableProgress = useSharedValue(0); const nonRepeatableProgress = useSharedValue(shouldHighlight ? 0 : 1); const {didScreenTransitionEnd} = useScreenWrapperTranstionStatus(); @@ -61,10 +62,17 @@ export default function useAnimatedHighlightStyle({ })); React.useEffect(() => { - if (!shouldHighlight || !didScreenTransitionEnd) { + if (!shouldHighlight || startHighlight) { return; } + setStartHighlight(true); + }, [shouldHighlight, startHighlight]); + React.useEffect(() => { + if (!startHighlight || !didScreenTransitionEnd) { + return; + } + setStartHighlight(false); InteractionManager.runAfterInteractions(() => { runOnJS(() => { nonRepeatableProgress.value = withDelay( @@ -84,7 +92,7 @@ export default function useAnimatedHighlightStyle({ }); }, [ didScreenTransitionEnd, - shouldHighlight, + startHighlight, itemEnterDelay, itemEnterDuration, highlightStartDelay, From ad79bcc704308592dbd6e2bda843b49fab0d7cd2 Mon Sep 17 00:00:00 2001 From: Sibtain Ali Date: Mon, 13 May 2024 01:15:41 +0500 Subject: [PATCH 3/4] fix: add timeout --- src/components/ScreenWrapper.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/src/components/ScreenWrapper.tsx b/src/components/ScreenWrapper.tsx index ec259e313507..fb7e6f9b882c 100644 --- a/src/components/ScreenWrapper.tsx +++ b/src/components/ScreenWrapper.tsx @@ -169,6 +169,7 @@ function ScreenWrapper( ).current; useEffect(() => { + // On iOS, the transitionEnd event doesn't trigger some times. As such, we need to set a timeout const timeout = setTimeout(() => { setDidScreenTransitionEnd(true); onEntryTransitionEnd?.(); From 74e8fc2190e31978c98e41bf2936f5ecb59e70f1 Mon Sep 17 00:00:00 2001 From: Sibtain Ali Date: Mon, 13 May 2024 01:17:43 +0500 Subject: [PATCH 4/4] revert podfile changes --- ios/Podfile.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ios/Podfile.lock b/ios/Podfile.lock index 90d69e768d52..d0155051fc3b 100644 --- a/ios/Podfile.lock +++ b/ios/Podfile.lock @@ -2577,4 +2577,4 @@ SPEC CHECKSUMS: PODFILE CHECKSUM: a25a81f2b50270f0c0bd0aff2e2ebe4d0b4ec06d -COCOAPODS: 1.14.2 +COCOAPODS: 1.13.0