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 Overlap When Moving Between Tabs #1573

Merged
merged 2 commits into from
Aug 29, 2024

Conversation

Songgyubin
Copy link
Contributor

@Songgyubin Songgyubin commented Aug 14, 2024

When you quickly switch between bottom tabs, the screens of the two tabs overlap.
An issue regarding this was also created, and it was resolved by updating the navigation library version.
Issues raised
Screenshot_20240814_221708

The above issue was resolved in navigation 2.8.0-beta04.
jetpack navigation release note
(Currently, NowInAndroid is using jetpack navigation version 2.8.0-alpha06)

The latest version is 2.8.0-beta07, but compileSdk version 35 is required, so the version was updated to beta06.

@Jaehwa-Noh
Copy link
Contributor

I think the resolve is about Predictive back feature.

@Songgyubin
Copy link
Contributor Author

@Jaehwa-Noh
Yes, I read it again and there is a screenshot of the release notes.
I removed the highlighted part because it did not seem to fit perfectly with the issue I raised.

However, we have confirmed that the issues raised have been resolved starting with version beta-04

I think it was probably fixed along with fixing the bug in the Predictive back function. What do you think?

@Jaehwa-Noh
Copy link
Contributor

@Songgyubin
I'm not sure the issue was resolved as I read a release note.

@Songgyubin
Copy link
Contributor Author

@Jaehwa-Noh

Yes, I tried it again, but there is no information about it in the release notes.

However, when I ran it and tested it, it seemed to have been resolved.

So, to get an accurate understanding, I commented on the same post as the issue I was raising.

Can I keep an eye on it and update this PR?

https://issuetracker.google.com/issues/338975163
https://issuetracker.google.com/issues/338975163#comment11

@Songgyubin
Copy link
Contributor Author

Songgyubin commented Aug 16, 2024

@Jaehwa-Noh
As a result of the inquiry it was said that it was the same problem that caused animation problems due to prediction back cancellation, and that the fix was applied in beta04.

스크린샷 2024-08-16 오후 12 12 17

Therefore, the issue of screen overlapping when switching tabs quickly has been corrected starting with version beta04.

@Jaehwa-Noh
Copy link
Contributor

https://android-review.googlesource.com/c/platform/frameworks/support/+/3138255/5/navigation/navigation-compose/src/main/java/androidx/navigation/compose/NavHost.kt#36

val transition = rememberTransition(transitionState, label = "entry")

        if (inPredictiveBack) {
            LaunchedEffect(progress) {
                val previousEntry = currentBackStack[currentBackStack.size - 2]
                transitionState.seekTo(progress, previousEntry)
            }
        } else {
            LaunchedEffect(backStackEntry) {
                // This ensures we don't animate after the back gesture is cancelled and we
                // are already on the current state
                if (transitionState.currentState != backStackEntry) {
                    transitionState.animateTo(backStackEntry)
                } else {
                    // convert from nanoseconds to milliseconds
                    val totalDuration = transition.totalDurationNanos / 1000000
                    // When the predictive back gesture is cancel, we need to manually animate
                    // the SeekableTransitionState from where it left off, to zero and then
                    // snapTo the final position.
                    animate(
                        transitionState.fraction,
                        0f,
                        animationSpec = tween((transitionState.fraction * totalDuration).toInt())
                    ) { value, _ ->
                        [email protected] {
                            if (value > 0) {
                                // Seek the original transition back to the currentState
                                transitionState.seekTo(value)
                            }
                            if (value == 0f) {
                                // Once we animate to the start, we need to snap to the right state.
                                transitionState.snapTo(backStackEntry)
                            }
                        }
                    }
                }
            }
        }

I've checked it, yes, there animation is fixed. This issue seems to be fixed that library version.
By the way, when I changed the navigation library to beta-06, I can't start the interests screen.

Could you possible to show the interests screen on this PR?

@Songgyubin
Copy link
Contributor Author

Songgyubin commented Aug 16, 2024

@Jaehwa-Noh
I also can't open the interests screen.
This is an unexpected situation.

The app crashes.

java.lang.NoSuchMethodError: No static method localLookaheadPositionOf-RE3cj74$default(Landroidx/compose/ui/layout/LookaheadScope;Landroidx/compose/ui/layout/LayoutCoordinates;Landroidx/compose/ui/layout/LayoutCoordinates;JILjava/lang/Object;)J in class Landroidx/compose/ui/layout/LookaheadScope;
or its super classes (declaration of 'androidx.compose.ui.layout.LookaheadScope' appears in /data/app/~~A60uN9g5j3zSh8LW1c2u6A==/com.google.samples.apps.nowinandroid.demo.debug-FbvxGIMsxBPgNmQdZ-yKSA==/base.apk!classes12.dex)

I tried debugging and it looks like the problem is related to AnimatedPane.

If I comment out AnimatedPane it works fine.

InterestsListDetailScreen.kt / 135 line

    ListDetailPaneScaffold(
        value = listDetailNavigator.scaffoldValue,
        directive = listDetailNavigator.scaffoldDirective,
        listPane = {
//            AnimatedPane {
                InterestsRoute(
                    onTopicClick = ::onTopicClickShowDetailPane,
                    highlightSelectedTopic = listDetailNavigator.isDetailPaneVisible(),
                )
//            }
        },
        detailPane = {
            AnimatedPane {
                key(nestedNavKey) {
                    NavHost(
                        navController = nestedNavController,
                        startDestination = nestedNavHostStartDestination,
                        route = DETAIL_PANE_NAVHOST_ROUTE,
                    ) {
                        topicScreen(
                            showBackButton = !listDetailNavigator.isListPaneVisible(),
                            onBackClick = listDetailNavigator::navigateBack,
                            onTopicClick = ::onTopicClickShowDetailPane,
                        )
                        composable(route = TOPIC_ROUTE) {
                            TopicDetailPlaceholder()
                        }
                    }
                }
            }
        },
    )

I think the exact cause needs to be analyzed further.

@Songgyubin
Copy link
Contributor Author

@Jaehwa-Noh
The problem was with androidx.compose.material3.adaptive

It has been resolved starting from androidx.compose.material3.adaptive version beta03, and I raised an issue and received a confirmation that the problem was resolved.
(current version: androidxComposeMaterial3Adaptive = "1.0.0-beta01")

The latest version is 1.1.0-alpha01, but since there are many changes, we don't know what problems will arise, and there is not enough time to test, so we updated it to 1.0.0-rc01 to fix the bug in the PR.

compose-material3-adaptive release note: https://developer.android.com/jetpack/androidx/releases/compose-material3-adaptive

After

Screen_recording_20240824_150848.webm

@alexvanyo alexvanyo self-requested a review August 27, 2024 19:29
@Songgyubin Songgyubin force-pushed the fix-overlap-moving-between-tabs branch from 8d3bc59 to e26d300 Compare August 28, 2024 04:15
@Songgyubin Songgyubin force-pushed the fix-overlap-moving-between-tabs branch from e26d300 to a8a8647 Compare August 29, 2024 00:54
Copy link
Contributor

@alexvanyo alexvanyo left a comment

Choose a reason for hiding this comment

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

The updates look great, thank you for investigating this and making the change!

@alexvanyo alexvanyo merged commit 02ecd0c into android:main Aug 29, 2024
4 checks passed
@Songgyubin Songgyubin deleted the fix-overlap-moving-between-tabs branch August 29, 2024 17:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants