Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(Android): fix draw ordering in transparent modal & stack nested in tabs interaction #2647

Merged
merged 3 commits into from
Jan 24, 2025

Conversation

kkafar
Copy link
Member

@kkafar kkafar commented Jan 24, 2025

Description

Fixes #2167

The exact error mechanism is really convoluted. The gist of it however, and the issue cause lies in the fact
that our drawing / container updating logic worked under implicit (and unspoken of) assumption that draw reordering would not be
applied for the transaction attaching very first screen in the stack. Everything worked correctly until #2019 caused needsDrawReordeing
to return true always when Build.VERSION.SDK_INT > 33 - and that means pretty much always in new apps. Previously it returned false,
in particular for the very first screen on stack because no one really sets stackAnimation for the very first screen, since it will have no animation anyway
(and we might enforce this somewhere in JS code also, I'm not sure now).

This PR restores returning false there for first screen on the stack & for any screen that uses animation: 'none'.

Summary of the error mechanism

Consider following case:

function App() {
    return (
        <Tabs>
            <Screen A>
                <Stack>
                    <Screen SA />
                    <Screen TM />
                </Stack>
            </Screen A>
            <Screen B />
        </Tabs>
    );
}

Initially Screen SA is rendered. Basically when [isDetachingCurrentScreen] was set for the very first screen (directly because return value of needsDrawReordeing) and then
we navigated to other tab Screen B - we cause whole stack Stack to be dropped & detached from window. Native callback onDetachedFromWindow gets called in ScreenContainer,
we detach every fragment and subview (to prevent whole different class of bugs) causing removeView callbacks in ScreenStack, leading to reverseLastTwoChildren flag being set to true. When we then change tab back to Screen SA in Stack
the drawing works as normal, because we have only one child. On navigation to Screen TM (transparent modal) value of the reverseLastTwoChildren flag causes the views to being drawn
in wrong order - transparent modal first and Screen SA second. In case of not transparent presentation there is no issue, because Screen SA would get detached.

Changes

Added param to needsDrawReordeing method informing of actual stack animation in use (in case of first screen we always set it to none).
When there is no animation for the disappearing screen - there is no need to change the draw ordering. Added appropriate code comment for the future.

Test code and steps to reproduce

Test2167

Checklist

  • Included code example that can be used to test this change
  • Ensured that CI passes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant