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 three-dot menu overlaps with emoji menu #38553

Merged
merged 6 commits into from
Mar 26, 2024
Merged
Show file tree
Hide file tree
Changes from 4 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
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 emojiAnchorDimission = useRef({
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be emojiAnchorDimension

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@DylanDylann I updated.

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,
});
emojiAnchorDimission.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,
});
emojiAnchorDimission.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={emojiAnchorDimission.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});
});
});
}
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 @@ -489,8 +490,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
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 @@ -4716,4 +4721,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};
Loading