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: The pressed state of menu items uses a very dark BG color #47036

Merged
Show file tree
Hide file tree
Changes from 11 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
2 changes: 1 addition & 1 deletion src/components/BaseMiniContextMenuItem.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ function BaseMiniContextMenuItem(
role={CONST.ROLE.BUTTON}
style={({hovered, pressed}) => [
styles.reportActionContextMenuMiniButton,
StyleUtils.getButtonBackgroundColorStyle(getButtonState(hovered, pressed, isDelayButtonStateComplete)),
StyleUtils.getButtonBackgroundColorStyle(getButtonState(hovered, pressed, isDelayButtonStateComplete), true),
isDelayButtonStateComplete && styles.cursorDefault,
]}
>
Expand Down
3 changes: 2 additions & 1 deletion src/components/LHNOptionsList/OptionRowLHN.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,8 @@ function OptionRowLHN({reportID, isFocused = false, onSelectRow = () => {}, opti
}
}}
withoutFocusOnSecondaryInteraction
activeOpacity={0.8}
activeOpacity={variables.pressDimValue}
opacityAnimationDuration={0}
style={[
styles.flexRow,
styles.alignItemsCenter,
Expand Down
8 changes: 2 additions & 6 deletions src/components/MenuItem.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import type {ReactElement, ReactNode} from 'react';
import React, {forwardRef, useContext, useMemo} from 'react';
import type {GestureResponderEvent, StyleProp, TextStyle, ViewStyle} from 'react-native';
import {ActivityIndicator, View} from 'react-native';
import type {AnimatedStyle} from 'react-native-reanimated';
import type {ValueOf} from 'type-fest';
import useResponsiveLayout from '@hooks/useResponsiveLayout';
import useStyleUtils from '@hooks/useStyleUtils';
Expand Down Expand Up @@ -91,9 +90,6 @@ type MenuItemBaseProps = {
/** Any additional styles to apply to the label */
labelStyle?: StyleProp<ViewStyle>;

/** Any adjustments to style when menu item is hovered or pressed */
hoverAndPressStyle?: StyleProp<AnimatedStyle<ViewStyle>>;

/** Additional styles to style the description text below the title */
descriptionTextStyle?: StyleProp<TextStyle>;

Expand Down Expand Up @@ -348,7 +344,6 @@ function MenuItem(
containerStyle,
titleStyle,
labelStyle,
hoverAndPressStyle,
descriptionTextStyle,
badgeStyle,
viewMode = CONST.OPTION_MODE.DEFAULT,
Expand Down Expand Up @@ -574,6 +569,8 @@ function MenuItem(
onPressOut={ControlSelection.unblock}
onSecondaryInteraction={onSecondaryInteraction}
wrapperStyle={outerWrapperStyle}
activeOpacity={variables.pressDimValue}
opacityAnimationDuration={0}
style={({pressed}) =>
[
containerStyle,
Expand All @@ -582,7 +579,6 @@ function MenuItem(
!shouldRemoveBackground &&
StyleUtils.getButtonBackgroundColorStyle(getButtonState(focused || isHovered, pressed, success, disabled, interactive), true),
...(Array.isArray(wrapperStyle) ? wrapperStyle : [wrapperStyle]),
!focused && (isHovered || pressed) && hoverAndPressStyle,
shouldGreyOutWhenDisabled && disabled && styles.buttonOpacityDisabled,
isHovered && interactive && !focused && !pressed && !shouldRemoveBackground && styles.hoveredComponentBG,
] as StyleProp<ViewStyle>
Expand Down
21 changes: 17 additions & 4 deletions src/components/OpacityView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,24 @@ type OpacityViewProps = {
*/
dimmingValue?: number;

/**
* The duration of the dimming animation
* @default variables.dimAnimationDuration
*/
dimAnimationDuration?: number;

/** Whether the view needs to be rendered offscreen (for Android only) */
needsOffscreenAlphaCompositing?: boolean;
};

function OpacityView({shouldDim, children, style = [], dimmingValue = variables.hoverDimValue, needsOffscreenAlphaCompositing = false}: OpacityViewProps) {
function OpacityView({
shouldDim,
dimAnimationDuration = variables.dimAnimationDuration,
children,
style = [],
dimmingValue = variables.hoverDimValue,
needsOffscreenAlphaCompositing = false,
}: OpacityViewProps) {
const opacity = useSharedValue(1);
const opacityStyle = useAnimatedStyle(() => ({
opacity: opacity.value,
Expand All @@ -37,11 +50,11 @@ function OpacityView({shouldDim, children, style = [], dimmingValue = variables.
React.useEffect(() => {
if (shouldDim) {
// eslint-disable-next-line react-compiler/react-compiler
opacity.value = withTiming(dimmingValue, {duration: 50});
opacity.value = withTiming(dimmingValue, {duration: dimAnimationDuration});
} else {
opacity.value = withTiming(1, {duration: 50});
opacity.value = withTiming(1, {duration: dimAnimationDuration});
}
}, [shouldDim, dimmingValue, opacity]);
}, [shouldDim, dimmingValue, opacity, dimAnimationDuration]);

return (
<Animated.View
Expand Down
8 changes: 8 additions & 0 deletions src/components/Pressable/PressableWithFeedback.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,12 @@ type PressableWithFeedbackProps = PressableProps & {
*/
hoverDimmingValue?: number;

/**
* The duration of the dimming animation
* @default variables.dimAnimationDuration
*/
dimAnimationDuration?: number;

/** Whether the view needs to be rendered offscreen (for Android only) */
needsOffscreenAlphaCompositing?: boolean;

Expand All @@ -40,6 +46,7 @@ function PressableWithFeedback(
needsOffscreenAlphaCompositing = false,
pressDimmingValue = variables.pressDimValue,
hoverDimmingValue = variables.hoverDimValue,
dimAnimationDuration,
...rest
}: PressableWithFeedbackProps,
ref: PressableRef,
Expand All @@ -51,6 +58,7 @@ function PressableWithFeedback(
<OpacityView
shouldDim={!!(!rest.disabled && (isPressed || isHovered))}
dimmingValue={isPressed ? pressDimmingValue : hoverDimmingValue}
dimAnimationDuration={dimAnimationDuration}
style={wrapperStyle}
needsOffscreenAlphaCompositing={needsOffscreenAlphaCompositing}
>
Expand Down
2 changes: 2 additions & 0 deletions src/components/PressableWithSecondaryInteraction/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ function PressableWithSecondaryInteraction(
preventDefaultContextMenu = true,
onSecondaryInteraction,
activeOpacity = 1,
opacityAnimationDuration,
...rest
}: PressableWithSecondaryInteractionProps,
ref: PressableRef,
Expand Down Expand Up @@ -100,6 +101,7 @@ function PressableWithSecondaryInteraction(
wrapperStyle={[StyleUtils.combineStyles(DeviceCapabilities.canUseTouchScreen() ? [styles.userSelectNone, styles.noSelect] : [], inlineStyle), wrapperStyle]}
onLongPress={onSecondaryInteraction ? executeSecondaryInteraction : undefined}
pressDimmingValue={activeOpacity}
dimAnimationDuration={opacityAnimationDuration}
ref={pressableRef}
style={(state) => [StyleUtils.parseStyleFromFunction(style, state), inlineStyle]}
needsOffscreenAlphaCompositing={needsOffscreenAlphaCompositing}
Expand Down
12 changes: 12 additions & 0 deletions src/components/PressableWithSecondaryInteraction/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,18 @@ type PressableWithSecondaryInteractionProps = PressableWithFeedbackProps & {
/** Opacity to reduce to when active */
activeOpacity?: number;

/**
* The duration of the opacity animation
* @default variables.dimAnimationDuration
*/
opacityAnimationDuration?: number;

/**
* The duration of the dimming animation
* @default variables.dimAnimationDuration
*/
dimAnimationDuration?: number;
rayane-djouah marked this conversation as resolved.
Show resolved Hide resolved

/** Used to apply styles to the Pressable */
style?: ParsableStyle;

Expand Down
1 change: 0 additions & 1 deletion src/components/ReportActionItem/MoneyReportView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,6 @@ function MoneyReportView({report, policy, isCombinedReport = false, shouldShowTo
interactive
shouldStackHorizontally={false}
onSecondaryInteraction={() => {}}
hoverAndPressStyle={false}
titleWithTooltips={[]}
brickRoadIndicator={violation ? 'error' : undefined}
errorText={violationTranslation}
Expand Down
1 change: 0 additions & 1 deletion src/components/ReportActionItem/TripDetailsView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,6 @@ function ReservationView({reservation}: ReservationViewProps) {
iconWidth={20}
iconStyles={[StyleUtils.getTripReservationIconContainer(false), styles.mr3]}
secondaryIconFill={theme.icon}
hoverAndPressStyle={styles.hoveredComponentBG}
/>
);
}
Expand Down
1 change: 0 additions & 1 deletion src/pages/OnboardingPurpose/BaseOnboardingPurpose.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@ function BaseOnboardingPurpose({shouldUseNativeStyles, shouldEnableMaxHeight, ro
iconHeight: variables.menuIconSize,
iconStyles: [styles.mh3],
wrapperStyle: [styles.purposeMenuItem],
hoverAndPressStyle: [styles.purposeMenuItemSelected],
numberOfLinesTitle: 0,
onPress: () => {
Welcome.setOnboardingPurposeSelected(choice);
Expand Down
1 change: 0 additions & 1 deletion src/pages/Search/SearchTypeMenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,6 @@ function SearchTypeMenu({queryJSON}: SearchTypeMenuProps) {
iconHeight={variables.iconSizeNormal}
wrapperStyle={styles.sectionMenuItem}
focused={index === activeItemIndex}
hoverAndPressStyle={styles.hoveredComponentBG}
onPress={onPress}
isPaneMenu
/>
Expand Down
1 change: 0 additions & 1 deletion src/pages/home/sidebar/AllSettingsScreen.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,6 @@ function AllSettingsScreen({policies}: AllSettingsScreenProps) {
wrapperStyle: styles.sectionMenuItem,
isPaneMenu: true,
focused: item.focused,
hoverAndPressStyle: styles.hoveredComponentBG,
brickRoadIndicator: item.brickRoadIndicator,
}));
}, [shouldUseNarrowLayout, policies, privateSubscription, waitForNavigate, translate, styles, allConnectionSyncProgresses]);
Expand Down
15 changes: 1 addition & 14 deletions src/pages/settings/InitialSettingsPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,6 @@ function InitialSettingsPage({userWallet, bankAccountList, fundList, walletTerms
shouldStackHorizontally={item.shouldStackHorizontally}
floatRightAvatarSize={item.avatarSize}
ref={popoverAnchor}
hoverAndPressStyle={styles.hoveredComponentBG}
shouldBlockSelection={!!item.link}
onSecondaryInteraction={item.link ? (event) => openPopover(item.link, event) : undefined}
focused={
Expand All @@ -350,19 +349,7 @@ function InitialSettingsPage({userWallet, bankAccountList, fundList, walletTerms
</View>
);
},
[
styles.pb4,
styles.mh3,
styles.sectionTitle,
styles.sectionMenuItem,
styles.hoveredComponentBG,
translate,
userWallet?.currentBalance,
isExecuting,
singleExecution,
activeCentralPaneRoute,
waitForNavigate,
],
[styles.pb4, styles.mh3, styles.sectionTitle, styles.sectionMenuItem, translate, userWallet?.currentBalance, isExecuting, singleExecution, activeCentralPaneRoute, waitForNavigate],
);

const accountMenuItems = useMemo(() => getMenuItemsSection(accountMenuItemsData), [accountMenuItemsData, getMenuItemsSection]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,6 @@ function CardSection() {
title={translate('subscription.cardSection.viewPaymentHistory')}
titleStyle={styles.textStrong}
onPress={() => Navigation.navigate(ROUTES.SEARCH_CENTRAL_PANE.getRoute({query: SearchUtils.buildCannedSearchQuery()}))}
hoverAndPressStyle={styles.hoveredComponentBG}
/>
)}

Expand Down
6 changes: 2 additions & 4 deletions src/pages/settings/Wallet/PaymentMethodList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -330,13 +330,12 @@ function PaymentMethodList({
title={translate('walletPage.addBankAccount')}
icon={Expensicons.Plus}
wrapperStyle={[styles.paymentMethod, listItemStyle]}
hoverAndPressStyle={styles.hoveredComponentBG}
ref={buttonRef}
disabled={!isUserValidated}
/>
),

[onPress, translate, styles.paymentMethod, styles.hoveredComponentBG, listItemStyle, buttonRef, isUserValidated],
[onPress, translate, styles.paymentMethod, listItemStyle, buttonRef, isUserValidated],
);

/**
Expand Down Expand Up @@ -365,7 +364,6 @@ function PaymentMethodList({
wrapperStyle={[styles.paymentMethod, listItemStyle]}
iconRight={item.iconRight}
badgeStyle={styles.badgeBordered}
hoverAndPressStyle={styles.hoveredComponentBG}
shouldShowRightIcon={item.shouldShowRightIcon}
shouldShowSelectedState={shouldShowSelectedState}
isSelected={selectedMethodID.toString() === item.methodID?.toString()}
Expand All @@ -376,7 +374,7 @@ function PaymentMethodList({
</OfflineWithFeedback>
),

[styles.ph6, styles.paymentMethod, styles.badgeBordered, styles.hoveredComponentBG, filteredPaymentMethods, translate, listItemStyle, shouldShowSelectedState, selectedMethodID],
[styles.ph6, styles.paymentMethod, styles.badgeBordered, filteredPaymentMethods, translate, listItemStyle, shouldShowSelectedState, selectedMethodID],
);

return (
Expand Down
2 changes: 0 additions & 2 deletions src/pages/settings/Wallet/WalletPage/WalletPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -485,7 +485,6 @@ function WalletPage({shouldListenForResize = false}: WalletPageProps) {
title={translate('common.transferBalance')}
icon={Expensicons.Transfer}
onPress={triggerKYCFlow}
hoverAndPressStyle={styles.hoveredComponentBG}
shouldShowRightIcon
disabled={network.isOffline}
wrapperStyle={[
Expand Down Expand Up @@ -530,7 +529,6 @@ function WalletPage({shouldListenForResize = false}: WalletPageProps) {
disabled={network.isOffline}
title={translate('walletPage.enableWallet')}
icon={Expensicons.Wallet}
hoverAndPressStyle={styles.hoveredComponentBG}
wrapperStyle={[
styles.transferBalance,
shouldUseNarrowLayout ? styles.mhn5 : styles.mhn8,
Expand Down
1 change: 0 additions & 1 deletion src/pages/workspace/WorkspaceInitialPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -402,7 +402,6 @@ function WorkspaceInitialPage({policyDraft, policy: policyProp, policyCategories
wrapperStyle={styles.sectionMenuItem}
highlighted={enabledItem?.routeName === item.routeName}
focused={!!(item.routeName && activeRoute?.startsWith(item.routeName))}
hoverAndPressStyle={styles.hoveredComponentBG}
isPaneMenu
/>
))}
Expand Down
4 changes: 0 additions & 4 deletions src/styles/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4324,10 +4324,6 @@ const styles = (theme: ThemeColors) =>
marginBottom: 8,
},

purposeMenuItemSelected: {
backgroundColor: theme.activeComponentBG,
},

willChangeTransform: {
willChange: 'transform',
},
Expand Down
2 changes: 1 addition & 1 deletion src/styles/utils/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1311,7 +1311,7 @@ const createStyleUtils = (theme: ThemeColors, styles: ThemeStyles) => ({
getButtonBackgroundColorStyle: (buttonState: ButtonStateName = CONST.BUTTON_STATES.DEFAULT, isMenuItem = false): ViewStyle => {
switch (buttonState) {
case CONST.BUTTON_STATES.PRESSED:
return {backgroundColor: theme.buttonPressedBG};
return isMenuItem ? {backgroundColor: theme.buttonHoveredBG} : {backgroundColor: theme.buttonPressedBG};
case CONST.BUTTON_STATES.ACTIVE:
return isMenuItem ? {backgroundColor: theme.border} : {backgroundColor: theme.buttonHoveredBG};
case CONST.BUTTON_STATES.DISABLED:
Expand Down
1 change: 1 addition & 0 deletions src/styles/variables.ts
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,7 @@ export default {
googleEmptyListViewHeight: 14,
hoverDimValue: 1,
pressDimValue: 0.8,
dimAnimationDuration: 50,
qrShareHorizontalPadding: 32,
menuIconSize: 48,

Expand Down
Loading