fix(modal): move dialogs above the on-screen keyboard - #5045
fix(modal): move dialogs above the on-screen keyboard#5045aliyilmaztech wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
Adds keyboard-aware layout behavior for Modal (and therefore Dialog) rendered via Portal, addressing edge-to-edge Android and iOS where the system doesn’t reliably resize the window when the on-screen keyboard appears.
Changes:
- Introduces a new
useKeyboardOverlaphook to compute how much the keyboard covers a full-screen container. - Updates
Modalto pad its wrapper by the computed keyboard overlap so content stays centered/accessible. - Adds unit tests and a new example dialog demonstrating the behavior with a
TextInput.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/utils/useKeyboardOverlap.tsx | New hook that listens to keyboard + dimension changes and computes keyboard overlap for padding. |
| src/components/Modal.tsx | Applies paddingBottom based on keyboard overlap and measures wrapper height for resize detection. |
| src/components/tests/Modal.test.tsx | Adds unit tests validating padding behavior across resized vs non-resized windows and hide/show transitions. |
| example/src/Examples/Dialogs/index.tsx | Exports the new dialog example. |
| example/src/Examples/Dialogs/DialogWithTextInput.tsx | New example showcasing a dialog with a text input to validate keyboard avoidance. |
| example/src/Examples/DialogExample.tsx | Wires the new example into the dialog examples screen. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| const getCurrentMetrics = () => | ||
| (Keyboard.isVisible() ? Keyboard.metrics() : null) ?? null; |
There was a problem hiding this comment.
Thanks — I checked the versions before changing this, and I don't think the guard would be reachable.
Keyboard.isVisible() and Keyboard.metrics() both shipped in React Native 0.65. This package requires react-native-reanimated >= 4.3.0 as a peer dependency, and Reanimated 4 requires the New Architecture, i.e. React Native 0.78+, so the effective floor here is far above 0.65 (the react-native: "*" peer range is nominal). A typeof guard would be dead code, and the as any cast would drop the KeyboardMetrics | null return type that metrics.screenY relies on below.
There is one real gap though, which is not about old React Native: react-native-web's Keyboard shim implements isVisible, addListener and dismiss, but not metrics or scheduleLayoutAnimation. Today that is harmless because its isVisible() always returns false and it never emits keyboard events, but the code relied on that implicitly. I made it explicit with an optional call and a comment, which keeps the types intact:
const getCurrentMetrics = () =>
// `metrics` is not implemented by react-native-web
(Keyboard.isVisible() ? Keyboard.metrics?.() : null) ?? null;| React.useEffect(() => { | ||
| const subscription = Dimensions.addEventListener('change', ({ screen }) => | ||
| setScreenHeight(screen.height) | ||
| ); | ||
|
|
||
| return () => subscription.remove(); | ||
| }, []); |
There was a problem hiding this comment.
Good catch — the subscription is gone entirely now.
The overlap is calculated from the measured bottom edge of the wrapper and the keyboard's screenY, both relative to the window, so the screen height is no longer part of the calculation and there is nothing to keep in sync with Dimensions. The only remaining effect is the keyboard subscription, which is already gated behind enabled.
| const { top, bottom } = useSafeAreaInsets(); | ||
| const opacity = useAnimatedValue(visible ? 1 : 0); | ||
| const [visibleInternal, setVisibleInternal] = React.useState(visible); | ||
| const [wrapperHeight, setWrapperHeight] = React.useState<number | null>(null); | ||
|
|
||
| const keyboardOverlap = useKeyboardOverlap({ | ||
| enabled: visibleInternal, | ||
| containerHeight: wrapperHeight, | ||
| topInset: top, | ||
| bottomInset: bottom, | ||
| }); |
There was a problem hiding this comment.
Agreed, and fixed — but by removing the assumption rather than by flattening the style.
useKeyboardOverlap no longer receives insets at all. Modal reports the wrapper's y + height from onLayout, and the overlap is the distance between that edge and the keyboard's screenY, so whatever margins end up applied (safe area insets, an override through style, or something else) are already reflected in the measured layout. Reading the flattened style would only cover numeric literals and would still be a second source of truth next to the actual layout.
This also removed the heuristic that detected system-resized windows by comparing the wrapper height with the available screen height: since both edges are now relative to the window, a resized window simply yields an overlap of zero.
Added a test that overrides marginBottom through style and asserts the padding follows the wrapper instead of the safe area inset.
| const onShow = (event: KeyboardEvent) => { | ||
| setMetrics(event.endCoordinates); | ||
| Keyboard.scheduleLayoutAnimation(event); | ||
| }; | ||
|
|
||
| const onHide = (event: KeyboardEvent) => { | ||
| setMetrics(null); | ||
| Keyboard.scheduleLayoutAnimation(event); | ||
| }; | ||
|
|
||
| // `keyboardWillShow` and `keyboardWillHide` are not emitted on Android. | ||
| const subscriptions = | ||
| Platform.OS === 'ios' | ||
| ? [ | ||
| Keyboard.addListener('keyboardWillShow', onShow), | ||
| Keyboard.addListener('keyboardWillHide', onHide), | ||
| ] | ||
| : [ | ||
| Keyboard.addListener('keyboardDidShow', onShow), | ||
| Keyboard.addListener('keyboardDidHide', onHide), | ||
| ]; | ||
|
|
||
| return () => subscriptions.forEach((subscription) => subscription.remove()); | ||
| }, [enabled]); |
There was a problem hiding this comment.
scheduleLayoutAnimation is now called as Keyboard.scheduleLayoutAnimation?.(event) with a comment, since react-native-web doesn't implement it.
I left the remove() calls as they are. The removeListener fallback in useIsKeyboardShown predates React Native 0.65, where EmitterSubscription.remove() became the supported way to unsubscribe; removeListener was deprecated in 0.65 and removed in 0.70, so the fallback path would only run on versions this package can no longer be installed on (react-native-reanimated >= 4.3.0 requires the New Architecture, i.e. React Native 0.78+). Note that useIsKeyboardShown still carries @ts-expect-error annotations for those calls, which is a sign that branch is legacy rather than something new code should copy.
5e69137 to
a550751
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Suppressed comments (5)
src/utils/useKeyboardOverlap.tsx:20
getCurrentMetricscallsKeyboard.isVisible()unconditionally. On platforms/React Native versions whereisVisibleisn’t implemented (notably web), this will throw at runtime. Guard the call with optional chaining (and accessmetricsthrough the same guarded object).
const getCurrentMetrics = () =>
// `metrics` is not implemented by react-native-web
(Keyboard.isVisible() ? Keyboard.metrics?.() : null) ?? null;
src/utils/useKeyboardOverlap.tsx:88
- Overlap is currently computed only from
metrics.screenY, but the PR description says metrics differ across platforms/versions and you need to consider keyboardheighttoo. Compute the keyboard’s top edge as the minimum ofscreenYandscreenHeight - height, then derive overlap from that.
return Math.max(0, containerBottom - metrics.screenY);
}
src/utils/useKeyboardOverlap.tsx:66
- Cleanup assumes every subscription has a
.remove()method. Elsewhere in the codebase (e.g.src/utils/useIsKeyboardShown.tsx) you keep a fallback to deprecatedKeyboard.removeListenerfor older React Native versions. This hook should do the same to avoid crashes on older RN implementations.
// `keyboardWillShow` and `keyboardWillHide` are not emitted on Android.
const subscriptions =
Platform.OS === 'ios'
? [
Keyboard.addListener('keyboardWillShow', onShow),
src/utils/useKeyboardOverlap.tsx:47
- The PR description says the keyboard should not be tracked on web, but the hook will still subscribe when
enabledis true (and may rely on missing Keyboard APIs). Consider bailing out early forPlatform.OS === 'web'.
if (!enabled) {
return undefined;
}
src/utils/useKeyboardOverlap.tsx:2
- The PR description notes overlap should be derived from both
screenYand the keyboardheight(whichever yields a larger overlap), but the current implementation only usesscreenY. ImportingDimensionshere is needed to compute the alternative top edge (screenHeight - height).
This issue also appears on line 87 of the same file.
import { Keyboard, Platform } from 'react-native';
| import { Keyboard, Platform } from 'react-native'; | ||
| import type { KeyboardEvent } from 'react-native'; | ||
|
|
||
| type KeyboardMetrics = NonNullable<ReturnType<typeof Keyboard.metrics>>; |
There was a problem hiding this comment.
Applied — KeyboardEvent['endCoordinates'] is the better spelling here, so thanks.
To be precise about the reasoning: the two are the same type (KeyboardEvent.endCoordinates is declared as KeyboardMetrics in React Native's own definitions, which don't export that alias), and the version concern doesn't apply on this branch — Keyboard.metrics() has been typed since React Native 0.65, while this package requires react-native-reanimated >= 4.3.0, i.e. the New Architecture and React Native 0.78+.
The change is still worth making on clarity grounds: the state holds exactly what keyboardWillShow/keyboardDidShow deliver in event.endCoordinates, so deriving it from KeyboardEvent, which is already imported for the handler signatures, says that directly and drops the NonNullable<ReturnType<…>> indirection.
The Keyboard.metrics?.() call itself stays as is: it's needed to pick up a keyboard that is already open when the modal mounts, and the optional call is there because react-native-web doesn't implement it.
a550751 to
4663c87
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.
Suppressed comments (1)
src/components/tests/Modal.test.tsx:617
- These tests emit only iOS keyboard events (
keyboardWillShow/keyboardWillHide), butuseKeyboardOverlapsubscribes tokeyboardDidShow/keyboardDidHideon non-iOS platforms. Since other test files mutatePlatform.OSwithout always restoring it, this block can become order-dependent/flaky. Emit bothWill*andDid*events here (or explicitly set/restorePlatform.OS) so the listener is triggered deterministically.
const showKeyboard = async () => {
await act(() => {
DeviceEventEmitter.emit('keyboardWillShow', {
endCoordinates: {
screenX: 0,
screenY: screenHeight - KEYBOARD_HEIGHT,
width: 750,
height: KEYBOARD_HEIGHT,
},
});
});
};
const hideKeyboard = async () => {
await act(() => {
DeviceEventEmitter.emit('keyboardWillHide', {
endCoordinates: {
screenX: 0,
screenY: screenHeight,
width: 750,
height: 0,
},
});
});
};
4663c87 to
388e5e4
Compare
|
One more note on the review feedback about the keyboard tests (the comment on Jest gives every test file its own module registry, so a That was still a real gap: the bug this PR fixes is an Android one, and the Android listeners ( // `keyboardWillShow` and `keyboardWillHide` are not emitted on Android
const keyboardEventsPerPlatform: Array<[typeof Platform.OS, string, string]> = [
['ios', 'keyboardWillShow', 'keyboardWillHide'],
['android', 'keyboardDidShow', 'keyboardDidHide'],
];
describe.each(keyboardEventsPerPlatform)(
'when the on-screen keyboard is shown on %s',
(platform, showEvent, hideEvent) => {
const originalPlatform = Platform.OS;
beforeAll(() => {
Platform.OS = platform;
});
afterAll(() => {
Platform.OS = originalPlatform;
});
…All four cases now run on both platforms, and |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.
Suppressed comments (1)
src/utils/useKeyboardOverlap.tsx:42
useState(getCurrentMetrics)runs on the initial render even whenenabledis false, which contradicts the PR note that nothing is computed while hidden and needlessly reads keyboard state for offscreen containers. You can keep the no-flicker behavior when initially enabled by lazy-initializing based onenabledand otherwise starting atnull.
const [metrics, setMetrics] = React.useState<KeyboardMetrics | null>(
getCurrentMetrics
);
Components rendered in a `Portal`, such as `Modal` and `Dialog`, used to be kept above the keyboard by the system: on Android `windowSoftInputMode` was set to `adjustResize`, which shrank the whole window. That no longer happens in edge-to-edge mode, which is enforced since Android 15, where the keyboard is reported as an inset that has to be handled by the app, and it never happened on iOS. Track the keyboard in `Modal` and pad the wrapper by the distance between its measured bottom edge and the top edge of the keyboard, so the content stays centered in the visible area. Both edges are relative to the window, so windows which are still resized by the system need no special casing: their wrapper is already laid out above the keyboard, which puts the overlap at or below zero. Fixes callstack#5021 Fixes callstack#4218 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
388e5e4 to
ff4bd58
Compare
Motivation
Components rendered in a
Portal, such asModalandDialog, are not moved out of the way of the on-screen keyboard.On Android this used to be handled by the system: React Native apps set
android:windowSoftInputMode="adjustResize", so the whole window was shrunk when the keyboard appeared and the modal, which fills the window, was re-centered in the remaining space. This no longer happens in edge-to-edge mode (opt-in viaedgeToEdgeEnabled, enforced since Android 15), where the keyboard is reported as an inset that the app has to handle itself, so the keyboard covers the dialog and its actions. On iOS the window is never resized, so this never worked there at all.KeyboardAvoidingViewcan't be used by the app as a workaround here, because the modal is rendered through aPortal, outside of the app's own view hierarchy.This PR makes
Modaltrack the keyboard and pad its wrapper by the part of it that the keyboard covers, so the content stays centered in the visible area.Dialoggets the fix for free, since it renders aModal.A few notes on the implementation:
onLayout) and the top edge of the keyboard (endCoordinates.screenY), which is the same relationKeyboardAvoidingViewuses. Both are relative to the window, so windows which are still resized by the system need no special casing: their wrapper is already laid out above the keyboard, which puts the overlap at or below zero. Nothing is assumed about the safe area insets either, so overridingmarginTop/marginBottomthrough thestyleprop keeps working.keyboardWillHide/keyboardDidHideclear the metrics instead of feeding them back into the calculation, which is what makesKeyboardAvoidingViewkeep a stale offset on Android in edge-to-edge mode (Fix Keyboard events and KeyboardAvoidingView on Android (edge-to-edge) react/react-native#55855).Keyboard.scheduleLayoutAnimation, so it follows the keyboard animation curve on iOS.Related issue
Fixes #5021
Fixes #4218
Test plan
Automated: new unit tests in
src/components/__tests__/Modal.test.tsxcover a window that is not resized (padding is applied), a window that is resized by the system (padding is not applied), overridden safe area insets, and hiding the keyboard (padding is restored).yarn test,yarn lintandyarn typecheckpass.Manual, using the new Dialog -> With text input example:
edgeToEdgeEnabled=trueinandroid/gradle.properties.Dialogexample and press With text input.edgeToEdgeEnabled=falseto confirm the previous behavior is unchanged, and on iOS, where the dialog now moves as well.