Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 61b57331 authored by Alejandro Nijamkin's avatar Alejandro Nijamkin
Browse files

[flexiglass] Fixes back navigation. 

The QS scene adds a BackHandler that conditions its enabled state on
whether QS is in edit mode (isCustomizing). This seemed to not have
worked with Flexiglass and the back callbacks were being invoked on the
BackHandler in SceneTransitionLayoutImpl even though the child
composable of the scene was changing its own BackHandler's enabled state
after that parent SceneTransitionLayoutImpl BackHandler was added.

The assumption is that because the BackHandler in
SceneTransitionLayoutImpl was being added conditionally, only when the
current scene had a "back" user action result, the order of addition of
BackHandlers was incorrect with the parent composable going through
recomposition in response to the new destinationScenes map from the
current scene.

The fix is to have the SceneTransitionLayoutImpl add its BackHandler
unconditionally but to set its enabled state based on whether the
current scene has a "back" user aciton result. This preserves the order
of addition of BackHandlers.

Fix: 332749288
Test: locally replaced the emptyMap() in QuickSettingsSceneViewModel
with a back action that goes back to the QS scene; just to test the
following two things (but also tested them without that change, just in
case)
Test: manually verified that unlock -> shade -> QS -> edit -> back, correctly returns to QS in non-edit mode
Test: manually verified that unlock -> shade -> QS -> back, correctly
returns to the shade scene
Flag: ACONFIG com.android.systemui.scene_container DEVELOPMENT

Change-Id: I3ad46557fbe634d89de46249bf775cf315d15f86
parent 7bfffc4f

packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/SceneTransitionLayoutImpl.kt

+10 −9
Original line number Diff line number Diff line
@@ -204,15 +204,16 @@ internal class SceneTransitionLayoutImpl( 
                    }



                // Handle back events.

                // TODO(b/290184746): Make sure that this works with SystemUI once we use

                // SceneTransitionLayout in Flexiglass.

                scene(state.transitionState.currentScene).userActions[Back]?.let { result ->

                val targetSceneForBackOrNull =

                    scene(state.transitionState.currentScene).userActions[Back]?.toScene

                BackHandler(

                    enabled = targetSceneForBackOrNull != null,

                ) {

                    targetSceneForBackOrNull?.let { targetSceneForBack ->

                        // TODO(b/290184746): Handle predictive back and use result.distance if

                        // specified.

                    BackHandler {

                        val targetScene = result.toScene

                        if (state.canChangeScene(targetScene)) {

                            with(state) { coroutineScope.onChangeScene(targetScene) }

                        if (state.canChangeScene(targetSceneForBack)) {

                            with(state) { coroutineScope.onChangeScene(targetSceneForBack) }

                        }

                    }

                }

packages/SystemUI/src/com/android/systemui/qs/ui/viewmodel/QuickSettingsSceneViewModel.kt

+0 −2
Original line number Diff line number Diff line
@@ -50,8 +50,6 @@ constructor( 
    val destinationScenes =

        qsSceneAdapter.isCustomizing.flatMapLatest { customizing ->

            if (customizing) {

                // TODO(b/332749288) Empty map so there are no back handlers and back can close

                // customizer

                flowOf(emptyMap())

                // TODO(b/330200163) Add an Up from Bottom to be able to collapse the shade

                // while customizing

packages/SystemUI/src/com/android/systemui/shade/GlanceableHubContainerController.kt

+35 −7
Original line number Diff line number Diff line
@@ -23,7 +23,13 @@ import android.view.GestureDetector 
import android.view.MotionEvent

import android.view.View

import android.view.ViewGroup

import androidx.activity.OnBackPressedDispatcher

import androidx.activity.OnBackPressedDispatcherOwner

import androidx.activity.setViewTreeOnBackPressedDispatcherOwner

import androidx.compose.ui.platform.ComposeView

import androidx.lifecycle.Lifecycle

import androidx.lifecycle.lifecycleScope

import androidx.lifecycle.repeatOnLifecycle

import com.android.compose.theme.PlatformTheme

import com.android.internal.annotations.VisibleForTesting

import com.android.systemui.communal.dagger.Communal
@@ -33,6 +39,7 @@ import com.android.systemui.communal.ui.viewmodel.CommunalViewModel 
import com.android.systemui.keyguard.domain.interactor.KeyguardInteractor

import com.android.systemui.keyguard.domain.interactor.KeyguardTransitionInteractor

import com.android.systemui.keyguard.shared.model.KeyguardState

import com.android.systemui.lifecycle.repeatWhenAttached

import com.android.systemui.res.R

import com.android.systemui.scene.shared.model.SceneDataSourceDelegator

import com.android.systemui.shade.domain.interactor.ShadeInteractor
@@ -40,6 +47,7 @@ import com.android.systemui.statusbar.phone.SystemUIDialogFactory 
import com.android.systemui.util.kotlin.collectFlow

import javax.inject.Inject

import kotlinx.coroutines.flow.Flow

import kotlinx.coroutines.launch



/**

 * Controller that's responsible for the glanceable hub container view and its touch handling.
@@ -139,6 +147,23 @@ constructor( 
    ): View {

        return initView(

            ComposeView(context).apply {

                repeatWhenAttached {

                    lifecycleScope.launch {

                        repeatOnLifecycle(Lifecycle.State.CREATED) {

                            setViewTreeOnBackPressedDispatcherOwner(

                                object : OnBackPressedDispatcherOwner {

                                    override val onBackPressedDispatcher =

                                        OnBackPressedDispatcher().apply {

                                            setOnBackInvokedDispatcher(

                                                viewRootImpl.onBackInvokedDispatcher

                                            )

                                        }



                                    override val lifecycle: Lifecycle =

                                        this@repeatWhenAttached.lifecycle

                                }

                            )



                            setContent {

                                PlatformTheme {

                                    CommunalContainer(
@@ -149,6 +174,9 @@ constructor( 
                                }

                            }

                        }

                    }

                }

            }

        )

    }