From 32a20b392c0fa87abb4b636352f0afb2f431406d Mon Sep 17 00:00:00 2001 From: Hai Zhang Date: Tue, 11 May 2021 23:20:48 -0700 Subject: [PATCH] Improve SettingsTransitionActivity for standalone usage. - Allow subclass to override when the transition should be enabled, because we may not want to enable it on other form factors, e.g. on wear. - Retrieve the Toolbar instance by overriding setActionBar() which should be called by any activity using Toolbar as its action bar. Even if there's any activity not using Toolbar as action bar, it won't behave well with our transition anyway. - Refactored ActivityOptions Bundle creation logic to simplify the code. I did the refactoring originally for sharing it with a new SettingsTransitionFragment, but then realized doing it in fragment would be very tedious and unreliable. Still the refactoring looks a good simplification to me. - Don't override onOptionsItemSelected() in SettingsTransitionActivity because different form factors might be expecting different behaviors for the home/up indicator on Toolbar, e.g. we might expand/collapse a drawer or do something else instead of finishing the current activity on Tablet. The original implementation that calls onBackPressed() is also technically incorrect because pressing up is a different action. The reason the original implementation did so is probably for enabling transition upon activity finish, but that should actually be done by calling finishAfterTransition(), so I've fixed the CollapsingToolbarBaseActivity to call finishAfterTransition() instead of finish(). This shouldn't bring any regression to existing users of SettingsTransitionActivity as well because currently the only user of it is CollapsingToolbarBaseActivity. Bug: 177638625 Test: manually on R and S platforms Change-Id: I3c5955db3753f00604b625bd21bf7e78f237ec15 --- .../CollapsingToolbarBaseActivity.java | 12 +- .../SettingsTransitionActivity.java | 117 ++++++------------ 2 files changed, 44 insertions(+), 85 deletions(-) diff --git a/packages/SettingsLib/CollapsingToolbarBaseActivity/src/com/android/settingslib/collapsingtoolbar/CollapsingToolbarBaseActivity.java b/packages/SettingsLib/CollapsingToolbarBaseActivity/src/com/android/settingslib/collapsingtoolbar/CollapsingToolbarBaseActivity.java index 83a838f54d28b..e6ca2e0d10c83 100644 --- a/packages/SettingsLib/CollapsingToolbarBaseActivity/src/com/android/settingslib/collapsingtoolbar/CollapsingToolbarBaseActivity.java +++ b/packages/SettingsLib/CollapsingToolbarBaseActivity/src/com/android/settingslib/collapsingtoolbar/CollapsingToolbarBaseActivity.java @@ -35,7 +35,6 @@ import com.google.android.material.resources.TextAppearanceConfig; public class CollapsingToolbarBaseActivity extends SettingsTransitionActivity { private CollapsingToolbarLayout mCollapsingToolbarLayout; - private Toolbar mToolbar; @Override protected void onCreate(@Nullable Bundle savedInstanceState) { @@ -45,8 +44,8 @@ public class CollapsingToolbarBaseActivity extends SettingsTransitionActivity { super.setContentView(R.layout.collapsing_toolbar_base_layout); mCollapsingToolbarLayout = findViewById(R.id.collapsing_toolbar); - mToolbar = findViewById(R.id.action_bar); - setActionBar(mToolbar); + final Toolbar toolbar = findViewById(R.id.action_bar); + setActionBar(toolbar); // Enable title and home button by default final ActionBar actionBar = getActionBar(); @@ -97,16 +96,11 @@ public class CollapsingToolbarBaseActivity extends SettingsTransitionActivity { @Override public boolean onNavigateUp() { if (!super.onNavigateUp()) { - finish(); + finishAfterTransition(); } return true; } - @Override - public Toolbar getToolbar() { - return mToolbar; - } - /** * Returns an instance of collapsing toolbar. */ diff --git a/packages/SettingsLib/CollapsingToolbarBaseActivity/src/com/android/settingslib/collapsingtoolbar/SettingsTransitionActivity.java b/packages/SettingsLib/CollapsingToolbarBaseActivity/src/com/android/settingslib/collapsingtoolbar/SettingsTransitionActivity.java index b4fe7ed1a8fb0..4c45c5e3cb567 100644 --- a/packages/SettingsLib/CollapsingToolbarBaseActivity/src/com/android/settingslib/collapsingtoolbar/SettingsTransitionActivity.java +++ b/packages/SettingsLib/CollapsingToolbarBaseActivity/src/com/android/settingslib/collapsingtoolbar/SettingsTransitionActivity.java @@ -18,14 +18,11 @@ package com.android.settingslib.collapsingtoolbar; import android.app.ActivityOptions; import android.content.Intent; -import android.os.Build; import android.os.Bundle; import android.util.Log; -import android.view.MenuItem; import android.view.Window; import android.widget.Toolbar; -import androidx.annotation.NonNull; import androidx.annotation.Nullable; import androidx.core.os.BuildCompat; import androidx.fragment.app.FragmentActivity; @@ -40,117 +37,85 @@ public abstract class SettingsTransitionActivity extends FragmentActivity { private static final String TAG = "SettingsTransitionActivity"; private static final int DEFAULT_REQUEST = -1; + private Toolbar mToolbar; + @Override protected void onCreate(@Nullable Bundle savedInstanceState) { - if (BuildCompat.isAtLeastS()) { + if (isSettingsTransitionEnabled()) { getWindow().requestFeature(Window.FEATURE_ACTIVITY_TRANSITIONS); SettingsTransitionHelper.applyForwardTransition(this); SettingsTransitionHelper.applyBackwardTransition(this); } + super.onCreate(savedInstanceState); } @Override - public void startActivity(Intent intent) { - if (!BuildCompat.isAtLeastS()) { - super.startActivity(intent); - return; - } - final Toolbar toolbar = getToolbar(); - if (toolbar == null) { - Log.w(TAG, "Toolbar is null. Cannot apply settings transition!"); - super.startActivity(intent); - return; - } - super.startActivity(intent, getActivityOptionsBundle(toolbar)); + public void setActionBar(@Nullable Toolbar toolbar) { + super.setActionBar(toolbar); + mToolbar = toolbar; + } + + @Override + public void startActivity(Intent intent) { + if (!isSettingsTransitionEnabled()) { + super.startActivity(intent); + return; + } + + super.startActivity(intent, createActivityOptionsBundleForTransition(null)); } @Override public void startActivity(Intent intent, @Nullable Bundle options) { - if (!BuildCompat.isAtLeastS()) { + if (!isSettingsTransitionEnabled()) { super.startActivity(intent, options); return; } - final Toolbar toolbar = getToolbar(); - if (toolbar == null) { - Log.w(TAG, "Toolbar is null. Cannot apply settings transition!"); - super.startActivity(intent, options); - return; - } - if (options != null) { - super.startActivity(intent, getMergedBundleForTransition(options)); - return; - } - super.startActivity(intent, getActivityOptionsBundle(toolbar)); + + super.startActivity(intent, createActivityOptionsBundleForTransition(options)); } @Override public void startActivityForResult(Intent intent, int requestCode) { - if (!BuildCompat.isAtLeastS() || requestCode == DEFAULT_REQUEST) { + if (!isSettingsTransitionEnabled() || requestCode == DEFAULT_REQUEST) { super.startActivityForResult(intent, requestCode); return; } - final Toolbar toolbar = getToolbar(); - if (toolbar == null) { - Log.w(TAG, "Toolbar is null. Cannot apply settings transition!"); - super.startActivityForResult(intent, requestCode); - return; - } - super.startActivityForResult(intent, requestCode, getActivityOptionsBundle(toolbar)); + super.startActivityForResult(intent, requestCode, createActivityOptionsBundleForTransition( + null)); } @Override public void startActivityForResult(Intent intent, int requestCode, @Nullable Bundle options) { - if (!BuildCompat.isAtLeastS() || requestCode == DEFAULT_REQUEST) { + if (!isSettingsTransitionEnabled() || requestCode == DEFAULT_REQUEST) { super.startActivityForResult(intent, requestCode, options); return; } - final Toolbar toolbar = getToolbar(); - if (toolbar == null) { - Log.w(TAG, "Toolbar is null. Cannot apply settings transition!"); - super.startActivityForResult(intent, requestCode, options); - return; - } - if (options != null) { - super.startActivityForResult(intent, requestCode, - getMergedBundleForTransition(options)); - return; - } - super.startActivityForResult(intent, requestCode, getActivityOptionsBundle(toolbar)); + super.startActivityForResult(intent, requestCode, createActivityOptionsBundleForTransition( + options)); } - @Override - public boolean onOptionsItemSelected(@NonNull MenuItem item) { - final int id = item.getItemId(); - if (id == android.R.id.home) { - // Make the up button behave the same as the back button. - onBackPressed(); - return true; - } - return super.onOptionsItemSelected(item); + protected boolean isSettingsTransitionEnabled() { + return BuildCompat.isAtLeastS(); } - /** - * Subclasses should implement this method and return their {@link Toolbar}. - */ - public abstract Toolbar getToolbar(); - - private Bundle getActivityOptionsBundle(Toolbar toolbar) { - return ActivityOptions.makeSceneTransitionAnimation(this, toolbar, - "shared_element_view").toBundle(); - } - - private Bundle getMergedBundleForTransition(@NonNull Bundle options) { - final Toolbar toolbar = getToolbar(); - final Bundle mergedBundle = new Bundle(); - mergedBundle.putAll(options); - final Bundle activityOptionsBundle = getActivityOptionsBundle(toolbar); - if (activityOptionsBundle != null) { - mergedBundle.putAll(activityOptionsBundle); + @Nullable + private Bundle createActivityOptionsBundleForTransition(@Nullable Bundle options) { + if (mToolbar == null) { + Log.w(TAG, "setActionBar(Toolbar) is not called. Cannot apply settings transition!"); + return options; } - return mergedBundle; + final Bundle transitionOptions = ActivityOptions.makeSceneTransitionAnimation(this, + mToolbar, "shared_element_view").toBundle(); + if (options == null) { + return transitionOptions; + } + final Bundle mergedOptions = new Bundle(options); + mergedOptions.putAll(transitionOptions); + return mergedOptions; } }