Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 36 additions & 21 deletions android/src/main/java/com/swmansion/rnscreens/ScreenStack.kt
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import com.swmansion.rnscreens.utils.setTweenAnimations
import java.util.Collections
import kotlin.collections.ArrayList
import kotlin.math.max
import kotlin.math.min

internal interface ChildDrawingOrderStrategy {
/**
Expand Down Expand Up @@ -68,8 +69,12 @@ internal class ReverseOrderInRange(
return
}

var startRange = range.start
var endRange = range.endInclusive
if (drawingOperations.isEmpty()) {
return
}

var startRange = min(range.start, drawingOperations.lastIndex)
var endRange = min(range.endInclusive, drawingOperations.lastIndex)
Comment on lines +76 to +77
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like handling this kind of API misuse here. This shouldn't be job of the ChildDrawingOrderStrategy to determine what is the range, it should get the range as argument and assume it makes sense.


while (startRange < endRange) {
Collections.swap(drawingOperations, startRange, endRange)
Expand All @@ -87,8 +92,7 @@ class ScreenStack(
private val drawingOpPool: MutableList<DrawingOp> = ArrayList()
private var drawingOps: MutableList<DrawingOp> = ArrayList()
private var topScreenWrapper: ScreenStackFragmentWrapper? = null
private var removalTransitionStarted = false
private var previousChildrenCount = 0
private var removalTransitionChildrenCount = 0

private var childDrawingOrderStrategy: ChildDrawingOrderStrategy? = null

Expand All @@ -97,7 +101,7 @@ class ScreenStack(
/**
* Marks given fragment as to-be-dismissed and performs updates on container
*
* @param fragmentWrapper to-be-dismissed wrapper
* @param screenFragment to-be-dismissed wrapper
*/
fun dismiss(screenFragment: ScreenStackFragmentWrapper) {
dismissedWrappers.add(screenFragment)
Expand All @@ -124,20 +128,22 @@ class ScreenStack(
override fun startViewTransition(view: View) {
super.startViewTransition(view)
childDrawingOrderStrategy?.enable()
removalTransitionStarted = true
removalTransitionChildrenCount += 1
// println("+1, now: " + removalTransitionChildrenCount)
}

override fun endViewTransition(view: View) {
super.endViewTransition(view)
childDrawingOrderStrategy?.disable()
if (removalTransitionStarted) {
removalTransitionStarted = false
removalTransitionChildrenCount = max(0, removalTransitionChildrenCount - 1)
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without this, the count would go negative - in the formSheet example (TestFormSheet -> Open Sheet -> Open Second) it prints:

+1, now: 1
+1, now: 2
-1, now: 1
-1, now: 0
-1, now: 0
-1, now: 0
-1, now: 0

// println("-1 , now: " + removalTransitionChildrenCount)
if (removalTransitionChildrenCount == 0) {
childDrawingOrderStrategy?.disable()
dispatchOnFinishTransitioning()
}
}

fun onViewAppearTransitionEnd() {
if (!removalTransitionStarted) {
if (removalTransitionChildrenCount == 0) {
dispatchOnFinishTransitioning()
}
}
Expand Down Expand Up @@ -229,16 +235,31 @@ class ScreenStack(
needsDrawReordering(newTop, stackAnimation) &&
visibleBottom == null
) {
// When using an open animation in which two screens overlap (eg. fade_from_bottom or
// slide_from_bottom), we want to draw the previous screen under the new one,
// which is apparently not the default option. Android always draws the disappearing view
// When using an open animation in which screens overlap (eg. fade_from_bottom or
// slide_from_bottom), we want to draw the previous screens under the new one,
// which is apparently not the default option. Android always draws the disappearing views
// on top of the appearing one. We then reverse the order of the views so the new screen
// appears on top of the previous one. You can read more about in the comment
// appears on top of the previous ones. You can read more about in the comment
// for the code we use to change that behavior:
// https://github.com/airbnb/native-navigation/blob/9cf50bf9b751b40778f473f3b19fcfe2c4d40599/lib/android/src/main/java/com/airbnb/android/react/navigation/ScreenCoordinatorLayout.java#L18
// Note: This should not be set in case there is only a single screen in stack or animation `none` is used.
// Atm needsDrawReordering implementation guards that assuming that first screen on stack uses `NONE` animation.
childDrawingOrderStrategy = SwapLastTwo()

val disappearingCount =
screenWrappers
.asReversed()
.asSequence()
.filter {
!dismissedWrappers.contains(it) &&
it.screen.activityState !== Screen.ActivityState.INACTIVE
}.drop(1)
.takeWhile { it.screen.isTransparent() }
.count() + 1

childDrawingOrderStrategy =
ReverseOrderInRange(
screenWrappers.lastIndex - disappearingCount..screenWrappers.lastIndex,
)
} else if (newTop != null &&
newTopAlreadyInStack &&
topScreenWrapper?.screen?.isTransparent() == true &&
Expand Down Expand Up @@ -354,12 +375,6 @@ class ScreenStack(
override fun dispatchDraw(canvas: Canvas) {
super.dispatchDraw(canvas)

// check the view removal is completed (by comparing the previous children count)
if (drawingOps.size < previousChildrenCount) {
childDrawingOrderStrategy = null
}
previousChildrenCount = drawingOps.size

Comment on lines -357 to -362
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think with disable() method on childDrawingOrderStrategy this probably isn't needed anymore but I am not 100% sure.

childDrawingOrderStrategy?.apply(drawingOps)

drawAndRelease()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -326,6 +326,8 @@ class ScreenStackFragment :
return null
}

return null

val animatorSet = AnimatorSet()
val dimmingDelegate = requireDimmingDelegate()

Expand Down Expand Up @@ -723,9 +725,9 @@ class ScreenStackFragment :
return dimmingDelegate!!
}

private class ScreensCoordinatorLayout(
internal class ScreensCoordinatorLayout(
context: Context,
private val fragment: ScreenStackFragment,
internal val fragment: ScreenStackFragment,
private val pointerEventsImpl: ReactPointerEventsView,
// ) : CoordinatorLayout(context), ReactCompoundViewGroup, ReactHitSlopView {
) : CoordinatorLayout(context),
Expand Down
3 changes: 3 additions & 0 deletions android/src/main/java/com/swmansion/rnscreens/ext/ViewExt.kt
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,14 @@ package com.swmansion.rnscreens.ext
import android.graphics.drawable.ColorDrawable
import android.view.View
import android.view.ViewGroup
import com.swmansion.rnscreens.ScreenStackFragment

internal fun View.parentAsView() = this.parent as? View

internal fun View.parentAsViewGroup() = this.parent as? ViewGroup

internal fun View.asScreensCoordinatorLayout() = this as? ScreenStackFragment.ScreensCoordinatorLayout

internal fun View.recycle(): View {
// screen fragments reuse view instances instead of creating new ones. In order to reuse a given
// view it needs to be detached from the view hierarchy to allow the fragment to attach it back.
Expand Down