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: Submit expense - Highlight jumps to manual when clicking back from start/stop #51020

Merged
merged 2 commits into from
Oct 21, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
41 changes: 6 additions & 35 deletions src/components/TabSelector/TabSelector.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import type {MaterialTopTabBarProps} from '@react-navigation/material-top-tabs/lib/typescript/src/types';
import React, {useCallback, useEffect, useMemo, useState} from 'react';
import type {Animated} from 'react-native';
import React, {useEffect, useMemo, useState} from 'react';
import {View} from 'react-native';
import FocusTrapContainerElement from '@components/FocusTrap/FocusTrapContainerElement';
import * as Expensicons from '@components/Icon/Expensicons';
Expand All @@ -10,6 +9,8 @@ import useTheme from '@hooks/useTheme';
import useThemeStyles from '@hooks/useThemeStyles';
import CONST from '@src/CONST';
import type IconAsset from '@src/types/utils/IconAsset';
import getBackgroundColor from './getBackground';
import getOpacity from './getOpacity';
import TabSelectorItem from './TabSelectorItem';

type TabSelectorProps = MaterialTopTabBarProps & {
Expand Down Expand Up @@ -50,43 +51,13 @@ function getIconAndTitle(route: string, translate: LocaleContextProps['translate
}
}

function getOpacity(position: Animated.AnimatedInterpolation<number>, routesLength: number, tabIndex: number, active: boolean, affectedTabs: number[]) {
const activeValue = active ? 1 : 0;
const inactiveValue = active ? 0 : 1;

if (routesLength > 1) {
const inputRange = Array.from({length: routesLength}, (v, i) => i);

return position.interpolate({
inputRange,
outputRange: inputRange.map((i) => (affectedTabs.includes(tabIndex) && i === tabIndex ? activeValue : inactiveValue)),
});
}
return activeValue;
}

function TabSelector({state, navigation, onTabPress = () => {}, position, onFocusTrapContainerElementChanged}: TabSelectorProps) {
const {translate} = useLocalize();
const theme = useTheme();
const styles = useThemeStyles();
const defaultAffectedAnimatedTabs = useMemo(() => Array.from({length: state.routes.length}, (v, i) => i), [state.routes.length]);
const [affectedAnimatedTabs, setAffectedAnimatedTabs] = useState(defaultAffectedAnimatedTabs);

const getBackgroundColor = useCallback(
(routesLength: number, tabIndex: number, affectedTabs: number[]) => {
if (routesLength > 1) {
const inputRange = Array.from({length: routesLength}, (v, i) => i);

return position.interpolate({
inputRange,
outputRange: inputRange.map((i) => (affectedTabs.includes(tabIndex) && i === tabIndex ? theme.border : theme.appBG)),
}) as unknown as Animated.AnimatedInterpolation<string>;
}
return theme.border;
},
[theme, position],
);

useEffect(() => {
// It is required to wait transition end to reset affectedAnimatedTabs because tabs style is still animating during transition.
setTimeout(() => {
Expand All @@ -98,10 +69,10 @@ function TabSelector({state, navigation, onTabPress = () => {}, position, onFocu
<FocusTrapContainerElement onContainerElementChanged={onFocusTrapContainerElementChanged}>
<View style={styles.tabSelector}>
{state.routes.map((route, index) => {
const activeOpacity = getOpacity(position, state.routes.length, index, true, affectedAnimatedTabs);
const inactiveOpacity = getOpacity(position, state.routes.length, index, false, affectedAnimatedTabs);
const backgroundColor = getBackgroundColor(state.routes.length, index, affectedAnimatedTabs);
Copy link
Contributor

Choose a reason for hiding this comment

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

Coming from #51821.
For the mWeb platform, removing these interpolations also stops the listener in useTabNavigatorFocus from being triggered. As a result, isTabActive doesn't update as expected, causing infinite loading in the scan option page.

const isActive = index === state.index;
const activeOpacity = getOpacity({routesLength: state.routes.length, tabIndex: index, active: true, affectedTabs: affectedAnimatedTabs, position, isActive});
const inactiveOpacity = getOpacity({routesLength: state.routes.length, tabIndex: index, active: false, affectedTabs: affectedAnimatedTabs, position, isActive});
const backgroundColor = getBackgroundColor({routesLength: state.routes.length, tabIndex: index, affectedTabs: affectedAnimatedTabs, theme, position, isActive});
const {icon, title} = getIconAndTitle(route.name, translate);

const onPress = () => {
Expand Down
17 changes: 17 additions & 0 deletions src/components/TabSelector/getBackground/index.native.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import type {Animated} from 'react-native';
import type GetBackgroudColor from './types';

const getBackgroundColor: GetBackgroudColor = ({routesLength, tabIndex, affectedTabs, theme, position}) => {
if (routesLength > 1) {
const inputRange = Array.from({length: routesLength}, (v, i) => i);
return position?.interpolate({
inputRange,
outputRange: inputRange.map((i) => {
return affectedTabs.includes(tabIndex) && i === tabIndex ? theme.border : theme.appBG;
}),
}) as unknown as Animated.AnimatedInterpolation<string>;
}
return theme.border;
};

export default getBackgroundColor;
9 changes: 9 additions & 0 deletions src/components/TabSelector/getBackground/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import type GetBackgroudColor from './types';

const getBackgroundColor: GetBackgroudColor = ({routesLength, tabIndex, affectedTabs, theme, isActive}) => {
if (routesLength > 1) {
return affectedTabs.includes(tabIndex) && isActive ? theme.border : theme.appBG;
}
return theme.border;
};
export default getBackgroundColor;
46 changes: 46 additions & 0 deletions src/components/TabSelector/getBackground/types.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
import type {Animated} from 'react-native';
import type {ThemeColors} from '@styles/theme/types';

/**
* Configuration for the getBackgroundColor function.
*/
type GetBackgroudColorConfig = {
/**
* The number of routes.
*/
routesLength: number;

/**
* The index of the current tab.
*/
tabIndex: number;

/**
* The indices of the affected tabs.
*/
affectedTabs: number[];

/**
* The theme colors.
*/
theme: ThemeColors;

/**
* The animated position interpolation.
*/
position: Animated.AnimatedInterpolation<number>;

/**
* Whether the tab is active.
*/
isActive: boolean;
};

/**
* Function to get the background color.
* @param args - The configuration for the background color.
* @returns The interpolated background color or a string.
*/
type GetBackgroudColor = (args: GetBackgroudColorConfig) => Animated.AnimatedInterpolation<string> | string;

export default GetBackgroudColor;
18 changes: 18 additions & 0 deletions src/components/TabSelector/getOpacity/index.native.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import type GetOpacity from './types';

const getOpacity: GetOpacity = ({routesLength, tabIndex, active, affectedTabs, position, isActive}) => {
const activeValue = active ? 1 : 0;
const inactiveValue = active ? 0 : 1;

if (routesLength > 1) {
const inputRange = Array.from({length: routesLength}, (v, i) => i);

return position?.interpolate({
inputRange,
outputRange: inputRange.map((i) => (affectedTabs.includes(tabIndex) && i === tabIndex && isActive ? activeValue : inactiveValue)),
Copy link
Contributor

Choose a reason for hiding this comment

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

FYI, this caused the following issue: #51297
In this PR, we appropriately modified the getBackgroundColor logic to differentiate between web and native platforms, using interpolation based on position variable for native and using isActive variable check for web. However, for getOpacity, we mistakenly combined both approaches (interpolation based on position variable and isActive variable check), which led to inconsistencies.
We need to remove isActive variable check from here.
More details can be found in this proposal: #51297 (comment)

});
}
return activeValue;
};

export default getOpacity;
13 changes: 13 additions & 0 deletions src/components/TabSelector/getOpacity/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import type GetOpacity from './types';

const getOpacity: GetOpacity = ({routesLength, tabIndex, active, affectedTabs, isActive}) => {
const activeValue = active ? 1 : 0;
const inactiveValue = active ? 0 : 1;

if (routesLength > 1) {
return affectedTabs.includes(tabIndex) && isActive ? activeValue : inactiveValue;
}
return activeValue;
};

export default getOpacity;
45 changes: 45 additions & 0 deletions src/components/TabSelector/getOpacity/types.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
import type {Animated} from 'react-native';

/**
* Configuration for the getOpacity function.
*/
type GetOpacityConfig = {
/**
* The number of routes in the tab bar.
*/
routesLength: number;

/**
* The index of the tab.
*/
tabIndex: number;

/**
* Whether we are calculating the opacity for the active tab.
*/
active: boolean;

/**
* The indexes of the tabs that are affected by the animation.
*/
affectedTabs: number[];

/**
* Scene's position, value which we would like to interpolate.
*/
position: Animated.AnimatedInterpolation<number>;

/**
* Whether the tab is active.
*/
isActive: boolean;
};

/**
* Function to get the opacity.
* @param args - The configuration for the opacity.
* @returns The interpolated opacity or a fixed value (1 or 0).
*/
type GetOpacity = (args: GetOpacityConfig) => 1 | 0 | Animated.AnimatedInterpolation<number>;

export default GetOpacity;
Loading