Skip to content

Commit

Permalink
Merge pull request #47036 from rayane-djouah/Fix--The-pressed-state-o…
Browse files Browse the repository at this point in the history
…f-menu-items-uses-a-very-dark-BG-color

Fix: The pressed state of menu items uses a very dark BG color
  • Loading branch information
arosiclair committed Sep 16, 2024
2 parents 568467c + 13d82c3 commit 0cd67be
Show file tree
Hide file tree
Showing 20 changed files with 43 additions and 44 deletions.
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
6 changes: 6 additions & 0 deletions src/components/PressableWithSecondaryInteraction/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,12 @@ type PressableWithSecondaryInteractionProps = PressableWithFeedbackProps & {
/** Opacity to reduce to when active */
activeOpacity?: number;

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

/** 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 @@ -243,7 +243,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

0 comments on commit 0cd67be

Please sign in to comment.