Skip to content

Commit

Permalink
fix(Android): separate transitions of sheet and dimming view (#2542)
Browse files Browse the repository at this point in the history
## Description

This PR allows the form sheet on Android to be dismissed using
`slide-down` animation. Currently the sheet fades-out together with the
dimming view due to using regular [Tween
animations](https://developer.android.com/guide/topics/resources/animation-resource#Tween),
which apply animation whole view hierarchy from the fragment's root view
down - this does not allow for separate animations for the sheet and
dimming view.

`Transition API` was considered as an implementation measure, however
due to numerous encountered problems, most prominent example being
"disappearing shadows during pop transitions", I rejected it.

The implementation settled on using custom value / object evaluators.
This approach comes with its own series of issues, with the most
prominent one:

* on old architecture the fragment transaction execution (transition)
starts before initial frame for content wrapper is received - I decided
to defer the transaction to the moment of receiving the layout.


> [!caution]
This PR changes current default animation for formsheets on Android.
This is potentially a breaking change. I haven't decided yet whether to
treat is as a fix or hide this new animation behind some prop.

WIP recordings of the changes.


## Changes


* Form Sheet screens use now custom animators instead of tween animation
(`setCustomTransition`) defined in `ScreenStack.onUpdate`.
* Removed `DimmingFragment` as there is no longer "nested fragment
strcuture". Dimming view is now attached directly into hierarchy (under
coordinator layout) w/o hosting it in separate fragment.
* Refactored native dismiss code

> [!note]
During testing I've noticed that for some reason the form sheet screens
do not receive `onWill(dis)appear` events on navigator. This is
something to investigate, however it seems that this is not a
regression.


## Test code and steps to reproduce

`TestFormSheet` / `TestAndroidTransitions` examples

## Checklist

- [x] Included code example that can be used to test this change
- [x] Ensured that CI passes
  • Loading branch information
kkafar authored Jan 31, 2025
1 parent a8ae962 commit 89f8ad3
Show file tree
Hide file tree
Showing 14 changed files with 595 additions and 534 deletions.
73 changes: 70 additions & 3 deletions android/src/main/java/com/swmansion/rnscreens/Screen.kt
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import android.content.pm.ActivityInfo
import android.graphics.Paint
import android.os.Parcelable
import android.util.SparseArray
import android.view.MotionEvent
import android.view.View
import android.view.ViewGroup
import android.view.WindowManager
Expand All @@ -17,20 +18,24 @@ import androidx.swiperefreshlayout.widget.SwipeRefreshLayout
import com.facebook.react.bridge.GuardedRunnable
import com.facebook.react.bridge.ReactContext
import com.facebook.react.uimanager.PixelUtil
import com.facebook.react.uimanager.ThemedReactContext
import com.facebook.react.uimanager.UIManagerHelper
import com.facebook.react.uimanager.UIManagerModule
import com.facebook.react.uimanager.events.EventDispatcher
import com.google.android.material.bottomsheet.BottomSheetBehavior
import com.google.android.material.shape.CornerFamily
import com.google.android.material.shape.MaterialShapeDrawable
import com.google.android.material.shape.ShapeAppearanceModel
import com.swmansion.rnscreens.bottomsheet.isSheetFitToContents
import com.swmansion.rnscreens.bottomsheet.usesFormSheetPresentation
import com.swmansion.rnscreens.events.HeaderHeightChangeEvent
import com.swmansion.rnscreens.events.SheetDetentChangedEvent
import com.swmansion.rnscreens.ext.parentAsViewGroup
import java.lang.ref.WeakReference

@SuppressLint("ViewConstructor") // Only we construct this view, it is never inflated.
class Screen(
val reactContext: ReactContext,
val reactContext: ThemedReactContext,
) : FabricEnabledViewGroup(reactContext),
ScreenContentWrapper.OnLayoutCallback {
val fragment: Fragment?
Expand Down Expand Up @@ -81,6 +86,13 @@ class Screen(
var sheetClosesOnTouchOutside = true
var sheetElevation: Float = 24F

/**
* When using form sheet presentation we want to delay enter transition **on Paper** in order
* to wait for initial layout from React, otherwise the animator-based animation will look
* glitchy. *This is not needed on Fabric*.
*/
var shouldTriggerPostponedTransitionAfterLayout = false

var footer: ScreenFooter? = null
set(value) {
if (value == null && field != null) {
Expand Down Expand Up @@ -110,7 +122,7 @@ class Screen(
* `fitToContents` for formSheets, as this is first entry point where we can acquire
* height of our content.
*/
override fun onLayoutCallback(
override fun onContentWrapperLayout(
changed: Boolean,
left: Int,
top: Int,
Expand All @@ -119,12 +131,23 @@ class Screen(
) {
val height = bottom - top

if (sheetDetents.count() == 1 && sheetDetents.first() == SHEET_FIT_TO_CONTENTS) {
if (isSheetFitToContents()) {
sheetBehavior?.let {
if (it.maxHeight != height) {
it.maxHeight = height
}
}

if (!BuildConfig.IS_NEW_ARCHITECTURE_ENABLED) {
// On old architecture we delay enter transition in order to wait for initial frame.
shouldTriggerPostponedTransitionAfterLayout = true
val parent = parentAsViewGroup()
if (parent != null && !parent.isInLayout) {
// There are reported cases (irreproducible) when Screen is not laid out after
// maxHeight is set on behaviour.
parent.requestLayout()
}
}
}
}

Expand Down Expand Up @@ -162,6 +185,17 @@ class Screen(

footer?.onParentLayout(changed, l, t, r, b, container!!.height)
notifyHeaderHeightChange(t)

if (!BuildConfig.IS_NEW_ARCHITECTURE_ENABLED) {
maybeTriggerPostponedTransition()
}
}
}

private fun maybeTriggerPostponedTransition() {
if (shouldTriggerPostponedTransitionAfterLayout) {
shouldTriggerPostponedTransitionAfterLayout = false
fragment?.startPostponedEnterTransition()
}
}

Expand Down Expand Up @@ -377,6 +411,28 @@ class Screen(
}
}

fun endRemovalTransition() {
if (!isBeingRemoved) {
return
}
isBeingRemoved = false
endTransitionRecursive(this)
}

private fun endTransitionRecursive(parent: ViewGroup) {
parent.children.forEach { childView ->
parent.endViewTransition(childView)

if (childView is ScreenStackHeaderConfig) {
endTransitionRecursive(childView.toolbar)
}

if (childView is ViewGroup) {
endTransitionRecursive(childView)
}
}
}

private fun startTransitionRecursive(parent: ViewGroup?) {
parent?.let {
for (i in 0 until it.childCount) {
Expand Down Expand Up @@ -407,6 +463,17 @@ class Screen(
}
}

// We do not want to perform any action, therefore do not need to override the associated method.
@SuppressLint("ClickableViewAccessibility")
override fun onTouchEvent(event: MotionEvent?): Boolean =
if (usesFormSheetPresentation()) {
// If we're a form sheet we want to consume the gestures to prevent
// DimmingView's callback from triggering when clicking on the sheet itself.
true
} else {
super.onTouchEvent(event)
}

private fun notifyHeaderHeightChange(headerHeight: Int) {
val screenContext = context as ReactContext
val surfaceId = UIManagerHelper.getSurfaceId(screenContext)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ class ScreenContentWrapper(
internal var delegate: OnLayoutCallback? = null

interface OnLayoutCallback {
fun onLayoutCallback(
fun onContentWrapperLayout(
changed: Boolean,
left: Int,
top: Int,
Expand All @@ -33,6 +33,6 @@ class ScreenContentWrapper(
right: Int,
bottom: Int,
) {
delegate?.onLayoutCallback(changed, left, top, right, bottom)
delegate?.onContentWrapperLayout(changed, left, top, right, bottom)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import com.facebook.react.bridge.UiThreadUtil
import com.facebook.react.uimanager.UIManagerHelper
import com.facebook.react.uimanager.events.Event
import com.facebook.react.uimanager.events.EventDispatcher
import com.swmansion.rnscreens.bottomsheet.DimmingFragment
import com.swmansion.rnscreens.events.HeaderBackButtonClickedEvent
import com.swmansion.rnscreens.events.ScreenAppearEvent
import com.swmansion.rnscreens.events.ScreenDisappearEvent
Expand Down Expand Up @@ -290,12 +289,7 @@ open class ScreenFragment :
// since we subscribe to parent's animation start/end and dispatch events in child from there
// check for `isTransitioning` should be enough since the child's animation should take only
// 20ms due to always being `StackAnimation.NONE` when nested stack being pushed
val parent =
if (parentFragment is DimmingFragment) {
parentFragment?.parentFragment
} else {
parentFragment
}
val parent = parentFragment
if (parent == null || (parent is ScreenFragment && !parent.isTransitioning)) {
// onViewAnimationStart/End is triggered from View#onAnimationStart/End method of the fragment's root
// view. We override an appropriate method of the StackFragment's
Expand Down
12 changes: 8 additions & 4 deletions android/src/main/java/com/swmansion/rnscreens/ScreenStack.kt
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,10 @@ import android.view.View
import com.facebook.react.bridge.ReactContext
import com.facebook.react.uimanager.UIManagerHelper
import com.swmansion.rnscreens.Screen.StackAnimation
import com.swmansion.rnscreens.bottomsheet.DimmingFragment
import com.swmansion.rnscreens.bottomsheet.isSheetFitToContents
import com.swmansion.rnscreens.events.StackFinishTransitioningEvent
import java.util.Collections
import kotlin.collections.ArrayList
import kotlin.collections.HashSet

class ScreenStack(
context: Context?,
Expand Down Expand Up @@ -50,7 +49,7 @@ class ScreenStack(

override fun adapt(screen: Screen): ScreenStackFragmentWrapper =
when (screen.stackPresentation) {
Screen.StackPresentation.FORM_SHEET -> DimmingFragment(ScreenStackFragment(screen))
Screen.StackPresentation.FORM_SHEET -> ScreenStackFragment(screen)
else -> ScreenStackFragment(screen)
}

Expand Down Expand Up @@ -242,7 +241,6 @@ class ScreenStack(
}
}
}

// animation logic end
goingForward = shouldUseOpenAnimation

Expand Down Expand Up @@ -302,6 +300,12 @@ class ScreenStack(
}
}
} else if (newTop != null && !newTop.fragment.isAdded) {
if (!BuildConfig.IS_NEW_ARCHITECTURE_ENABLED && newTop.screen.isSheetFitToContents()) {
// On old architecture the content wrapper might not have received its frame yet,
// which is required to determine height of the sheet after animation. Therefore
// we delay the transition and trigger it after views receive the layout.
newTop.fragment.postponeEnterTransition()
}
it.add(id, newTop.fragment)
}
topScreenWrapper = newTop as? ScreenStackFragmentWrapper
Expand Down
Loading

0 comments on commit 89f8ad3

Please sign in to comment.