From 00f0a93772b74a7503a65f75c1adc3454e52e9de Mon Sep 17 00:00:00 2001 From: Winson Chung Date: Tue, 27 Apr 2021 17:08:14 -0700 Subject: [PATCH] Fix a couple issues with nav bar & rotation button - Ensure that newly added buttons to the contextual group are not visible by default, this can cause issues where they are clickable if setButtonVisibility() is not called to ensure only one button is visible - Fix an issue where the rotation controller and context group buttons were not updated on nav mode change (leading to floating rotation button w/ 3 button nav and rotation context button w/ gestural nav). This is a regression in S due to us not reinflating the entire nav bar view on nav mode change. - Move ime switcher handling to NavBar to be consistent with other contextual buttons - Prevent reloading nav bar multiple times on nav mode change Fixes: 183475747 Test: Change nav modes and dump the context buttons to verify they are updated correctly Test: atest SystemUITests Change-Id: I023cf5f4a0689c6498043a6c8685c5c920d8da46 --- .../systemui/navigationbar/NavigationBar.java | 10 +++- .../NavigationBarController.java | 3 ++ .../NavigationBarInflaterView.java | 1 - .../navigationbar/NavigationBarView.java | 51 ++++++++++--------- .../RotationButtonController.java | 15 +++--- .../buttons/ContextualButtonGroup.java | 13 +++++ .../NavigationBarRotationContextTest.java | 6 ++- 7 files changed, 65 insertions(+), 34 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/navigationbar/NavigationBar.java b/packages/SystemUI/src/com/android/systemui/navigationbar/NavigationBar.java index 56375adb3d4e3..4e41d75e3f437 100644 --- a/packages/SystemUI/src/com/android/systemui/navigationbar/NavigationBar.java +++ b/packages/SystemUI/src/com/android/systemui/navigationbar/NavigationBar.java @@ -101,6 +101,7 @@ import android.view.WindowManager; import android.view.accessibility.AccessibilityEvent; import android.view.accessibility.AccessibilityManager; import android.view.accessibility.AccessibilityManager.AccessibilityServicesStateChangeListener; +import android.view.inputmethod.InputMethodManager; import androidx.annotation.VisibleForTesting; @@ -1175,6 +1176,9 @@ public class NavigationBar implements View.OnAttachStateChangeListener, accessibilityButton.setOnLongClickListener(this::onAccessibilityLongClick); updateAccessibilityServicesState(mAccessibilityManager); + ButtonDispatcher imeSwitcherButton = mNavigationBarView.getImeSwitchButton(); + imeSwitcherButton.setOnClickListener(this::onImeSwitcherClick); + updateScreenPinningGestures(); } @@ -1274,6 +1278,11 @@ public class NavigationBar implements View.OnAttachStateChangeListener, mCommandQueue.toggleRecentApps(); } + private void onImeSwitcherClick(View v) { + mContext.getSystemService(InputMethodManager.class).showInputMethodPickerFromSystem( + true /* showAuxiliarySubtypes */, mDisplayId); + }; + private boolean onLongPressBackHome(View v) { return onLongPressNavigationButtons(v, R.id.back, R.id.home); } @@ -1282,7 +1291,6 @@ public class NavigationBar implements View.OnAttachStateChangeListener, return onLongPressNavigationButtons(v, R.id.back, R.id.recent_apps); } - /** * This handles long-press of both back and recents/home. Back is the common button with * combination of recents if it is visible or home if recents is invisible. diff --git a/packages/SystemUI/src/com/android/systemui/navigationbar/NavigationBarController.java b/packages/SystemUI/src/com/android/systemui/navigationbar/NavigationBarController.java index 3544f60601a86..66cfae4315d8a 100644 --- a/packages/SystemUI/src/com/android/systemui/navigationbar/NavigationBarController.java +++ b/packages/SystemUI/src/com/android/systemui/navigationbar/NavigationBarController.java @@ -217,6 +217,9 @@ public class NavigationBarController implements Callbacks, @Override public void onNavigationModeChanged(int mode) { + if (mNavMode == mode) { + return; + } final int oldMode = mNavMode; mNavMode = mode; mHandler.post(() -> { diff --git a/packages/SystemUI/src/com/android/systemui/navigationbar/NavigationBarInflaterView.java b/packages/SystemUI/src/com/android/systemui/navigationbar/NavigationBarInflaterView.java index 7342f91a81087..4d9175b8db686 100644 --- a/packages/SystemUI/src/com/android/systemui/navigationbar/NavigationBarInflaterView.java +++ b/packages/SystemUI/src/com/android/systemui/navigationbar/NavigationBarInflaterView.java @@ -158,7 +158,6 @@ public class NavigationBarInflaterView extends FrameLayout } public void onLikelyDefaultLayoutChange() { - // Reevaluate new layout final String newValue = getDefaultLayout(); if (!Objects.equals(mCurrentLayout, newValue)) { diff --git a/packages/SystemUI/src/com/android/systemui/navigationbar/NavigationBarView.java b/packages/SystemUI/src/com/android/systemui/navigationbar/NavigationBarView.java index 0ed4d861c7127..f82d265d12d69 100644 --- a/packages/SystemUI/src/com/android/systemui/navigationbar/NavigationBarView.java +++ b/packages/SystemUI/src/com/android/systemui/navigationbar/NavigationBarView.java @@ -166,6 +166,7 @@ public class NavigationBarView extends FrameLayout implements private NavigationBarInflaterView mNavigationInflaterView; private RecentsOnboarding mRecentsOnboarding; private NotificationPanelViewController mPanelView; + private RotationContextButton mRotationContextButton; private FloatingRotationButton mFloatingRotationButton; private RotationButtonController mRotationButtonController; private NavigationBarOverlayController mNavBarOverlayController; @@ -233,14 +234,6 @@ public class NavigationBarView extends FrameLayout implements } } - private final OnClickListener mImeSwitcherClickListener = new OnClickListener() { - @Override - public void onClick(View view) { - mContext.getSystemService(InputMethodManager.class).showInputMethodPickerFromSystem( - true /* showAuxiliarySubtypes */, getContext().getDisplayId()); - } - }; - private final AccessibilityDelegate mQuickStepAccessibilityDelegate = new AccessibilityDelegate() { private AccessibilityAction mToggleOverviewAction; @@ -311,33 +304,26 @@ public class NavigationBarView extends FrameLayout implements mIsVertical = false; mLongClickableAccessibilityButton = false; mNavBarMode = Dependency.get(NavigationModeController.class).addListener(this); - boolean isGesturalMode = isGesturalMode(mNavBarMode); mSysUiFlagContainer = Dependency.get(SysUiState.class); // Set up the context group of buttons mContextualButtonGroup = new ContextualButtonGroup(R.id.menu_container); final ContextualButton imeSwitcherButton = new ContextualButton(R.id.ime_switcher, mLightContext, R.drawable.ic_ime_switcher_default); - final RotationContextButton rotateSuggestionButton = new RotationContextButton( - R.id.rotate_suggestion, mLightContext, - R.drawable.ic_sysbar_rotate_button_ccw_start_0); final ContextualButton accessibilityButton = new ContextualButton(R.id.accessibility_button, mLightContext, R.drawable.ic_sysbar_accessibility_button); mContextualButtonGroup.addButton(imeSwitcherButton); - if (!isGesturalMode) { - mContextualButtonGroup.addButton(rotateSuggestionButton); - } mContextualButtonGroup.addButton(accessibilityButton); + mRotationContextButton = new RotationContextButton(R.id.rotate_suggestion, + mLightContext, R.drawable.ic_sysbar_rotate_button_ccw_start_0); + mFloatingRotationButton = new FloatingRotationButton(context); + mRotationButtonController = new RotationButtonController(mLightContext, + mLightIconColor, mDarkIconColor); + updateRotationButton(); mOverviewProxyService = Dependency.get(OverviewProxyService.class); - mFloatingRotationButton = new FloatingRotationButton(context); mRecentsOnboarding = new RecentsOnboarding(context, mOverviewProxyService); - mRotationButtonController = new RotationButtonController(mLightContext, - mLightIconColor, mDarkIconColor, - isGesturalMode ? mFloatingRotationButton : rotateSuggestionButton, - mRotationButtonListener); - mNavBarOverlayController = Dependency.get(NavigationBarOverlayController.class); if (mNavBarOverlayController.isNavigationBarOverlayEnabled()) { mNavBarOverlayController.init( @@ -357,7 +343,6 @@ public class NavigationBarView extends FrameLayout implements mButtonDispatchers.put(R.id.recent_apps, new ButtonDispatcher(R.id.recent_apps)); mButtonDispatchers.put(R.id.ime_switcher, imeSwitcherButton); mButtonDispatchers.put(R.id.accessibility_button, accessibilityButton); - mButtonDispatchers.put(R.id.rotate_suggestion, rotateSuggestionButton); mButtonDispatchers.put(R.id.menu_container, mContextualButtonGroup); mDeadZone = new DeadZone(this); @@ -555,6 +540,23 @@ public class NavigationBarView extends FrameLayout implements } } + /** + * Updates the rotation button based on the current navigation mode. + */ + private void updateRotationButton() { + if (isGesturalMode(mNavBarMode)) { + mContextualButtonGroup.removeButton(R.id.rotate_suggestion); + mButtonDispatchers.remove(R.id.rotate_suggestion); + mRotationButtonController.setRotationButton(mFloatingRotationButton, + mRotationButtonListener); + } else if (mContextualButtonGroup.getContextButton(R.id.rotate_suggestion) == null) { + mContextualButtonGroup.addButton(mRotationContextButton); + mButtonDispatchers.put(R.id.rotate_suggestion, mRotationContextButton); + mRotationButtonController.setRotationButton(mRotationContextButton, + mRotationButtonListener); + } + } + public KeyButtonDrawable getBackDrawable() { KeyButtonDrawable drawable = getDrawable(getBackDrawableRes()); orientBackButton(drawable); @@ -908,6 +910,7 @@ public class NavigationBarView extends FrameLayout implements mBarTransitions.onNavigationModeChanged(mNavBarMode); mEdgeBackGestureHandler.onNavigationModeChanged(mNavBarMode); mRecentsOnboarding.onNavigationModeChanged(mNavBarMode); + updateRotationButton(); if (isGesturalMode(mNavBarMode)) { mRegionSamplingHelper.start(mSamplingBounds); @@ -932,7 +935,6 @@ public class NavigationBarView extends FrameLayout implements mNavigationInflaterView = findViewById(R.id.navigation_inflater); mNavigationInflaterView.setButtonDispatchers(mButtonDispatchers); - getImeSwitchButton().setOnClickListener(mImeSwitcherClickListener); updateOrientationViews(); reloadNavIcons(); } @@ -1027,6 +1029,9 @@ public class NavigationBarView extends FrameLayout implements private void updateButtonLocation(ButtonDispatcher button, boolean inScreenSpace, boolean useNearestRegion) { + if (button == null) { + return; + } View view = button.getCurrentView(); if (view == null || !button.isVisible()) { return; diff --git a/packages/SystemUI/src/com/android/systemui/navigationbar/RotationButtonController.java b/packages/SystemUI/src/com/android/systemui/navigationbar/RotationButtonController.java index 4bcb0193c7d0f..ddf089bac25e8 100644 --- a/packages/SystemUI/src/com/android/systemui/navigationbar/RotationButtonController.java +++ b/packages/SystemUI/src/com/android/systemui/navigationbar/RotationButtonController.java @@ -66,10 +66,10 @@ public class RotationButtonController { private static final int NUM_ACCEPTED_ROTATION_SUGGESTIONS_FOR_INTRODUCTION = 3; private final Context mContext; - private final RotationButton mRotationButton; private final Handler mMainThreadHandler = new Handler(Looper.getMainLooper()); private final UiEventLogger mUiEventLogger = new UiEventLoggerImpl(); private final ViewRippler mViewRippler = new ViewRippler(); + private RotationButton mRotationButton; private int mLastRotationSuggestion; private boolean mPendingRotationSuggestion; @@ -125,20 +125,21 @@ public class RotationButtonController { } RotationButtonController(Context context, @ColorInt int lightIconColor, - @ColorInt int darkIconColor, RotationButton rotationButton, - Consumer visibilityChangedCallback) { + @ColorInt int darkIconColor) { mContext = context; mLightIconColor = lightIconColor; mDarkIconColor = darkIconColor; - mRotationButton = rotationButton; - mRotationButton.setRotationButtonController(this); mIsNavigationBarShowing = true; mRotationLockController = Dependency.get(RotationLockController.class); mAccessibilityManagerWrapper = Dependency.get(AccessibilityManagerWrapper.class); - - // Register the task stack listener mTaskStackListener = new TaskStackListenerImpl(); + } + + void setRotationButton(RotationButton rotationButton, + Consumer visibilityChangedCallback) { + mRotationButton = rotationButton; + mRotationButton.setRotationButtonController(this); mRotationButton.setOnClickListener(this::onRotateSuggestionClick); mRotationButton.setOnHoverListener(this::onRotateSuggestionHover); mRotationButton.setVisibilityChangedCallback(visibilityChangedCallback); diff --git a/packages/SystemUI/src/com/android/systemui/navigationbar/buttons/ContextualButtonGroup.java b/packages/SystemUI/src/com/android/systemui/navigationbar/buttons/ContextualButtonGroup.java index 50b638bcc903f..2ace303986fe0 100644 --- a/packages/SystemUI/src/com/android/systemui/navigationbar/buttons/ContextualButtonGroup.java +++ b/packages/SystemUI/src/com/android/systemui/navigationbar/buttons/ContextualButtonGroup.java @@ -41,10 +41,23 @@ public class ContextualButtonGroup extends ButtonDispatcher { * @param button the button added to the group */ public void addButton(@NonNull ContextualButton button) { + // By default buttons in the context group are not visible until + // {@link #setButtonVisibility()) is called to show one of the buttons + button.setVisibility(View.INVISIBLE); button.attachToGroup(this); mButtonData.add(new ButtonData(button)); } + /** + * Removes a contextual button from the group. + */ + public void removeButton(@IdRes int buttonResId) { + int index = getContextButtonIndex(buttonResId); + if (index != INVALID_INDEX) { + mButtonData.remove(index); + } + } + public ContextualButton getContextButton(@IdRes int buttonResId) { int index = getContextButtonIndex(buttonResId); if (index != INVALID_INDEX) { diff --git a/packages/SystemUI/tests/src/com/android/systemui/navigationbar/NavigationBarRotationContextTest.java b/packages/SystemUI/tests/src/com/android/systemui/navigationbar/NavigationBarRotationContextTest.java index 47f41836f1a63..eac68f6e76f45 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/navigationbar/NavigationBarRotationContextTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/navigationbar/NavigationBarRotationContextTest.java @@ -62,8 +62,10 @@ public class NavigationBarRotationContextTest extends SysuiTestCase { final View view = new View(mContext); mRotationButton = mock(RotationButton.class); - mRotationButtonController = spy(new RotationButtonController(mContext, 0, 0, - mRotationButton, (visibility) -> {})); + mRotationButtonController = new RotationButtonController(mContext, 0, 0); + mRotationButtonController.setRotationButton(mRotationButton, (visibility) -> {}); + // Due to a mockito issue, only spy the object after setting the initial state + mRotationButtonController = spy(mRotationButtonController); final KeyButtonDrawable kbd = mock(KeyButtonDrawable.class); doReturn(view).when(mRotationButton).getCurrentView(); doReturn(true).when(mRotationButton).acceptRotationProposal();