Skip to content

Commit

Permalink
Merge pull request #38553 from dukenv0307/fix/37266
Browse files Browse the repository at this point in the history
Fix three-dot menu overlaps with emoji menu
  • Loading branch information
cristipaval authored Mar 26, 2024
2 parents a51e60e + 640b2af commit 9dd66cf
Show file tree
Hide file tree
Showing 10 changed files with 95 additions and 22 deletions.
7 changes: 6 additions & 1 deletion src/components/ContextMenuItem.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import type {ForwardedRef} from 'react';
import React, {forwardRef, useImperativeHandle} from 'react';
import type {GestureResponderEvent, StyleProp, ViewStyle} from 'react-native';
import type {GestureResponderEvent, StyleProp, View, ViewStyle} from 'react-native';
import useStyleUtils from '@hooks/useStyleUtils';
import useThemeStyles from '@hooks/useThemeStyles';
import useThrottledButtonState from '@hooks/useThrottledButtonState';
Expand Down Expand Up @@ -46,6 +46,9 @@ type ContextMenuItemProps = {
wrapperStyle?: StyleProp<ViewStyle>;

shouldPreventDefaultFocusOnPress?: boolean;

/** The ref of mini context menu item */
buttonRef?: React.RefObject<View>;
};

type ContextMenuItemHandle = {
Expand All @@ -66,6 +69,7 @@ function ContextMenuItem(
shouldLimitWidth = true,
wrapperStyle,
shouldPreventDefaultFocusOnPress = true,
buttonRef = {current: null},
}: ContextMenuItemProps,
ref: ForwardedRef<ContextMenuItemHandle>,
) {
Expand Down Expand Up @@ -94,6 +98,7 @@ function ContextMenuItem(

return isMini ? (
<BaseMiniContextMenuItem
ref={buttonRef}
tooltipText={itemText}
onPress={triggerPressAndUpdateSuccess}
isDelayButtonStateComplete={!isThrottledButtonActive}
Expand Down
24 changes: 22 additions & 2 deletions src/components/EmojiPicker/EmojiPicker.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ const EmojiPicker = forwardRef((props, ref) => {
const [emojiPopoverAnchorOrigin, setEmojiPopoverAnchorOrigin] = useState(DEFAULT_ANCHOR_ORIGIN);
const [activeID, setActiveID] = useState();
const emojiPopoverAnchorRef = useRef(null);
const emojiAnchorDimension = useRef({
width: 0,
height: 0,
});
const onModalHide = useRef(() => {});
const onEmojiSelected = useRef(() => {});
const activeEmoji = useRef();
Expand Down Expand Up @@ -76,7 +80,14 @@ const EmojiPicker = forwardRef((props, ref) => {
// eslint-disable-next-line es/no-optional-chaining
onWillShow?.();
setIsEmojiPickerVisible(true);
setEmojiPopoverAnchorPosition(value);
setEmojiPopoverAnchorPosition({
horizontal: value.horizontal,
vertical: value.vertical,
});
emojiAnchorDimension.current = {
width: value.width,
height: value.height,
};
setEmojiPopoverAnchorOrigin(anchorOriginValue);
setActiveID(id);
});
Expand Down Expand Up @@ -155,7 +166,14 @@ const EmojiPicker = forwardRef((props, ref) => {
return;
}
calculateAnchorPosition(emojiPopoverAnchor.current, emojiPopoverAnchorOrigin).then((value) => {
setEmojiPopoverAnchorPosition(value);
setEmojiPopoverAnchorPosition({
horizontal: value.horizontal,
vertical: value.vertical,
});
emojiAnchorDimension.current = {
width: value.width,
height: value.height,
};
});
});
return () => {
Expand Down Expand Up @@ -192,7 +210,9 @@ const EmojiPicker = forwardRef((props, ref) => {
anchorAlignment={emojiPopoverAnchorOrigin}
outerStyle={StyleUtils.getOuterModalStyle(windowHeight, props.viewportOffsetTop)}
innerContainerStyle={styles.popoverInnerContainer}
anchorDimensions={emojiAnchorDimension.current}
avoidKeyboard
shoudSwitchPositionIfOverflow
>
<EmojiPickerMenu
onEmojiSelected={selectEmoji}
Expand Down
22 changes: 20 additions & 2 deletions src/components/PopoverWithMeasuredContent.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import useThemeStyles from '@hooks/useThemeStyles';
import useWindowDimensions from '@hooks/useWindowDimensions';
import PopoverWithMeasuredContentUtils from '@libs/PopoverWithMeasuredContentUtils';
import CONST from '@src/CONST';
import type {AnchorPosition} from '@src/styles';
import type {AnchorDimensions, AnchorPosition} from '@src/styles';
import Popover from './Popover';
import type {PopoverProps} from './Popover/types';
import type {WindowDimensionsProps} from './withWindowDimensions/types';
Expand All @@ -15,6 +15,12 @@ type PopoverWithMeasuredContentProps = Omit<PopoverProps, 'anchorPosition' | key
/** The horizontal and vertical anchors points for the popover */
anchorPosition: AnchorPosition;

/** The dimension of anchor component */
anchorDimensions?: AnchorDimensions;

/** Whether we should change the vertical position if the popover's position is overflow */
shoudSwitchPositionIfOverflow?: boolean;

/** Whether handle navigation back when modal show. */
shouldHandleNavigationBack?: boolean;
};
Expand Down Expand Up @@ -45,6 +51,11 @@ function PopoverWithMeasuredContent({
statusBarTranslucent = true,
avoidKeyboard = false,
hideModalContentWhileAnimating = false,
anchorDimensions = {
height: 0,
width: 0,
},
shoudSwitchPositionIfOverflow = false,
shouldHandleNavigationBack = false,
...props
}: PopoverWithMeasuredContentProps) {
Expand Down Expand Up @@ -114,11 +125,18 @@ function PopoverWithMeasuredContent({
}, [anchorPosition, anchorAlignment, popoverWidth, popoverHeight]);

const horizontalShift = PopoverWithMeasuredContentUtils.computeHorizontalShift(adjustedAnchorPosition.left, popoverWidth, windowWidth);
const verticalShift = PopoverWithMeasuredContentUtils.computeVerticalShift(adjustedAnchorPosition.top, popoverHeight, windowHeight);
const verticalShift = PopoverWithMeasuredContentUtils.computeVerticalShift(
adjustedAnchorPosition.top,
popoverHeight,
windowHeight,
anchorDimensions.height,
shoudSwitchPositionIfOverflow,
);
const shiftedAnchorPosition = {
left: adjustedAnchorPosition.left + horizontalShift,
bottom: windowHeight - (adjustedAnchorPosition.top + popoverHeight) - verticalShift,
};

return isContentMeasured ? (
<Popover
shouldHandleNavigationBack={shouldHandleNavigationBack}
Expand Down
8 changes: 5 additions & 3 deletions src/libs/PopoverWithMeasuredContentUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,18 +32,20 @@ function computeHorizontalShift(anchorLeftEdge: number, menuWidth: number, windo
* @param anchorTopEdge - Menu's anchor Top edge.
* @param menuHeight - The height of the menu itself.
* @param windowHeight - The height of the Window.
* @param anchorHeight - The height of anchor component
* @param shoudSwitchPositionIfOverflow -
*/
function computeVerticalShift(anchorTopEdge: number, menuHeight: number, windowHeight: number): number {
function computeVerticalShift(anchorTopEdge: number, menuHeight: number, windowHeight: number, anchorHeight: number, shoudSwitchPositionIfOverflow = false): number {
const popoverBottomEdge = anchorTopEdge + menuHeight;

if (anchorTopEdge < 0) {
// Anchor is in top window Edge, shift bottom by a multiple of four.
return roundToNearestMultipleOfFour(0 - anchorTopEdge);
return roundToNearestMultipleOfFour(shoudSwitchPositionIfOverflow ? menuHeight + anchorHeight : 0 - anchorTopEdge);
}

if (popoverBottomEdge > windowHeight) {
// Anchor is in Bottom window Edge, shift top by a multiple of four.
return roundToNearestMultipleOfFour(windowHeight - popoverBottomEdge);
return roundToNearestMultipleOfFour(shoudSwitchPositionIfOverflow ? -(menuHeight + anchorHeight) : windowHeight - popoverBottomEdge);
}

// Anchor is not in the gutter, so no need to shift it vertically
Expand Down
10 changes: 5 additions & 5 deletions src/libs/calculateAnchorPosition.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
import type {ValueOf} from 'type-fest';
import type {ContextMenuAnchor} from '@pages/home/report/ContextMenu/ReportActionContextMenu';
import CONST from '@src/CONST';
import type {AnchorPosition} from '@src/styles';
import type {AnchorDimensions, AnchorPosition} from '@src/styles';

type AnchorOrigin = {
horizontal: ValueOf<typeof CONST.MODAL.ANCHOR_ORIGIN_HORIZONTAL>;
Expand All @@ -13,18 +13,18 @@ type AnchorOrigin = {
/**
* Gets the x,y position of the passed in component for the purpose of anchoring another component to it.
*/
export default function calculateAnchorPosition(anchorComponent: ContextMenuAnchor, anchorOrigin?: AnchorOrigin): Promise<AnchorPosition> {
export default function calculateAnchorPosition(anchorComponent: ContextMenuAnchor, anchorOrigin?: AnchorOrigin): Promise<AnchorPosition & AnchorDimensions> {
return new Promise((resolve) => {
if (!anchorComponent || !('measureInWindow' in anchorComponent)) {
resolve({horizontal: 0, vertical: 0});
resolve({horizontal: 0, vertical: 0, width: 0, height: 0});
return;
}
anchorComponent.measureInWindow((x, y, width, height) => {
if (anchorOrigin?.vertical === CONST.MODAL.ANCHOR_ORIGIN_VERTICAL.TOP && anchorOrigin?.horizontal === CONST.MODAL.ANCHOR_ORIGIN_HORIZONTAL.LEFT) {
resolve({horizontal: x, vertical: y + height + (anchorOrigin?.shiftVertical ?? 0)});
resolve({horizontal: x, vertical: y + height + (anchorOrigin?.shiftVertical ?? 0), width, height});
return;
}
resolve({horizontal: x + width, vertical: y + (anchorOrigin?.shiftVertical ?? 0)});
resolve({horizontal: x + width, vertical: y + (anchorOrigin?.shiftVertical ?? 0), width, height});
});
});
}
16 changes: 13 additions & 3 deletions src/pages/home/report/ContextMenu/BaseReportActionContextMenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@ function BaseReportActionContextMenu({
const [shouldKeepOpen, setShouldKeepOpen] = useState(false);
const wrapperStyle = StyleUtils.getReportActionContextMenuStyles(isMini, isSmallScreenWidth);
const {isOffline} = useNetwork();
const threedotRef = useRef<View>(null);

const reportAction: OnyxEntry<ReportAction> = useMemo(() => {
if (isEmptyObject(reportActions) || reportActionID === '0') {
Expand Down Expand Up @@ -193,14 +194,14 @@ function BaseReportActionContextMenu({
{isActive: shouldEnableArrowNavigation},
);

const openOverflowMenu = (event: GestureResponderEvent | MouseEvent) => {
const openOverflowMenu = (event: GestureResponderEvent | MouseEvent, anchorRef: MutableRefObject<View | null>) => {
const originalReportID = ReportUtils.getOriginalReportID(reportID, reportAction);
const originalReport = ReportUtils.getReport(originalReportID);
showContextMenu(
CONST.CONTEXT_MENU_TYPES.REPORT_ACTION,
event,
selection,
anchor?.current as ViewType | RNText | null,
anchorRef?.current as ViewType | RNText | null,
reportID,
reportAction?.reportActionID,
originalReportID,
Expand All @@ -216,6 +217,8 @@ function BaseReportActionContextMenu({
undefined,
filteredContextMenuActions,
true,
() => {},
true,
);
};

Expand Down Expand Up @@ -249,19 +252,26 @@ function BaseReportActionContextMenu({
textTranslateKey === 'reportActionContextMenu.deleteAction' ||
textTranslateKey === 'reportActionContextMenu.deleteConfirmation';
const text = textTranslateKey && (isKeyInActionUpdateKeys ? translate(textTranslateKey, {action: reportAction}) : translate(textTranslateKey));
const isMenuAction = textTranslateKey === 'reportActionContextMenu.menu';

return (
<ContextMenuItem
ref={(ref) => {
menuItemRefs.current[index] = ref;
}}
buttonRef={isMenuAction ? threedotRef : {current: null}}
icon={contextAction.icon}
text={text ?? ''}
successIcon={contextAction.successIcon}
successText={contextAction.successTextTranslateKey ? translate(contextAction.successTextTranslateKey) : undefined}
isMini={isMini}
key={contextAction.textTranslateKey}
onPress={(event) => interceptAnonymousUser(() => contextAction.onPress?.(closePopup, {...payload, event}), contextAction.isAnonymousAction)}
onPress={(event) =>
interceptAnonymousUser(
() => contextAction.onPress?.(closePopup, {...payload, event, ...(isMenuAction ? {anchorRef: threedotRef} : {})}),
contextAction.isAnonymousAction,
)
}
description={contextAction.getDescription?.(selection) ?? ''}
isAnonymousAction={contextAction.isAnonymousAction}
isFocused={focusedIndex === index}
Expand Down
7 changes: 4 additions & 3 deletions src/pages/home/report/ContextMenu/ContextMenuActions.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,10 @@ type ContextMenuActionPayload = {
interceptAnonymousUser: (callback: () => void, isAnonymousAction?: boolean) => void;
anchor?: MutableRefObject<HTMLDivElement | View | Text | null>;
checkIfContextMenuActive?: () => void;
openOverflowMenu: (event: GestureResponderEvent | MouseEvent) => void;
openOverflowMenu: (event: GestureResponderEvent | MouseEvent, anchorRef: MutableRefObject<View | null>) => void;
event?: GestureResponderEvent | MouseEvent | KeyboardEvent;
setIsEmojiPickerActive?: (state: boolean) => void;
anchorRef?: MutableRefObject<View | null>;
};

type OnPress = (closePopover: boolean, payload: ContextMenuActionPayload, selection?: string, reportID?: string, draftMessage?: string) => void;
Expand Down Expand Up @@ -490,8 +491,8 @@ const ContextMenuActions: ContextMenuAction[] = [
textTranslateKey: 'reportActionContextMenu.menu',
icon: Expensicons.ThreeDots,
shouldShow: (type, reportAction, isArchivedRoom, betas, anchor, isChronosReport, reportID, isPinnedChat, isUnreadChat, isOffline, isMini) => isMini,
onPress: (closePopover, {openOverflowMenu, event, openContextMenu}) => {
openOverflowMenu(event as GestureResponderEvent | MouseEvent);
onPress: (closePopover, {openOverflowMenu, event, openContextMenu, anchorRef}) => {
openOverflowMenu(event as GestureResponderEvent | MouseEvent, anchorRef ?? {current: null});
openContextMenu();
},
getDescription: () => {},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import calculateAnchorPosition from '@libs/calculateAnchorPosition';
import * as ReportActionsUtils from '@libs/ReportActionsUtils';
import * as IOU from '@userActions/IOU';
import * as Report from '@userActions/Report';
import type {AnchorDimensions} from '@src/styles';
import type {ReportAction} from '@src/types/onyx';
import BaseReportActionContextMenu from './BaseReportActionContextMenu';
import type {ContextMenuAction} from './ContextMenuActions';
Expand Down Expand Up @@ -67,6 +68,10 @@ function PopoverReportActionContextMenu(_props: unknown, ref: ForwardedRef<Repor
const dimensionsEventListener = useRef<EmitterSubscription | null>(null);
const contextMenuAnchorRef = useRef<ContextMenuAnchor>(null);
const contextMenuTargetNode = useRef<HTMLDivElement | null>(null);
const contextMenuDimensions = useRef<AnchorDimensions>({
width: 0,
height: 0,
});

const onPopoverShow = useRef(() => {});
const onPopoverHide = useRef(() => {});
Expand Down Expand Up @@ -164,6 +169,7 @@ function PopoverReportActionContextMenu(_props: unknown, ref: ForwardedRef<Repor
disabledOptions = [],
shouldCloseOnTarget = false,
setIsEmojiPickerActive = () => {},
isOverflowMenu = false,
) => {
const {pageX = 0, pageY = 0} = extractPointerEvent(event);
contextMenuAnchorRef.current = contextMenuAnchor;
Expand All @@ -180,9 +186,10 @@ function PopoverReportActionContextMenu(_props: unknown, ref: ForwardedRef<Repor
onEmojiPickerToggle.current = setIsEmojiPickerActive;

new Promise<void>((resolve) => {
if (!pageX && !pageY && contextMenuAnchorRef.current) {
if (Boolean(!pageX && !pageY && contextMenuAnchorRef.current) || isOverflowMenu) {
calculateAnchorPosition(contextMenuAnchorRef.current).then((position) => {
popoverAnchorPosition.current = position;
popoverAnchorPosition.current = {horizontal: position.horizontal, vertical: position.vertical};
contextMenuDimensions.current = {width: position.vertical, height: position.height};
resolve();
});
} else {
Expand Down Expand Up @@ -319,7 +326,9 @@ function PopoverReportActionContextMenu(_props: unknown, ref: ForwardedRef<Repor
shouldSetModalVisibility={false}
fullscreen
withoutOverlay
anchorDimensions={contextMenuDimensions.current}
anchorRef={anchorRef}
shoudSwitchPositionIfOverflow
>
<BaseReportActionContextMenu
isVisible
Expand Down
3 changes: 3 additions & 0 deletions src/pages/home/report/ContextMenu/ReportActionContextMenu.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ type ShowContextMenu = (
disabledOptions?: ContextMenuAction[],
shouldCloseOnTarget?: boolean,
setIsEmojiPickerActive?: (state: boolean) => void,
isOverflowMenu?: boolean,
) => void;

type ReportActionContextMenu = {
Expand Down Expand Up @@ -117,6 +118,7 @@ function showContextMenu(
disabledActions: ContextMenuAction[] = [],
shouldCloseOnTarget = false,
setIsEmojiPickerActive = () => {},
isOverflowMenu = false,
) {
if (!contextMenuRef.current) {
return;
Expand Down Expand Up @@ -146,6 +148,7 @@ function showContextMenu(
disabledActions,
shouldCloseOnTarget,
setIsEmojiPickerActive,
isOverflowMenu,
);
}

Expand Down
7 changes: 6 additions & 1 deletion src/styles/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,11 @@ import variables from './variables';
type ColorScheme = (typeof CONST.COLOR_SCHEME)[keyof typeof CONST.COLOR_SCHEME];
type StatusBarStyle = (typeof CONST.STATUS_BAR_STYLE)[keyof typeof CONST.STATUS_BAR_STYLE];

type AnchorDimensions = {
width: number;
height: number;
};

type AnchorPosition = {
horizontal: number;
vertical: number;
Expand Down Expand Up @@ -4698,4 +4703,4 @@ const defaultStyles = styles(defaultTheme);

export default styles;
export {defaultStyles};
export type {Styles, ThemeStyles, StatusBarStyle, ColorScheme, AnchorPosition};
export type {Styles, ThemeStyles, StatusBarStyle, ColorScheme, AnchorPosition, AnchorDimensions};

0 comments on commit 9dd66cf

Please sign in to comment.