Skip to content

Commit d58a8ae

Browse files
mountinymountiny
authored
Merge pull request Expensify#63393 from callstack-internal/perf/improve-popover-measurments-frequancy
2 parents 425d43f + b9afc28 commit d58a8ae

5 files changed

Lines changed: 85 additions & 34 deletions

File tree

‎src/components/DatePicker/DatePickerModal.tsx‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ function DatePickerModal({
7373
shouldSwitchPositionIfOverflow
7474
shouldEnableNewFocusManagement
7575
shouldMeasureAnchorPositionFromTop={shouldPositionFromTop}
76+
shouldSkipRemeasurement
7677
>
7778
<CalendarPicker
7879
minDate={minDate}

‎src/components/EmojiPicker/EmojiPicker.tsx‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -216,6 +216,7 @@ function EmojiPicker({viewportOffsetTop}: EmojiPickerProps, ref: ForwardedRef<Em
216216
shouldSwitchPositionIfOverflow
217217
shouldEnableNewFocusManagement
218218
restoreFocusType={CONST.MODAL.RESTORE_FOCUS_TYPE.DELETE}
219+
shouldSkipRemeasurement
219220
>
220221
<FocusTrapForModal active={isEmojiPickerVisible}>
221222
<View>

‎src/components/EmojiPicker/EmojiPickerMenu/index.native.tsx‎

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,8 @@ import useSingleExecution from '@hooks/useSingleExecution';
1414
import useStyleUtils from '@hooks/useStyleUtils';
1515
import useThemeStyles from '@hooks/useThemeStyles';
1616
import useWindowDimensions from '@hooks/useWindowDimensions';
17-
import * as EmojiUtils from '@libs/EmojiUtils';
17+
import type {EmojiPickerList, EmojiPickerListItem} from '@libs/EmojiUtils';
18+
import {getRemovedSkinToneEmoji} from '@libs/EmojiUtils';
1819
import CONST from '@src/CONST';
1920
import type {TranslationPaths} from '@src/languages/types';
2021
import BaseEmojiPickerMenu from './BaseEmojiPickerMenu';
@@ -44,7 +45,7 @@ function EmojiPickerMenu({onEmojiSelected, activeEmoji}: EmojiPickerMenuProps, r
4445
} = useEmojiPickerMenu();
4546
const StyleUtils = useStyleUtils();
4647

47-
const updateEmojiList = (emojiData: EmojiUtils.EmojiPickerList | Emoji[], headerData: number[] = []) => {
48+
const updateEmojiList = (emojiData: EmojiPickerList | Emoji[], headerData: number[] = []) => {
4849
setFilteredEmojis(emojiData);
4950
setHeaderIndices(headerData);
5051

@@ -68,17 +69,20 @@ function EmojiPickerMenu({onEmojiSelected, activeEmoji}: EmojiPickerMenuProps, r
6869
}
6970
}, 300);
7071

71-
const scrollToHeader = (headerIndex: number) => {
72-
const calculatedOffset = Math.floor(headerIndex / CONST.EMOJI_NUM_PER_ROW) * CONST.EMOJI_PICKER_HEADER_HEIGHT;
73-
emojiListRef.current?.scrollToOffset({offset: calculatedOffset, animated: true});
74-
};
72+
const scrollToHeader = useCallback(
73+
(headerIndex: number) => {
74+
const calculatedOffset = Math.floor(headerIndex / CONST.EMOJI_NUM_PER_ROW) * CONST.EMOJI_PICKER_HEADER_HEIGHT;
75+
emojiListRef.current?.scrollToOffset({offset: calculatedOffset, animated: true});
76+
},
77+
[emojiListRef],
78+
);
7579

7680
/**
7781
* Given an emoji item object, render a component based on its type.
7882
* Items with the code "SPACER" return nothing and are used to fill rows up to 8
7983
* so that the sticky headers function properly.
8084
*/
81-
const renderItem: ListRenderItem<EmojiUtils.EmojiPickerListItem> = useCallback(
85+
const renderItem: ListRenderItem<EmojiPickerListItem> = useCallback(
8286
({item, target}) => {
8387
const code = item.code;
8488
const types = 'types' in item ? item.types : undefined;
@@ -96,7 +100,7 @@ function EmojiPickerMenu({onEmojiSelected, activeEmoji}: EmojiPickerMenuProps, r
96100
}
97101

98102
const emojiCode = typeof preferredSkinTone === 'number' && preferredSkinTone !== -1 && types?.at(preferredSkinTone) ? types.at(preferredSkinTone) : code;
99-
const shouldEmojiBeHighlighted = !!activeEmoji && EmojiUtils.getRemovedSkinToneEmoji(emojiCode) === EmojiUtils.getRemovedSkinToneEmoji(activeEmoji);
103+
const shouldEmojiBeHighlighted = !!activeEmoji && getRemovedSkinToneEmoji(emojiCode) === getRemovedSkinToneEmoji(activeEmoji);
100104

101105
return (
102106
<EmojiPickerMenuItem

‎src/components/PopoverWithMeasuredContent.tsx‎

Lines changed: 50 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,9 @@ type PopoverWithMeasuredContentProps = Omit<PopoverProps, 'anchorPosition'> & {
2929

3030
/** Whether we should should use top side for the anchor positioning */
3131
shouldMeasureAnchorPositionFromTop?: boolean;
32+
33+
/** Whether to skip re-measurement when becoming visible (for components with static dimensions) */
34+
shouldSkipRemeasurement?: boolean;
3235
};
3336

3437
/**
@@ -66,6 +69,7 @@ function PopoverWithMeasuredContent({
6669
shouldHandleNavigationBack = false,
6770
shouldEnableNewFocusManagement,
6871
shouldMeasureAnchorPositionFromTop = false,
72+
shouldSkipRemeasurement = false,
6973
...props
7074
}: PopoverWithMeasuredContentProps) {
7175
const actionSheetAwareScrollViewContext = useContext(ActionSheetAwareScrollView.ActionSheetAwareScrollViewContext);
@@ -75,15 +79,28 @@ function PopoverWithMeasuredContent({
7579
const [popoverHeight, setPopoverHeight] = useState(popoverDimensions.height);
7680
const [isContentMeasured, setIsContentMeasured] = useState(popoverWidth > 0 && popoverHeight > 0);
7781
const prevIsVisible = usePrevious(isVisible);
82+
const prevAnchorPosition = usePrevious(anchorPosition);
83+
const prevWindowDimensions = usePrevious({windowWidth, windowHeight});
7884

7985
const modalId = useMemo(() => ComposerFocusManager.getId(), []);
8086

8187
if (!prevIsVisible && isVisible && shouldEnableNewFocusManagement) {
8288
ComposerFocusManager.saveFocusState(modalId);
8389
}
8490

85-
if (!prevIsVisible && isVisible && isContentMeasured) {
86-
setIsContentMeasured(false);
91+
if (!prevIsVisible && isVisible && isContentMeasured && !shouldSkipRemeasurement) {
92+
// Check if anything significant changed that would require re-measurement
93+
const hasAnchorPositionChanged = !isEqual(prevAnchorPosition, anchorPosition);
94+
const hasWindowSizeChanged = !isEqual(prevWindowDimensions, {windowWidth, windowHeight});
95+
const hasStaticDimensions = popoverDimensions.width > 0 && popoverDimensions.height > 0;
96+
97+
// Only reset if:
98+
// 1. We don't have static dimensions, OR
99+
// 2. The anchor position changed significantly, OR
100+
// 3. The window size changed significantly
101+
if (!hasStaticDimensions || hasAnchorPositionChanged || hasWindowSizeChanged) {
102+
setIsContentMeasured(false);
103+
}
87104
}
88105

89106
/**
@@ -144,28 +161,39 @@ function PopoverWithMeasuredContent({
144161
};
145162
}, [anchorPosition, anchorAlignment, popoverWidth, popoverHeight]);
146163

147-
const horizontalShift = PopoverWithMeasuredContentUtils.computeHorizontalShift(adjustedAnchorPosition.left, popoverWidth, windowWidth);
148-
const verticalShift = PopoverWithMeasuredContentUtils.computeVerticalShift(
149-
adjustedAnchorPosition.top,
150-
popoverHeight,
151-
windowHeight,
152-
anchorDimensions.height,
153-
shouldSwitchPositionIfOverflow,
154-
);
155-
const shiftedAnchorPosition: PopoverAnchorPosition = {
156-
left: adjustedAnchorPosition.left + horizontalShift,
157-
...(shouldMeasureAnchorPositionFromTop ? {top: adjustedAnchorPosition.top + verticalShift} : {}),
158-
};
164+
const positionCalculations = useMemo(() => {
165+
const horizontalShift = PopoverWithMeasuredContentUtils.computeHorizontalShift(adjustedAnchorPosition.left, popoverWidth, windowWidth);
166+
const verticalShift = PopoverWithMeasuredContentUtils.computeVerticalShift(
167+
adjustedAnchorPosition.top,
168+
popoverHeight,
169+
windowHeight,
170+
anchorDimensions.height,
171+
shouldSwitchPositionIfOverflow,
172+
);
173+
return {horizontalShift, verticalShift};
174+
}, [adjustedAnchorPosition.left, adjustedAnchorPosition.top, popoverWidth, popoverHeight, windowWidth, windowHeight, anchorDimensions.height, shouldSwitchPositionIfOverflow]);
175+
176+
const shiftedAnchorPosition: PopoverAnchorPosition = useMemo(() => {
177+
const result: PopoverAnchorPosition = {
178+
left: adjustedAnchorPosition.left + positionCalculations.horizontalShift,
179+
};
159180

160-
if (anchorAlignment.vertical === CONST.MODAL.ANCHOR_ORIGIN_VERTICAL.TOP) {
161-
const top = adjustedAnchorPosition.top + verticalShift;
162-
const maxTop = windowHeight - popoverHeight - verticalShift;
163-
shiftedAnchorPosition.top = Math.min(Math.max(verticalShift, top), maxTop);
164-
}
181+
if (shouldMeasureAnchorPositionFromTop) {
182+
result.top = adjustedAnchorPosition.top + positionCalculations.verticalShift;
183+
}
165184

166-
if (anchorAlignment.vertical === CONST.MODAL.ANCHOR_ORIGIN_VERTICAL.BOTTOM) {
167-
shiftedAnchorPosition.bottom = windowHeight - (adjustedAnchorPosition.top + popoverHeight) - verticalShift;
168-
}
185+
if (anchorAlignment.vertical === CONST.MODAL.ANCHOR_ORIGIN_VERTICAL.TOP) {
186+
const top = adjustedAnchorPosition.top + positionCalculations.verticalShift;
187+
const maxTop = windowHeight - popoverHeight - positionCalculations.verticalShift;
188+
result.top = Math.min(Math.max(positionCalculations.verticalShift, top), maxTop);
189+
}
190+
191+
if (anchorAlignment.vertical === CONST.MODAL.ANCHOR_ORIGIN_VERTICAL.BOTTOM) {
192+
result.bottom = windowHeight - (adjustedAnchorPosition.top + popoverHeight) - positionCalculations.verticalShift;
193+
}
194+
195+
return result;
196+
}, [adjustedAnchorPosition, positionCalculations, anchorAlignment.vertical, windowHeight, popoverHeight, shouldMeasureAnchorPositionFromTop]);
169197

170198
return isContentMeasured ? (
171199
<Popover

‎src/pages/home/report/ReportActionCompose/ReportActionCompose.tsx‎

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -416,12 +416,29 @@ function ReportActionCompose({
416416
// eslint-disable-next-line react-compiler/react-compiler
417417
onSubmitAction = handleSendMessage;
418418

419+
const emojiPositionValues = useMemo(
420+
() => ({
421+
secondaryRowHeight: styles.chatItemComposeSecondaryRow.height,
422+
secondaryRowMarginTop: styles.chatItemComposeSecondaryRow.marginTop,
423+
secondaryRowMarginBottom: styles.chatItemComposeSecondaryRow.marginBottom,
424+
composeBoxMinHeight: styles.chatItemComposeBox.minHeight,
425+
emojiButtonHeight: styles.chatItemEmojiButton.height,
426+
}),
427+
[
428+
styles.chatItemComposeSecondaryRow.height,
429+
styles.chatItemComposeSecondaryRow.marginTop,
430+
styles.chatItemComposeSecondaryRow.marginBottom,
431+
styles.chatItemComposeBox.minHeight,
432+
styles.chatItemEmojiButton.height,
433+
],
434+
);
435+
419436
const emojiShiftVertical = useMemo(() => {
420-
const chatItemComposeSecondaryRowHeight = styles.chatItemComposeSecondaryRow.height + styles.chatItemComposeSecondaryRow.marginTop + styles.chatItemComposeSecondaryRow.marginBottom;
421-
const reportActionComposeHeight = styles.chatItemComposeBox.minHeight + chatItemComposeSecondaryRowHeight;
422-
const emojiOffsetWithComposeBox = (styles.chatItemComposeBox.minHeight - styles.chatItemEmojiButton.height) / 2;
437+
const chatItemComposeSecondaryRowHeight = emojiPositionValues.secondaryRowHeight + emojiPositionValues.secondaryRowMarginTop + emojiPositionValues.secondaryRowMarginBottom;
438+
const reportActionComposeHeight = emojiPositionValues.composeBoxMinHeight + chatItemComposeSecondaryRowHeight;
439+
const emojiOffsetWithComposeBox = (emojiPositionValues.composeBoxMinHeight - emojiPositionValues.emojiButtonHeight) / 2;
423440
return reportActionComposeHeight - emojiOffsetWithComposeBox - CONST.MENU_POSITION_REPORT_ACTION_COMPOSE_BOTTOM;
424-
}, [styles]);
441+
}, [emojiPositionValues]);
425442

426443
const validateMaxLength = useCallback(
427444
(value: string) => {

0 commit comments

Comments
 (0)