Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix bottom tab highlight delay #44931

Merged
merged 17 commits into from
Jul 11, 2024
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions src/components/Search/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import ONYXKEYS from '@src/ONYXKEYS';
import ROUTES from '@src/ROUTES';
import type SearchResults from '@src/types/onyx/SearchResults';
import type {SearchDataTypes, SearchQuery} from '@src/types/onyx/SearchResults';
import isLoadingOnyxValue from '@src/types/utils/isLoadingOnyxValue';
import SearchListWithHeader from './SearchListWithHeader';
import SearchPageHeader from './SearchPageHeader';

Expand Down Expand Up @@ -69,7 +68,7 @@ function Search({query, policyIDs, sortBy, sortOrder}: SearchProps) {
);

const hash = SearchUtils.getQueryHash(query, policyIDs, sortBy, sortOrder);
const [currentSearchResults, searchResultsMeta] = useOnyx(`${ONYXKEYS.COLLECTION.SNAPSHOT}${hash}`);
const [currentSearchResults] = useOnyx(`${ONYXKEYS.COLLECTION.SNAPSHOT}${hash}`);

// save last non-empty search results to avoid ugly flash of loading screen when hash changes and onyx returns empty data
if (currentSearchResults?.data && currentSearchResults !== lastSearchResultsRef.current) {
Expand All @@ -87,7 +86,7 @@ function Search({query, policyIDs, sortBy, sortOrder}: SearchProps) {
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [hash, isOffline]);

const isLoadingItems = (!isOffline && isLoadingOnyxValue(searchResultsMeta)) || searchResults?.data === undefined;
const isLoadingItems = !isOffline && searchResults?.data === undefined;
const isLoadingMoreItems = !isLoadingItems && searchResults?.search?.isLoading && searchResults?.search?.offset > 0;
const shouldShowEmptyState = !isLoadingItems && SearchUtils.isSearchResultsEmpty(searchResults);

Expand Down
8 changes: 8 additions & 0 deletions src/hooks/useActiveBottomTabRoute.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import {useContext} from 'react';
import ActiveBottomTabRouteContext from '@libs/Navigation/AppNavigator/Navigators/ActiveBottomTabRouteContext';

function useActiveBottomTabRoute() {
return useContext(ActiveBottomTabRouteContext);
}

export default useActiveBottomTabRoute;
9 changes: 0 additions & 9 deletions src/hooks/useActiveRoute.ts

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import React from 'react';
import type {BottomTabScreensParamList, NavigationPartialRoute} from '@libs/Navigation/types';

const ActiveBottomTabRouteContext = React.createContext<NavigationPartialRoute<keyof BottomTabScreensParamList> | undefined>(undefined);

export default ActiveBottomTabRouteContext;

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,15 @@ import {useNavigationState} from '@react-navigation/native';
import type {StackNavigationOptions} from '@react-navigation/stack';
import React from 'react';
import createCustomBottomTabNavigator from '@libs/Navigation/AppNavigator/createCustomBottomTabNavigator';
import getTopmostBottomTabRoute from '@libs/Navigation/getTopmostBottomTabRoute';
import getTopmostCentralPaneRoute from '@libs/Navigation/getTopmostCentralPaneRoute';
import type {BottomTabNavigatorParamList, CentralPaneName, NavigationPartialRoute, RootStackParamList} from '@libs/Navigation/types';
import type {BottomTabNavigatorParamList, BottomTabScreensParamList, NavigationPartialRoute, RootStackParamList} from '@libs/Navigation/types';
import {isBottomTabName} from '@libs/NavigationUtils';
import SidebarScreen from '@pages/home/sidebar/SidebarScreen';
import SearchPageBottomTab from '@pages/Search/SearchPageBottomTab';
import SCREENS from '@src/SCREENS';
import type ReactComponentModule from '@src/types/utils/ReactComponentModule';
import ActiveRouteContext from './ActiveRouteContext';
import ActiveBottomTabRouteContext from './ActiveBottomTabRouteContext';

const loadInitialSettingsPage = () => require<ReactComponentModule>('../../../../pages/settings/InitialSettingsPage').default;
const Tab = createCustomBottomTabNavigator<BottomTabNavigatorParamList>();
Expand All @@ -19,10 +21,22 @@ const screenOptions: StackNavigationOptions = {
};

function BottomTabNavigator() {
const activeRoute = useNavigationState<RootStackParamList, NavigationPartialRoute<CentralPaneName> | undefined>(getTopmostCentralPaneRoute);
const activeRoute = useNavigationState<RootStackParamList, NavigationPartialRoute<keyof BottomTabScreensParamList> | undefined>((state) => {
if (!state) {
return undefined;
}
let route: NavigationPartialRoute<keyof BottomTabScreensParamList> | undefined;
for (const selector of [getTopmostBottomTabRoute, getTopmostCentralPaneRoute]) {
const selectedRoute = selector(state);
if (isBottomTabName(selectedRoute?.name)) {
route = selectedRoute as NavigationPartialRoute<keyof BottomTabScreensParamList>;
}
}

return route;
});
return (
<ActiveRouteContext.Provider value={activeRoute}>
<ActiveBottomTabRouteContext.Provider value={activeRoute}>
<Tab.Navigator screenOptions={screenOptions}>
<Tab.Screen
name={SCREENS.HOME}
Expand All @@ -37,7 +51,7 @@ function BottomTabNavigator() {
getComponent={loadInitialSettingsPage}
/>
</Tab.Navigator>
</ActiveRouteContext.Provider>
</ActiveBottomTabRouteContext.Provider>
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,12 @@ import React, {useCallback, useEffect} from 'react';
import {View} from 'react-native';
import type {OnyxEntry} from 'react-native-onyx';
import {withOnyx} from 'react-native-onyx';
import type {TupleToUnion} from 'type-fest';
import Icon from '@components/Icon';
import * as Expensicons from '@components/Icon/Expensicons';
import {PressableWithFeedback} from '@components/Pressable';
import Tooltip from '@components/Tooltip';
import useActiveBottomTabRoute from '@hooks/useActiveBottomTabRoute';
import useActiveWorkspace from '@hooks/useActiveWorkspace';
import useLocalize from '@hooks/useLocalize';
import useTheme from '@hooks/useTheme';
Expand All @@ -17,7 +19,7 @@ import getTopmostBottomTabRoute from '@libs/Navigation/getTopmostBottomTabRoute'
import getTopmostCentralPaneRoute from '@libs/Navigation/getTopmostCentralPaneRoute';
import Navigation from '@libs/Navigation/Navigation';
import type {RootStackParamList, State} from '@libs/Navigation/types';
import {isCentralPaneName} from '@libs/NavigationUtils';
import {isCentralPaneName, isHomeTabName, isSearchTabName, isSettingTabName} from '@libs/NavigationUtils';
import {getChatTabBrickRoad} from '@libs/WorkspacesSettingsUtils';
import BottomTabAvatar from '@pages/home/sidebar/BottomTabAvatar';
import BottomTabBarFloatingActionButton from '@pages/home/sidebar/BottomTabBarFloatingActionButton';
Expand All @@ -40,6 +42,7 @@ function BottomTabBar({isLoadingApp = false}: PurposeForUsingExpensifyModalProps
const styles = useThemeStyles();
const {translate} = useLocalize();
const navigation = useNavigation();
const HOME_SCREENS = [SCREENS.HOME, SCREENS.REPORT];
const {activeWorkspaceID: contextActiveWorkspaceID} = useActiveWorkspace();
const activeWorkspaceID = sessionStorage.getItem(CONST.SESSION_STORAGE_KEYS.ACTIVE_WORKSPACE_ID) ?? contextActiveWorkspaceID;

Expand Down Expand Up @@ -69,6 +72,7 @@ function BottomTabBar({isLoadingApp = false}: PurposeForUsingExpensifyModalProps
return topmostBottomTabRoute?.name ?? SCREENS.HOME;
});

const activeBottomTabRoute = useActiveBottomTabRoute();
const chatTabBrickRoad = getChatTabBrickRoad(activeWorkspaceID);

const navigateToChats = useCallback(() => {
Expand All @@ -92,7 +96,7 @@ function BottomTabBar({isLoadingApp = false}: PurposeForUsingExpensifyModalProps
<View>
<Icon
src={Expensicons.Inbox}
fill={currentTabName === SCREENS.HOME ? theme.iconMenu : theme.icon}
fill={isHomeTabName(activeBottomTabRoute?.name as TupleToUnion<typeof HOME_SCREENS>) ? theme.iconMenu : theme.icon}
width={variables.iconBottomBar}
height={variables.iconBottomBar}
/>
Expand All @@ -105,7 +109,7 @@ function BottomTabBar({isLoadingApp = false}: PurposeForUsingExpensifyModalProps
<Tooltip text={translate('common.search')}>
<PressableWithFeedback
onPress={() => {
if (currentTabName === SCREENS.SEARCH.BOTTOM_TAB || currentTabName === SCREENS.SEARCH.CENTRAL_PANE) {
if (isSearchTabName(activeBottomTabRoute?.name)) {
return;
}
interceptAnonymousUser(() => Navigation.navigate(ROUTES.SEARCH.getRoute(CONST.SEARCH.TAB.ALL)));
Expand All @@ -118,14 +122,14 @@ function BottomTabBar({isLoadingApp = false}: PurposeForUsingExpensifyModalProps
<View>
<Icon
src={Expensicons.MoneySearch}
fill={currentTabName === SCREENS.SEARCH.BOTTOM_TAB || currentTabName === SCREENS.SEARCH.CENTRAL_PANE ? theme.iconMenu : theme.icon}
fill={isSearchTabName(activeBottomTabRoute?.name) ? theme.iconMenu : theme.icon}
width={variables.iconBottomBar}
height={variables.iconBottomBar}
/>
</View>
</PressableWithFeedback>
</Tooltip>
<BottomTabAvatar isSelected={currentTabName === SCREENS.SETTINGS.ROOT} />
<BottomTabAvatar isSelected={isSettingTabName(activeBottomTabRoute?.name)} />
<View style={[styles.flex1, styles.bottomTabBarItem]}>
<BottomTabBarFloatingActionButton />
</View>
Expand Down
13 changes: 13 additions & 0 deletions src/libs/Navigation/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,12 @@ type CentralPaneScreensParamList = {
[SCREENS.SETTINGS.SUBSCRIPTION.ROOT]: undefined;
};

type SearchNavigatorParamList = {
[SCREENS.SEARCH.BOTTOM_TAB]: undefined;
[SCREENS.SEARCH.CENTRAL_PANE]: undefined;
[SCREENS.SEARCH.REPORT_RHP]: undefined;
};

Comment on lines +82 to +87
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @Zakpak0, I am working on some navigation changes and I noticed these new types. Could you please explain to me why this type is necessary? I don't see any navigator named SearchNavigator.

Copy link
Contributor Author

@Zakpak0 Zakpak0 Jul 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Np @adamgrzybowski , happy to.
If I recall correctly, I was having a hard time finding an existing type that defined the search tab's routes in isolation.
I created that to supplement the creation of this type here.
Maybe a better name for it would be SearchTabRoutes.
This is my first time touching the code base and I was attempting to follow the patterns I saw.
Even if I was being a bit naive.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am working on some navigation changes

Please feel free to mini-refactor if you're working on this at the moment and you see fit, as long as this PRs changes won't throw type errors I think you're good to go!

type SettingsNavigatorParamList = {
[SCREENS.SETTINGS.SHARE_CODE]: undefined;
[SCREENS.SETTINGS.PROFILE.ROOT]: undefined;
Expand Down Expand Up @@ -997,6 +1003,8 @@ type ExplanationModalNavigatorParamList = {
[SCREENS.EXPLANATION_MODAL.ROOT]: undefined;
};

type BottomTabScreensParamList = {[SCREENS.HOME]: undefined; [SCREENS.REPORT]: undefined} & SearchNavigatorParamList & SettingsNavigatorParamList;

type BottomTabNavigatorParamList = {
[SCREENS.HOME]: {policyID?: string};
[SCREENS.SEARCH.BOTTOM_TAB]: {
Expand Down Expand Up @@ -1095,6 +1103,8 @@ type RootStackParamList = PublicScreensParamList & AuthScreensParamList & LeftMo

type BottomTabName = keyof BottomTabNavigatorParamList;

type BottomTabScreenName = keyof BottomTabScreensParamList;

type FullScreenName = keyof FullScreenNavigatorParamList;

type CentralPaneName = keyof CentralPaneScreensParamList;
Expand All @@ -1112,6 +1122,8 @@ export type {
CentralPaneName,
BackToParams,
BottomTabName,
BottomTabScreenName,
BottomTabScreensParamList,
BottomTabNavigatorParamList,
DetailsNavigatorParamList,
EditRequestNavigatorParamList,
Expand Down Expand Up @@ -1142,6 +1154,7 @@ export type {
RoomInviteNavigatorParamList,
RoomMembersNavigatorParamList,
RootStackParamList,
SearchNavigatorParamList,
SettingsNavigatorParamList,
SignInNavigatorParamList,
FeatureTrainingNavigatorParamList,
Expand Down
43 changes: 42 additions & 1 deletion src/libs/NavigationUtils.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import cloneDeep from 'lodash/cloneDeep';
import type {TupleToUnion} from 'type-fest';
import {flattenObject} from '@src/languages/translations';
import SCREENS from '@src/SCREENS';
import getTopmostBottomTabRoute from './Navigation/getTopmostBottomTabRoute';
import type {CentralPaneName, RootStackParamList, State} from './Navigation/types';
Expand Down Expand Up @@ -34,4 +36,43 @@ const removePolicyIDParamFromState = (state: State<RootStackParamList>) => {
return stateCopy;
};

export {isCentralPaneName, removePolicyIDParamFromState};
const SETTINGS_SCREENS = Object.values(flattenObject(SCREENS.SETTINGS));
const SEARCH_SCREENS = Object.values(flattenObject(SCREENS.SEARCH));
const HOME_SCREENS = [SCREENS.HOME, SCREENS.REPORT];
const BOTTOM_TAB_SCREEN_NAMES = new Set([...SETTINGS_SCREENS, ...SEARCH_SCREENS, ...HOME_SCREENS]);

const SETTINGS_TAB_SCREEN_NAMES = new Set(SETTINGS_SCREENS);

const SEARCH_TAB_SCREEN_NAMES = new Set(SEARCH_SCREENS);

const HOME_SCREEN_NAMES = new Set(HOME_SCREENS);

function isBottomTabName(screen: TupleToUnion<typeof SETTINGS_SCREENS> | undefined) {
if (!screen) {
return false;
}
return BOTTOM_TAB_SCREEN_NAMES.has(screen);
}

function isSettingTabName(screen: TupleToUnion<typeof SETTINGS_SCREENS> | undefined) {
if (!screen) {
return false;
}
return SETTINGS_TAB_SCREEN_NAMES.has(screen);
}

function isSearchTabName(screen: TupleToUnion<typeof SEARCH_SCREENS> | undefined) {
if (!screen) {
return false;
}
return SEARCH_TAB_SCREEN_NAMES.has(screen);
}

function isHomeTabName(screen: TupleToUnion<typeof HOME_SCREENS> | undefined) {
if (!screen) {
return false;
}
return HOME_SCREEN_NAMES.has(screen);
}

export {isCentralPaneName, isBottomTabName, isSearchTabName, isSettingTabName, isHomeTabName, removePolicyIDParamFromState};
10 changes: 5 additions & 5 deletions src/pages/Search/SearchPageBottomTab.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import React, {useMemo} from 'react';
import FullPageNotFoundView from '@components/BlockingViews/FullPageNotFoundView';
import ScreenWrapper from '@components/ScreenWrapper';
import Search from '@components/Search';
import useActiveRoute from '@hooks/useActiveRoute';
import useActiveBottomTabRoute from '@hooks/useActiveBottomTabRoute';
import useLocalize from '@hooks/useLocalize';
import useThemeStyles from '@hooks/useThemeStyles';
import useWindowDimensions from '@hooks/useWindowDimensions';
Expand All @@ -27,7 +27,7 @@ const defaultSearchProps = {
function SearchPageBottomTab() {
const {translate} = useLocalize();
const {isSmallScreenWidth} = useWindowDimensions();
const activeRoute = useActiveRoute();
const activeBottomTabRoute = useActiveBottomTabRoute();
const styles = useThemeStyles();

const {
Expand All @@ -36,11 +36,11 @@ function SearchPageBottomTab() {
sortBy,
sortOrder,
} = useMemo(() => {
if (activeRoute?.name !== SCREENS.SEARCH.CENTRAL_PANE || !activeRoute.params) {
if (activeBottomTabRoute?.name !== SCREENS.SEARCH.CENTRAL_PANE || !activeBottomTabRoute.params) {
return defaultSearchProps;
}
return {...defaultSearchProps, ...activeRoute.params} as SearchPageProps['route']['params'];
}, [activeRoute]);
return {...defaultSearchProps, ...activeBottomTabRoute.params} as SearchPageProps['route']['params'];
}, [activeBottomTabRoute]);

const query = rawQuery as SearchQuery;

Expand Down
12 changes: 8 additions & 4 deletions src/pages/settings/InitialSettingsPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import Text from '@components/Text';
import Tooltip from '@components/Tooltip';
import type {WithCurrentUserPersonalDetailsProps} from '@components/withCurrentUserPersonalDetails';
import withCurrentUserPersonalDetails from '@components/withCurrentUserPersonalDetails';
import useActiveRoute from '@hooks/useActiveRoute';
import useActiveBottomTabRoute from '@hooks/useActiveBottomTabRoute';
import useLocalize from '@hooks/useLocalize';
import useNetwork from '@hooks/useNetwork';
import useSingleExecution from '@hooks/useSingleExecution';
Expand Down Expand Up @@ -105,7 +105,7 @@ function InitialSettingsPage({session, userWallet, bankAccountList, fundList, wa
const waitForNavigate = useWaitForNavigation();
const popoverAnchor = useRef(null);
const {translate, formatPhoneNumber} = useLocalize();
const activeRoute = useActiveRoute();
const activeBottomTabRoute = useActiveBottomTabRoute();
const emojiCode = currentUserPersonalDetails?.status?.emojiCode ?? '';

const [shouldShowSignoutConfirmModal, setShouldShowSignoutConfirmModal] = useState(false);
Expand Down Expand Up @@ -332,7 +332,11 @@ function InitialSettingsPage({session, userWallet, bankAccountList, fundList, wa
hoverAndPressStyle={styles.hoveredComponentBG}
shouldBlockSelection={!!item.link}
onSecondaryInteraction={item.link ? (event) => openPopover(item.link, event) : undefined}
focused={!!activeRoute && !!item.routeName && !!(activeRoute.name.toLowerCase().replaceAll('_', '') === item.routeName.toLowerCase().replaceAll('/', ''))}
focused={
!!activeBottomTabRoute &&
!!item.routeName &&
!!(activeBottomTabRoute.name.toLowerCase().replaceAll('_', '') === item.routeName.toLowerCase().replaceAll('/', ''))
}
isPaneMenu
iconRight={item.iconRight}
shouldShowRightIcon={item.shouldShowRightIcon}
Expand All @@ -352,7 +356,7 @@ function InitialSettingsPage({session, userWallet, bankAccountList, fundList, wa
userWallet?.currentBalance,
isExecuting,
singleExecution,
activeRoute,
activeBottomTabRoute,
waitForNavigate,
],
);
Expand Down
Loading