[manchoudhary]

Motivation

Menu only learned keyboard height from keyboardDidShow after mount. iOS does not replay that event for a keyboard that is already open, so a Menu that mounts while the keyboard is visible (for example a row added while typing) measured against the full window and laid items under the keyboard.

This uses Keyboard.metrics() when showing the menu (and seeds the ref on mount) so the already-open keyboard is accounted for.

Related issue

Fixes #5096

Screenshots / Videos

N/A — layout behavior; verified manually on iPhone 16 simulator.

Test plan

• yarn lint
• yarn typecheck
• yarn test
• Manual: open Menu with keyboard already open; menu stays above keyboard and scrolls instead of placing items under the keyboard

[MikitasK]

good job 👍 👍
I reproduced this #5096 issue on iOS: this branch caps the list correctly & stays
scrollable ✅

I suppose, just 1 thing is blocking the merge atm: web crash noted inline.
rest looks good to me 👌

[manchoudhary]

Thanks for the review! Addressed both points:

1. Blocker: The call itself is now optional: Keyboard.metrics?.()?.height ?? fallback. On react-native-web, where Keyboard.metrics is undefined, it no longer throws undefined is not a function. I added a test that sets Keyboard.metrics to undefined and asserts the menu still positions correctly. Spies and the original value are restored in finally.

2. #5073: It rewrites the same windowLayoutResult.height - keyboardHeightRef.current block without handling an already-open keyboard. This PR changes that line to use getKeyboardHeight(keyboardHeightRef.current). The test “accounts for a keyboard that is already open when the menu mounts” asserts the final rendered height, so it will fail if that call is dropped in a merge. I’ll rebase on whichever lands first, and I’ll also leave a note on #5073.

yarn lint, yarn typecheck and yarn test all pass. Ready for another look.

[MikitasK]

thanks for quick update 🙏
both points look good now 👌

just 2 thing atm:

1. merge with main broke imports in test file - happy to approve once that's fixed
2. could we add a short before/after video from simulator? it'd make it much easier for the final pass

[manchoudhary]

I have removed duplicate code

[Doberjohn]

CI ran on 87b2485, and Lint, Typecheck and the unit tests fail on Menu.test.tsx, from the merge of main: line 3 repeats the expect, it, jest import from @jest/globals, and the react-native import on line 1 lost Platform, which the web focus tests use. Could you drop the extra import and import Keyboard and Platform together? Then yarn lint, yarn typecheck and yarn jest src/components/__tests__/Menu.test.tsx should pass.

[Doberjohn]

Thanks @manchoudhary , this looks good: Keyboard.metrics?.() covers a keyboard that's already open, seeding the ref on mount gets the first layout right, and the two tests cover both that and web without metrics. Approving.