Skip to content

Commit 89f8ad3

Browse files
authored
fix(Android): separate transitions of sheet and dimming view (#2542)
## 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
1 parent a8ae962 commit 89f8ad3

14 files changed

+595
-534
lines changed

android/src/main/java/com/swmansion/rnscreens/Screen.kt

Lines changed: 70 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import android.content.pm.ActivityInfo
55
import android.graphics.Paint
66
import android.os.Parcelable
77
import android.util.SparseArray
8+
import android.view.MotionEvent
89
import android.view.View
910
import android.view.ViewGroup
1011
import android.view.WindowManager
@@ -17,20 +18,24 @@ import androidx.swiperefreshlayout.widget.SwipeRefreshLayout
1718
import com.facebook.react.bridge.GuardedRunnable
1819
import com.facebook.react.bridge.ReactContext
1920
import com.facebook.react.uimanager.PixelUtil
21+
import com.facebook.react.uimanager.ThemedReactContext
2022
import com.facebook.react.uimanager.UIManagerHelper
2123
import com.facebook.react.uimanager.UIManagerModule
2224
import com.facebook.react.uimanager.events.EventDispatcher
2325
import com.google.android.material.bottomsheet.BottomSheetBehavior
2426
import com.google.android.material.shape.CornerFamily
2527
import com.google.android.material.shape.MaterialShapeDrawable
2628
import com.google.android.material.shape.ShapeAppearanceModel
29+
import com.swmansion.rnscreens.bottomsheet.isSheetFitToContents
30+
import com.swmansion.rnscreens.bottomsheet.usesFormSheetPresentation
2731
import com.swmansion.rnscreens.events.HeaderHeightChangeEvent
2832
import com.swmansion.rnscreens.events.SheetDetentChangedEvent
33+
import com.swmansion.rnscreens.ext.parentAsViewGroup
2934
import java.lang.ref.WeakReference
3035

3136
@SuppressLint("ViewConstructor") // Only we construct this view, it is never inflated.
3237
class Screen(
33-
val reactContext: ReactContext,
38+
val reactContext: ThemedReactContext,
3439
) : FabricEnabledViewGroup(reactContext),
3540
ScreenContentWrapper.OnLayoutCallback {
3641
val fragment: Fragment?
@@ -81,6 +86,13 @@ class Screen(
8186
var sheetClosesOnTouchOutside = true
8287
var sheetElevation: Float = 24F
8388

89+
/**
90+
* When using form sheet presentation we want to delay enter transition **on Paper** in order
91+
* to wait for initial layout from React, otherwise the animator-based animation will look
92+
* glitchy. *This is not needed on Fabric*.
93+
*/
94+
var shouldTriggerPostponedTransitionAfterLayout = false
95+
8496
var footer: ScreenFooter? = null
8597
set(value) {
8698
if (value == null && field != null) {
@@ -110,7 +122,7 @@ class Screen(
110122
* `fitToContents` for formSheets, as this is first entry point where we can acquire
111123
* height of our content.
112124
*/
113-
override fun onLayoutCallback(
125+
override fun onContentWrapperLayout(
114126
changed: Boolean,
115127
left: Int,
116128
top: Int,
@@ -119,12 +131,23 @@ class Screen(
119131
) {
120132
val height = bottom - top
121133

122-
if (sheetDetents.count() == 1 && sheetDetents.first() == SHEET_FIT_TO_CONTENTS) {
134+
if (isSheetFitToContents()) {
123135
sheetBehavior?.let {
124136
if (it.maxHeight != height) {
125137
it.maxHeight = height
126138
}
127139
}
140+
141+
if (!BuildConfig.IS_NEW_ARCHITECTURE_ENABLED) {
142+
// On old architecture we delay enter transition in order to wait for initial frame.
143+
shouldTriggerPostponedTransitionAfterLayout = true
144+
val parent = parentAsViewGroup()
145+
if (parent != null && !parent.isInLayout) {
146+
// There are reported cases (irreproducible) when Screen is not laid out after
147+
// maxHeight is set on behaviour.
148+
parent.requestLayout()
149+
}
150+
}
128151
}
129152
}
130153

@@ -162,6 +185,17 @@ class Screen(
162185

163186
footer?.onParentLayout(changed, l, t, r, b, container!!.height)
164187
notifyHeaderHeightChange(t)
188+
189+
if (!BuildConfig.IS_NEW_ARCHITECTURE_ENABLED) {
190+
maybeTriggerPostponedTransition()
191+
}
192+
}
193+
}
194+
195+
private fun maybeTriggerPostponedTransition() {
196+
if (shouldTriggerPostponedTransitionAfterLayout) {
197+
shouldTriggerPostponedTransitionAfterLayout = false
198+
fragment?.startPostponedEnterTransition()
165199
}
166200
}
167201

@@ -377,6 +411,28 @@ class Screen(
377411
}
378412
}
379413

414+
fun endRemovalTransition() {
415+
if (!isBeingRemoved) {
416+
return
417+
}
418+
isBeingRemoved = false
419+
endTransitionRecursive(this)
420+
}
421+
422+
private fun endTransitionRecursive(parent: ViewGroup) {
423+
parent.children.forEach { childView ->
424+
parent.endViewTransition(childView)
425+
426+
if (childView is ScreenStackHeaderConfig) {
427+
endTransitionRecursive(childView.toolbar)
428+
}
429+
430+
if (childView is ViewGroup) {
431+
endTransitionRecursive(childView)
432+
}
433+
}
434+
}
435+
380436
private fun startTransitionRecursive(parent: ViewGroup?) {
381437
parent?.let {
382438
for (i in 0 until it.childCount) {
@@ -407,6 +463,17 @@ class Screen(
407463
}
408464
}
409465

466+
// We do not want to perform any action, therefore do not need to override the associated method.
467+
@SuppressLint("ClickableViewAccessibility")
468+
override fun onTouchEvent(event: MotionEvent?): Boolean =
469+
if (usesFormSheetPresentation()) {
470+
// If we're a form sheet we want to consume the gestures to prevent
471+
// DimmingView's callback from triggering when clicking on the sheet itself.
472+
true
473+
} else {
474+
super.onTouchEvent(event)
475+
}
476+
410477
private fun notifyHeaderHeightChange(headerHeight: Int) {
411478
val screenContext = context as ReactContext
412479
val surfaceId = UIManagerHelper.getSurfaceId(screenContext)

android/src/main/java/com/swmansion/rnscreens/ScreenContentWrapper.kt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ class ScreenContentWrapper(
1717
internal var delegate: OnLayoutCallback? = null
1818

1919
interface OnLayoutCallback {
20-
fun onLayoutCallback(
20+
fun onContentWrapperLayout(
2121
changed: Boolean,
2222
left: Int,
2323
top: Int,
@@ -33,6 +33,6 @@ class ScreenContentWrapper(
3333
right: Int,
3434
bottom: Int,
3535
) {
36-
delegate?.onLayoutCallback(changed, left, top, right, bottom)
36+
delegate?.onContentWrapperLayout(changed, left, top, right, bottom)
3737
}
3838
}

android/src/main/java/com/swmansion/rnscreens/ScreenFragment.kt

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ import com.facebook.react.bridge.UiThreadUtil
1515
import com.facebook.react.uimanager.UIManagerHelper
1616
import com.facebook.react.uimanager.events.Event
1717
import com.facebook.react.uimanager.events.EventDispatcher
18-
import com.swmansion.rnscreens.bottomsheet.DimmingFragment
1918
import com.swmansion.rnscreens.events.HeaderBackButtonClickedEvent
2019
import com.swmansion.rnscreens.events.ScreenAppearEvent
2120
import com.swmansion.rnscreens.events.ScreenDisappearEvent
@@ -290,12 +289,7 @@ open class ScreenFragment :
290289
// since we subscribe to parent's animation start/end and dispatch events in child from there
291290
// check for `isTransitioning` should be enough since the child's animation should take only
292291
// 20ms due to always being `StackAnimation.NONE` when nested stack being pushed
293-
val parent =
294-
if (parentFragment is DimmingFragment) {
295-
parentFragment?.parentFragment
296-
} else {
297-
parentFragment
298-
}
292+
val parent = parentFragment
299293
if (parent == null || (parent is ScreenFragment && !parent.isTransitioning)) {
300294
// onViewAnimationStart/End is triggered from View#onAnimationStart/End method of the fragment's root
301295
// view. We override an appropriate method of the StackFragment's

android/src/main/java/com/swmansion/rnscreens/ScreenStack.kt

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,10 @@ import android.view.View
77
import com.facebook.react.bridge.ReactContext
88
import com.facebook.react.uimanager.UIManagerHelper
99
import com.swmansion.rnscreens.Screen.StackAnimation
10-
import com.swmansion.rnscreens.bottomsheet.DimmingFragment
10+
import com.swmansion.rnscreens.bottomsheet.isSheetFitToContents
1111
import com.swmansion.rnscreens.events.StackFinishTransitioningEvent
1212
import java.util.Collections
1313
import kotlin.collections.ArrayList
14-
import kotlin.collections.HashSet
1514

1615
class ScreenStack(
1716
context: Context?,
@@ -50,7 +49,7 @@ class ScreenStack(
5049

5150
override fun adapt(screen: Screen): ScreenStackFragmentWrapper =
5251
when (screen.stackPresentation) {
53-
Screen.StackPresentation.FORM_SHEET -> DimmingFragment(ScreenStackFragment(screen))
52+
Screen.StackPresentation.FORM_SHEET -> ScreenStackFragment(screen)
5453
else -> ScreenStackFragment(screen)
5554
}
5655

@@ -242,7 +241,6 @@ class ScreenStack(
242241
}
243242
}
244243
}
245-
246244
// animation logic end
247245
goingForward = shouldUseOpenAnimation
248246

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

0 commit comments

Comments
 (0)