fix(Android): drawing order for multiple disappearing screens#2795
fix(Android): drawing order for multiple disappearing screens#2795
Conversation
| 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) | ||
| // println("-1, now: " + removalTransitionChildrenCount) | ||
| if (removalTransitionChildrenCount == 0) { | ||
| childDrawingOrderStrategy?.disable() | ||
| dispatchOnFinishTransitioning() | ||
| } | ||
| } | ||
|
|
||
| fun onViewAppearTransitionEnd() { | ||
| if (!removalTransitionStarted) { | ||
| if (removalTransitionChildrenCount == 0) { | ||
| dispatchOnFinishTransitioning() | ||
| } | ||
| } |
There was a problem hiding this comment.
I'm not sure whether this properly mirrors logic from before.
| childDrawingOrderStrategy?.disable() | ||
| if (removalTransitionStarted) { | ||
| removalTransitionStarted = false | ||
| removalTransitionChildrenCount = max(0, removalTransitionChildrenCount - 1) |
There was a problem hiding this comment.
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
| 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) |
There was a problem hiding this comment.
I'm not sure what reference should I use here to count disappearing screens - I mirrored logic from notDismissedWrappers but I guess you can also use the stack?
| // check the view removal is completed (by comparing the previous children count) | ||
| if (drawingOps.size < previousChildrenCount) { | ||
| childDrawingOrderStrategy = null | ||
| } | ||
| previousChildrenCount = drawingOps.size | ||
|
|
There was a problem hiding this comment.
I think with disable() method on childDrawingOrderStrategy this probably isn't needed anymore but I am not 100% sure.
kkafar
left a comment
kkafar
left a comment
There was a problem hiding this comment.
In general I think that what we might want to do is:
- Mark the to-be-removed children for removal & make this state
internal, so that the screen stack has access to it.
When screenstack starts view removing transition on some coordinator layout (SCL), we should store reference to this SCL in screen stack.
When screenstack ends view transition we should remove the reference.
Only when the reference store is empty, we should disable child drawing order strategy. In case the number of transitioning children mutates & single child drawing order strategy is not enough, we should find a way to update it or write adaptive drawing order strategy. I'll add a draft of the solution soon.
| var startRange = min(range.start, drawingOperations.lastIndex) | ||
| var endRange = min(range.endInclusive, drawingOperations.lastIndex) |
There was a problem hiding this comment.
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.
|
Continuation & new approach in #2806. |
2 participants
Description
WIP
Changes
WIP
Test code and steps to reproduce
WIP
Checklist