Skip to content

Commit 812beaf

Browse files
authored
Merge pull request Expensify#68156 from Expensify/revert-67035-feat/popover-optimization
[CP Staging] Revert "PopoverWithMeasuredContent optimization"
2 parents 1218425 + 0033307 commit 812beaf

5 files changed

Lines changed: 40 additions & 79 deletions

File tree

src/components/PopoverWithMeasuredContent/PopoverWithMeasuredContentBase.tsx renamed to src/components/PopoverWithMeasuredContent.tsx

Lines changed: 37 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,38 @@
1-
import {deepEqual} from 'fast-equals';
1+
import {circularDeepEqual, deepEqual} from 'fast-equals';
22
import React, {useContext, useEffect, useMemo, useState} from 'react';
33
import type {LayoutChangeEvent} from 'react-native';
44
import {View} from 'react-native';
5-
import * as ActionSheetAwareScrollView from '@components/ActionSheetAwareScrollView';
6-
import type {PopoverAnchorPosition} from '@components/Modal/types';
7-
import Popover from '@components/Popover';
85
import usePrevious from '@hooks/usePrevious';
96
import useThemeStyles from '@hooks/useThemeStyles';
107
import useWindowDimensions from '@hooks/useWindowDimensions';
118
import ComposerFocusManager from '@libs/ComposerFocusManager';
129
import PopoverWithMeasuredContentUtils from '@libs/PopoverWithMeasuredContentUtils';
1310
import CONST from '@src/CONST';
14-
import type {PopoverWithMeasuredContentProps} from './types';
11+
import type {AnchorDimensions, AnchorPosition} from '@src/styles';
12+
import * as ActionSheetAwareScrollView from './ActionSheetAwareScrollView';
13+
import type {PopoverAnchorPosition} from './Modal/types';
14+
import Popover from './Popover';
15+
import type PopoverProps from './Popover/types';
16+
17+
type PopoverWithMeasuredContentProps = Omit<PopoverProps, 'anchorPosition'> & {
18+
/** The horizontal and vertical anchors points for the popover */
19+
anchorPosition: AnchorPosition;
20+
21+
/** The dimension of anchor component */
22+
anchorDimensions?: AnchorDimensions;
23+
24+
/** Whether we should change the vertical position if the popover's position is overflow */
25+
shouldSwitchPositionIfOverflow?: boolean;
26+
27+
/** Whether handle navigation back when modal show. */
28+
shouldHandleNavigationBack?: boolean;
29+
30+
/** Whether we should should use top side for the anchor positioning */
31+
shouldMeasureAnchorPositionFromTop?: boolean;
32+
33+
/** Whether to skip re-measurement when becoming visible (for components with static dimensions) */
34+
shouldSkipRemeasurement?: boolean;
35+
};
1536

1637
/**
1738
* This is a convenient wrapper around the regular Popover component that allows us to use a more sophisticated
@@ -20,7 +41,7 @@ import type {PopoverWithMeasuredContentProps} from './types';
2041
* anchor position.
2142
*/
2243

23-
function PopoverWithMeasuredContentBase({
44+
function PopoverWithMeasuredContent({
2445
popoverDimensions = {
2546
height: 0,
2647
width: 0,
@@ -210,10 +231,17 @@ function PopoverWithMeasuredContentBase({
210231
style={styles.invisiblePopover}
211232
onLayout={measurePopover}
212233
>
213-
{(isVisible || prevIsVisible) && children}
234+
{children}
214235
</View>
215236
);
216237
}
217-
PopoverWithMeasuredContentBase.displayName = 'PopoverWithMeasuredContentBase';
238+
PopoverWithMeasuredContent.displayName = 'PopoverWithMeasuredContent';
239+
240+
export default React.memo(PopoverWithMeasuredContent, (prevProps, nextProps) => {
241+
if (prevProps.isVisible === nextProps.isVisible && nextProps.isVisible === false) {
242+
return true;
243+
}
244+
return circularDeepEqual(prevProps, nextProps);
245+
});
218246

219-
export default PopoverWithMeasuredContentBase;
247+
export type {PopoverWithMeasuredContentProps};

src/components/PopoverWithMeasuredContent/index.tsx

Lines changed: 0 additions & 40 deletions
This file was deleted.

src/components/PopoverWithMeasuredContent/types.ts

Lines changed: 0 additions & 25 deletions
This file was deleted.

src/components/Search/FilterDropdowns/DropdownButton.tsx

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,9 @@ function DropdownButton({label, value, viewportOffsetTop, PopoverComponent}: Dro
103103
}, [isSmallScreenWidth, styles]);
104104

105105
const popoverContent = useMemo(() => {
106+
if (!isOverlayVisible) {
107+
return null;
108+
}
106109
return PopoverComponent({closeOverlay: toggleOverlay});
107110
// PopoverComponent is stable so we don't need it here as a dep.
108111
// eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps

tests/ui/components/OnboardingHelpDropdownButtonTest.tsx

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,6 @@ import waitForBatchedUpdatesWithAct from '../../utils/waitForBatchedUpdatesWithA
1818
jest.mock('@libs/actions/Link', () => ({
1919
openExternalLink: jest.fn(),
2020
}));
21-
22-
jest.mock('@hooks/useResponsiveLayout', () => () => ({
23-
isSmallScreenWidth: false,
24-
}));
25-
2621
jest.mock('@libs/Navigation/Navigation', () => ({
2722
navigate: jest.fn(),
2823
}));

0 commit comments

Comments
 (0)