Skip to content

Commit

Permalink
fix: incorrect Modal layout (if it was open while keyboard was visi…
Browse files Browse the repository at this point in the history
…ble) (#624)

## 📜 Description

Fixed a problem of `Modal` window not stretching to full height of the
screen if `Modal` was opened when keyboard was open.

## 💡 Motivation and Context

The problem was caused by the fact that I attached
`OnApplyWindowListener` to a `rootView`. Somehow it was breaking a
layout and always kept `keyboard space` if Modal was opened when
keyboard is visible.

The first approach was an attempt to resize that window. I've tried to
specify `.layoutParams` but it was a failed attempt. Why? Because in the
end I had two options:
- either it wasn't working.
- triggered a crash;

After some time I wrote a code where I set `OnApplyWindowListener`
conditionally:
- if keyboard open then I set it to `DialogRootViewGroup` (I was getting
an instance of `ReactModalHostView.hostVIew` via reflection);
- if keyboard is not open then attach to root view.

While it was working approach it has several downsides:
- it is pretty complex (reflection, conditional code, making some
internals methods `isKeyboardVisible` public one);
- when you often toggle keyboard between emoji/text states, then at some
point it may not update position properly.

So I decided to look for a more robust approach - and I decided to apply
changes that I used in `EdgeToEdgeReactViewGroup`. I created a custom
view (0 width and height) and was attaching it to the `rootView` of the
modal. In this case I don't inject my custom handler into `rootView`, so
the layout is managed properly.

While it works I noticed that sometimes red (animated) or gray
(reanimated) circles may behave differently and eventually they have a
broken state. The problem is that sometimes `onProgress`/`onEnd` may be
triggered from main callback. I added `Suspense` mechanism where I can
toggle off the functionality on demand and pause callbacks events.

While it was working good on Android 12+ it wasn't working on Android <
12. The problem comes from the fact that on these OS versions Android
emulates keyboard transitions, and listens to main window. So if we
pause main window callback then all transitions are not working - the
obvious way to fix is to make `.suspense` call conditional.

I refactored that part (reduced code duplication, moved everything to
reusable code, etc.) and now this PR is ready to be merged 😊

Closes
#619

## 📢 Changelog

<!-- High level overview of important changes -->
<!-- For example: fixed status bar manipulation; added new types
declarations; -->
<!-- If your changes don't affect one of platform/language below - then
remove this platform/language -->

### JS

- added `suspendable` to `cspell` ignore;

### E2E

- added new test verifying a layout correctness when Modal has been
opened with an opened keyboard;

### Android

- added new `constants` package with new `Keyboard` class (with
`IS_ANIMATION_EMULATED` field);
- added `Suspendable` interface;
- `KeyboardAnimationCallback` now implements `Suspendable`;
- added early return to `onStart`/`onProgress`/`onEnd` callbacks if
callback is suspended;
- change `WindowInsetsCompat.CONSUMED` back to `insets`;
- `syncKeyboardPosition` now can accept `height` and `visible` as
params;
- pass `KeyboardAnimationCallback` instance to `ModalAttachedWatcher`
from `EdgeToEdgeReactViewGroup`;
- add `eventView` to `rootView` of `Modal` when modal is shown;
- set `setOnApplyWindowInsetsListener` to `eventView`;
- remove `eventView` when `Modal` dismissed;
- force `syncKeyboardPosition` to closed keyboard state when `Modal` is
shown on Android 12+;
- suspend main `KeyboardAnimationCallback` when `Modal` is shown on
Android 12+;
- unsuspend main `KeyboardAnimationCallback` when `Modal` is dismissed;

## 🤔 How Has This Been Tested?

Tested manually on:
- Pixel 3A (API 33);
- Pixel 6 Pro (API 28);

Additionally covered everything with e2e tests 😎 

## 📸 Screenshots (if appropriate):

|Device|Before|After|
|-------|-------|-----|
|Pixel 3A (API 33)|<img width="412" alt="image"
src="https://github.com/user-attachments/assets/3119149d-a5b5-41fd-911b-a761dbf80896">|<img
width="405" alt="image"
src="https://github.com/user-attachments/assets/05096970-5830-4e77-b8a2-bb9c3b90fa1a">|
|Pixel 6 Pro (API 28)|<img width="417" alt="image"
src="https://github.com/user-attachments/assets/4e89428f-9271-4e32-b837-66e6a3432b5d">|<img
width="413" alt="image"
src="https://github.com/user-attachments/assets/b027f7e1-7618-4511-a5f1-c0bbbf0d64f2">|

## 📝 Checklist

- [x] CI successfully passed
- [x] I added new mocks and corresponding unit-tests if library API was
changed
  • Loading branch information
kirillzyusko authored Oct 10, 2024
1 parent 4b52227 commit bb5e9e1
Show file tree
Hide file tree
Showing 18 changed files with 107 additions and 16 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package com.reactnativekeyboardcontroller.constants

import android.os.Build

object Keyboard {
val IS_ANIMATION_EMULATED = Build.VERSION.SDK_INT < Build.VERSION_CODES.R
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package com.reactnativekeyboardcontroller.listeners

import android.os.Build
import android.view.View
import android.view.ViewTreeObserver.OnGlobalFocusChangeListener
import androidx.core.graphics.Insets
Expand All @@ -14,6 +13,7 @@ import com.facebook.react.uimanager.ThemedReactContext
import com.facebook.react.uimanager.UIManagerHelper
import com.facebook.react.views.textinput.ReactEditText
import com.facebook.react.views.view.ReactViewGroup
import com.reactnativekeyboardcontroller.constants.Keyboard
import com.reactnativekeyboardcontroller.events.KeyboardTransitionEvent
import com.reactnativekeyboardcontroller.extensions.dispatchEvent
import com.reactnativekeyboardcontroller.extensions.dp
Expand All @@ -24,7 +24,7 @@ import com.reactnativekeyboardcontroller.log.Logger
import kotlin.math.abs

private val TAG = KeyboardAnimationCallback::class.qualifiedName
private val isResizeHandledInCallbackMethods = Build.VERSION.SDK_INT < Build.VERSION_CODES.R
private val isResizeHandledInCallbackMethods = Keyboard.IS_ANIMATION_EMULATED

data class KeyboardAnimationCallbackConfig(
val persistentInsetTypes: Int,
Expand All @@ -33,13 +33,22 @@ data class KeyboardAnimationCallbackConfig(
val hasTranslucentNavigationBar: Boolean = false,
)

interface Suspendable {
var isSuspended: Boolean

fun suspend(suspended: Boolean) {
isSuspended = suspended
}
}

class KeyboardAnimationCallback(
val eventPropagationView: ReactViewGroup,
val view: View,
val context: ThemedReactContext?,
private val config: KeyboardAnimationCallbackConfig,
) : WindowInsetsAnimationCompat.Callback(config.dispatchMode),
OnApplyWindowInsetsListener {
OnApplyWindowInsetsListener,
Suspendable {
private val surfaceId = UIManagerHelper.getSurfaceId(eventPropagationView)

// state variables
Expand All @@ -50,6 +59,7 @@ class KeyboardAnimationCallback(
private var duration = 0
private var viewTagFocused = -1
private var animationsToSkip = hashSetOf<WindowInsetsAnimationCompat>()
override var isSuspended: Boolean = false

// listeners
private val focusListener =
Expand Down Expand Up @@ -147,15 +157,15 @@ class KeyboardAnimationCallback(
this.onKeyboardResized(keyboardHeight)
}

return WindowInsetsCompat.CONSUMED
return insets
}

@Suppress("detekt:ReturnCount")
override fun onStart(
animation: WindowInsetsAnimationCompat,
bounds: WindowInsetsAnimationCompat.BoundsCompat,
): WindowInsetsAnimationCompat.BoundsCompat {
if (!animation.isKeyboardAnimation) {
if (!animation.isKeyboardAnimation || isSuspended) {
return bounds
}

Expand Down Expand Up @@ -211,7 +221,13 @@ class KeyboardAnimationCallback(
// onProgress() is called when any of the running animations progress...

// ignore non-keyboard animation or animation that we intentionally want to skip
runningAnimations.find { it.isKeyboardAnimation && !animationsToSkip.contains(it) } ?: return insets
val shouldSkipAnimation =
runningAnimations.find {
it.isKeyboardAnimation && !animationsToSkip.contains(it)
} == null
if (isSuspended || shouldSkipAnimation) {
return insets
}

// First we get the insets which are potentially deferred
val typesInset = insets.getInsets(config.deferredInsetTypes)
Expand Down Expand Up @@ -262,7 +278,7 @@ class KeyboardAnimationCallback(
override fun onEnd(animation: WindowInsetsAnimationCompat) {
super.onEnd(animation)

if (!animation.isKeyboardAnimation) {
if (!animation.isKeyboardAnimation || isSuspended) {
return
}

Expand Down Expand Up @@ -311,10 +327,13 @@ class KeyboardAnimationCallback(
duration = 0
}

fun syncKeyboardPosition() {
val keyboardHeight = getCurrentKeyboardHeight()
fun syncKeyboardPosition(
height: Double? = null,
isVisible: Boolean? = null,
) {
val keyboardHeight = height ?: getCurrentKeyboardHeight()
// update internal state
isKeyboardVisible = isKeyboardVisible()
isKeyboardVisible = isVisible ?: isKeyboardVisible()
prevKeyboardHeight = keyboardHeight
isTransitioning = false
duration = 0
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.reactnativekeyboardcontroller.modal

import android.view.ViewGroup
import android.view.WindowManager
import androidx.core.view.ViewCompat
import com.facebook.react.uimanager.ThemedReactContext
Expand All @@ -10,16 +11,20 @@ import com.facebook.react.uimanager.events.EventDispatcherListener
import com.facebook.react.views.modal.ReactModalHostView
import com.facebook.react.views.view.ReactViewGroup
import com.reactnativekeyboardcontroller.BuildConfig
import com.reactnativekeyboardcontroller.constants.Keyboard
import com.reactnativekeyboardcontroller.extensions.removeSelf
import com.reactnativekeyboardcontroller.listeners.KeyboardAnimationCallback
import com.reactnativekeyboardcontroller.listeners.KeyboardAnimationCallbackConfig
import com.reactnativekeyboardcontroller.log.Logger

private val TAG = ModalAttachedWatcher::class.qualifiedName
private val areEventsComingFromOwnWindow = !Keyboard.IS_ANIMATION_EMULATED

class ModalAttachedWatcher(
private val view: ReactViewGroup,
private val reactContext: ThemedReactContext,
private val config: () -> KeyboardAnimationCallbackConfig,
private var callback: () -> KeyboardAnimationCallback?,
) : EventDispatcherListener {
private val archType = if (BuildConfig.IS_NEW_ARCHITECTURE_ENABLED) UIManagerType.FABRIC else UIManagerType.DEFAULT
private val uiManager = UIManagerHelper.getUIManager(reactContext.reactApplicationContext, archType)
Expand All @@ -44,9 +49,13 @@ class ModalAttachedWatcher(

val dialog = modal.dialog
val window = dialog?.window
val rootView = window?.decorView?.rootView
val rootView = window?.decorView?.rootView as ViewGroup?

if (rootView != null) {
val eventView =
ReactViewGroup(reactContext).apply {
layoutParams = ViewGroup.LayoutParams(0, 0)
}
val callback =
KeyboardAnimationCallback(
view = rootView,
Expand All @@ -55,16 +64,35 @@ class ModalAttachedWatcher(
config = config(),
)

rootView.addView(eventView)
// on Android < 12 all events for `WindowInsetsAnimationCallback`
// go through main `rootView`, so we don't need to stop main
// callback - otherwise keyboard transitions will not be animated
this.callback()?.suspend(areEventsComingFromOwnWindow)
ViewCompat.setWindowInsetsAnimationCallback(rootView, callback)
ViewCompat.setOnApplyWindowInsetsListener(rootView, callback)
ViewCompat.setOnApplyWindowInsetsListener(eventView, callback)

dialog.setOnDismissListener {
if (areEventsComingFromOwnWindow) {
// when modal is shown then keyboard will be hidden by default
//
// - if events are coming from main window - then keyboard position
// will be synchronized from main window callback
// - if events are coming from modal window - then we need to update
// position ourself, because callback can be attached after keyboard
// auto-dismissal and we may miss some events and keyboard position
// will be outdated
callback.syncKeyboardPosition(0.0, false)
}

dialog?.setOnDismissListener {
callback.syncKeyboardPosition()
callback.destroy()
eventView.removeSelf()
this.callback()?.suspend(false)
}

// imitating edge-to-edge mode behavior
window.setSoftInputMode(WindowManager.LayoutParams.SOFT_INPUT_ADJUST_NOTHING)
window?.setSoftInputMode(WindowManager.LayoutParams.SOFT_INPUT_ADJUST_NOTHING)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ class EdgeToEdgeReactViewGroup(
)

// managers/watchers
private val modalAttachedWatcher = ModalAttachedWatcher(this, reactContext, ::config)
private val modalAttachedWatcher = ModalAttachedWatcher(this, reactContext, ::config, ::getKeyboardCallback)

init {
reactContext.setupWindowDimensionsListener()
Expand Down Expand Up @@ -195,6 +195,10 @@ class EdgeToEdgeReactViewGroup(
}
// endregion

// region Helpers
private fun getKeyboardCallback(): KeyboardAnimationCallback? = this.callback
// endregion

// region Props setters
fun setStatusBarTranslucent(isStatusBarTranslucent: Boolean) {
this.isStatusBarTranslucent = isStatusBarTranslucent
Expand Down
3 changes: 2 additions & 1 deletion cspell.json
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,8 @@
"zxcvbnmasdfghjklqwertyuiop",
"autoresizing",
"focusable",
"iphonesimulator"
"iphonesimulator",
"suspendable"
],
"ignorePaths": [
"node_modules",
Expand Down
32 changes: 32 additions & 0 deletions e2e/kit/008-modal.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ describe("Modal integration", () => {
it("should open modal", async () => {
await waitAndTap("show_button");
await waitForElementById("close_button");
await waitForExpect(async () => {
await expectBitmapsToBeEqual("ModalJustOpened");
});
});

it("should open keyboard", async () => {
Expand All @@ -34,4 +37,33 @@ describe("Modal integration", () => {
await expectBitmapsToBeEqual("ModalKeyboardClosed");
});
});

it("should open keyboard without Modal", async () => {
await waitAndTap("keyboard_animation_text_input");
});

it("should open modal and close keyboard", async () => {
await waitAndTap("show_button");
await waitForElementById("close_button");
await waitForExpect(async () => {
await expectBitmapsToBeEqual("ModalJustOpened");
});
});

it("should open keyboard again", async () => {
await element(by.id("keyboard_animation_text_input")).atIndex(0).tap();
await waitForExpect(async () => {
await expectBitmapsToBeEqual("ModalKeyboardOpened");
});
});

it("should close modal again", async () => {
await waitAndTap("close_button");
});

it("should restore initial state before Modal show", async () => {
await waitForExpect(async () => {
await expectBitmapsToBeEqual("ModalBeforeOpening");
});
});
});
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added e2e/kit/assets/ios/iPhone 14/ModalJustOpened.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.

0 comments on commit bb5e9e1

Please sign in to comment.