Skip to content

Commit 393d5a7

Browse files
committed
perf: narrow useCloseOnModalCover to a derived boolean selector
1 parent 909b422 commit 393d5a7

4 files changed

Lines changed: 16 additions & 17 deletions

File tree

src/components/PopoverMenu/v2/content/useCloseOnModalCover.ts

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,13 @@
1+
import {isModalCoveringSelector} from '@selectors/Modal';
12
import {useEffect, useRef} from 'react';
23
import useOnyx from '@hooks/useOnyx';
34
import ONYXKEYS from '@src/ONYXKEYS';
45

56
function useCloseOnModalCover(isVisible: boolean, close: () => void): void {
6-
const [modal, modalMeta] = useOnyx(ONYXKEYS.MODAL);
7+
const [isCovered, modalMeta] = useOnyx(ONYXKEYS.MODAL, {selector: isModalCoveringSelector});
78
const isLoaded = modalMeta.status === 'loaded';
8-
const isCovered = !!modal?.willAlertModalBecomeVisible && !modal?.isPopover;
9-
// Seed with current value: mounting inside an already-covered modal isn't a fresh cover.
109
const wasCoveredRef = useRef(isCovered);
1110
useEffect(() => {
12-
// Onyx hydration can flip undefined→true with a stale cover from a prior modal; wait for the loaded snapshot.
1311
if (!isLoaded) {
1412
return;
1513
}

src/selectors/Modal.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,4 +6,6 @@ const willAlertModalBecomeVisibleSelector = (modal: OnyxEntry<Modal>) => modal?.
66

77
const isRHPVisibleSelector = (modal: OnyxEntry<Modal>) => modal?.type === CONST.MODAL.MODAL_TYPE.RIGHT_DOCKED;
88

9-
export {willAlertModalBecomeVisibleSelector, isRHPVisibleSelector};
9+
const isModalCoveringSelector = (modal: OnyxEntry<Modal>) => !!modal?.willAlertModalBecomeVisible && !modal?.isPopover;
10+
11+
export {willAlertModalBecomeVisibleSelector, isRHPVisibleSelector, isModalCoveringSelector};

tests/unit/PopoverMenuV2Test.tsx

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -138,14 +138,15 @@ const mockModalState: {
138138
jest.mock('@hooks/useOnyx', () => {
139139
/* eslint-disable @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-unsafe-call, @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-unsafe-return -- jest.requireActual returns an untyped module; standard RN-mock pattern in this repo. */
140140
const ReactActual = jest.requireActual('react');
141-
return () => {
141+
return (_key: string, options?: {selector?: (v: unknown) => unknown}) => {
142142
const [, force] = ReactActual.useState({});
143143
ReactActual.useEffect(() => {
144144
const listener = () => force({});
145145
mockModalState.listeners.add(listener);
146146
return () => mockModalState.listeners.delete(listener);
147147
}, []);
148-
return [mockModalState.value, {status: 'loaded'}];
148+
const value = options?.selector ? options.selector(mockModalState.value) : mockModalState.value;
149+
return [value, {status: 'loaded'}];
149150
};
150151
});
151152

tests/unit/createContextNamespaceTest.tsx

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import {render, screen} from '@testing-library/react-native';
2-
import React, {useImperativeHandle} from 'react';
3-
import type {Ref} from 'react';
2+
import React, {use, useImperativeHandle} from 'react';
3+
import type {Context, Ref} from 'react';
44
import Text from '@components/Text';
55
import createContextNamespace from '@hooks/createContextNamespace';
66

@@ -14,10 +14,10 @@ function Consumer() {
1414
return <Text>{value.label}</Text>;
1515
}
1616

17-
type CaptureHandle<T> = {value: T};
17+
type CaptureHandle<T> = {value: T | null};
1818

19-
function CaptureConsumer<T>({useCtx, consumerName, captureRef}: {useCtx: (consumerName: string) => T; consumerName: string; captureRef: Ref<CaptureHandle<T>>}) {
20-
const value = useCtx(consumerName);
19+
function CaptureConsumer<T>({context, captureRef}: {context: Context<T | null>; captureRef: Ref<CaptureHandle<T>>}) {
20+
const value = use(context);
2121
useImperativeHandle(captureRef, () => ({value}), [value]);
2222
return null;
2323
}
@@ -60,8 +60,7 @@ describe('createContextNamespace', () => {
6060
render(
6161
<FooContext value={value}>
6262
<CaptureConsumer<FooValue>
63-
useCtx={useFoo}
64-
consumerName="useFoo"
63+
context={FooContext}
6564
captureRef={captureRef}
6665
/>
6766
</FooContext>,
@@ -70,13 +69,12 @@ describe('createContextNamespace', () => {
7069
});
7170

7271
it('does NOT throw when the value is a falsy-but-non-null primitive (e.g. 0)', () => {
73-
const [ZeroContext, useZero] = createContextNamespace('ZeroRoot')<number>();
72+
const [ZeroContext] = createContextNamespace('ZeroRoot')<number>();
7473
const captureRef = React.createRef<CaptureHandle<number>>();
7574
render(
7675
<ZeroContext value={0}>
7776
<CaptureConsumer<number>
78-
useCtx={useZero}
79-
consumerName="useZero"
77+
context={ZeroContext}
8078
captureRef={captureRef}
8179
/>
8280
</ZeroContext>,

0 commit comments

Comments
 (0)