From 2f757eec2f7cd318491240173ee5e1da915aa6cb Mon Sep 17 00:00:00 2001 From: Jacqueline Bronger Date: Tue, 22 Feb 2022 13:15:22 +0100 Subject: [PATCH] Improve TV PiP menu buttons - Save focused button when moving and changing orientation (e.g. during expanded mode being toggled) - Focus the first button when the menu is opened - Makes sure all buttons are visibly reachable by setting the right gravity based on orientation and whether all the buttons fit - Scrolls to the focused button if it happens to go out of bounds during any change - Fade buttons out and in during orientation change - Scale buttons to 1.1x when selected - Fix move button description (typo in resource reference) Bug: 220109276 Bug: 219887361 Test: manual Change-Id: Ibe1b2ee3e505bd218c49f6cd0cbcbf7956452b26 --- .../tv_pip_menu_action_button_animator.xml | 53 +++++ .../Shell/res/layout/tv_pip_menu.xml | 8 +- .../res/layout/tv_pip_menu_action_button.xml | 11 +- .../Shell/res/values-tvdpi/dimen.xml | 2 +- .../wm/shell/pip/tv/TvPipController.java | 2 + .../shell/pip/tv/TvPipMenuActionButton.java | 12 +- .../wm/shell/pip/tv/TvPipMenuController.java | 12 +- .../wm/shell/pip/tv/TvPipMenuView.java | 223 +++++++++++++++--- 8 files changed, 275 insertions(+), 48 deletions(-) create mode 100644 libs/WindowManager/Shell/res/animator/tv_pip_menu_action_button_animator.xml diff --git a/libs/WindowManager/Shell/res/animator/tv_pip_menu_action_button_animator.xml b/libs/WindowManager/Shell/res/animator/tv_pip_menu_action_button_animator.xml new file mode 100644 index 0000000000000..7475abac46957 --- /dev/null +++ b/libs/WindowManager/Shell/res/animator/tv_pip_menu_action_button_animator.xml @@ -0,0 +1,53 @@ + + + + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/libs/WindowManager/Shell/res/layout/tv_pip_menu.xml b/libs/WindowManager/Shell/res/layout/tv_pip_menu.xml index dbd5a9b370ab6..5413213356a8c 100644 --- a/libs/WindowManager/Shell/res/layout/tv_pip_menu.xml +++ b/libs/WindowManager/Shell/res/layout/tv_pip_menu.xml @@ -33,7 +33,6 @@ android:id="@+id/tv_pip_menu_scroll" android:layout_width="match_parent" android:layout_height="match_parent" - android:gravity="center_horizontal" android:layout_alignTop="@+id/tv_pip" android:layout_alignStart="@+id/tv_pip" android:layout_alignEnd="@+id/tv_pip" @@ -49,15 +48,12 @@ android:layout_alignStart="@+id/tv_pip" android:layout_alignEnd="@+id/tv_pip" android:layout_alignBottom="@+id/tv_pip" - android:gravity="center_vertical" android:scrollbars="none"> @@ -77,7 +73,7 @@ android:layout_width="wrap_content" android:layout_height="wrap_content" android:src="@drawable/pip_ic_move_white" - android:text="@String/pip_move" /> + android:text="@string/pip_move" /> + + - 40dp + 48dp 20dp 20dp 4dp diff --git a/libs/WindowManager/Shell/src/com/android/wm/shell/pip/tv/TvPipController.java b/libs/WindowManager/Shell/src/com/android/wm/shell/pip/tv/TvPipController.java index 7f155cae4ff5e..8326588bbbad8 100644 --- a/libs/WindowManager/Shell/src/com/android/wm/shell/pip/tv/TvPipController.java +++ b/libs/WindowManager/Shell/src/com/android/wm/shell/pip/tv/TvPipController.java @@ -384,6 +384,7 @@ public class TvPipController implements PipTransitionController.PipTransitionCal animationDuration, rect -> { mTvPipMenuController.updateExpansionState(); }); + mTvPipMenuController.onPipTransitionStarted(bounds); } /** @@ -573,6 +574,7 @@ public class TvPipController implements PipTransitionController.PipTransitionCal "%s: onExpandedAspectRatioChanged: %f", TAG, ratio); mTvPipBoundsState.setDesiredTvExpandedAspectRatio(ratio, false); + mTvPipMenuController.updateExpansionState(); // 1) PiP is expanded and only aspect ratio changed, but wasn't disabled // --> update bounds, but don't toggle diff --git a/libs/WindowManager/Shell/src/com/android/wm/shell/pip/tv/TvPipMenuActionButton.java b/libs/WindowManager/Shell/src/com/android/wm/shell/pip/tv/TvPipMenuActionButton.java index abbc614b4b4ff..a09aab666a313 100644 --- a/libs/WindowManager/Shell/src/com/android/wm/shell/pip/tv/TvPipMenuActionButton.java +++ b/libs/WindowManager/Shell/src/com/android/wm/shell/pip/tv/TvPipMenuActionButton.java @@ -33,6 +33,7 @@ import com.android.wm.shell.R; */ public class TvPipMenuActionButton extends RelativeLayout implements View.OnClickListener { private final ImageView mIconImageView; + private final View mButtonBackgroundView; private final View mButtonView; private OnClickListener mOnClickListener; @@ -57,6 +58,7 @@ public class TvPipMenuActionButton extends RelativeLayout implements View.OnClic mIconImageView = findViewById(R.id.icon); mButtonView = findViewById(R.id.button); + mButtonBackgroundView = findViewById(R.id.background); final int[] values = new int[]{android.R.attr.src, android.R.attr.text}; final TypedArray typedArray = context.obtainStyledAttributes(attrs, values, defStyleAttr, @@ -132,9 +134,17 @@ public class TvPipMenuActionButton extends RelativeLayout implements View.OnClic getResources().getColorStateList( isCustomCloseAction ? R.color.tv_pip_menu_close_icon : R.color.tv_pip_menu_icon)); - mButtonView.setBackgroundTintList(getResources() + mButtonBackgroundView.setBackgroundTintList(getResources() .getColorStateList(isCustomCloseAction ? R.color.tv_pip_menu_close_icon_bg : R.color.tv_pip_menu_icon_bg)); } + @Override + public String toString() { + if (mButtonView.getContentDescription() == null) { + return TvPipMenuActionButton.class.getSimpleName(); + } + return mButtonView.getContentDescription().toString(); + } + } diff --git a/libs/WindowManager/Shell/src/com/android/wm/shell/pip/tv/TvPipMenuController.java b/libs/WindowManager/Shell/src/com/android/wm/shell/pip/tv/TvPipMenuController.java index bbd21b4940a5e..132c04481bce4 100644 --- a/libs/WindowManager/Shell/src/com/android/wm/shell/pip/tv/TvPipMenuController.java +++ b/libs/WindowManager/Shell/src/com/android/wm/shell/pip/tv/TvPipMenuController.java @@ -199,6 +199,9 @@ public class TvPipMenuController implements PipMenuController, TvPipMenuView.Lis void notifyPipAnimating(boolean animating) { mPipMenuView.setEduTextActive(!animating); + if (!animating) { + mPipMenuView.onPipTransitionFinished(); + } } void showMovementMenuOnly() { @@ -235,6 +238,13 @@ public class TvPipMenuController implements PipMenuController, TvPipMenuView.Lis } else { mPipMenuView.showButtonsMenu(); } + mPipMenuView.updateBounds(mTvPipBoundsState.getBounds()); + } + + void onPipTransitionStarted(Rect finishBounds) { + if (mPipMenuView != null) { + mPipMenuView.onPipTransitionStarted(finishBounds); + } } private void maybeCloseEduText() { @@ -559,7 +569,7 @@ public class TvPipMenuController implements PipMenuController, TvPipMenuView.Lis menuBounds.height())); if (mPipMenuView != null) { - mPipMenuView.updateLayout(destinationBounds); + mPipMenuView.updateBounds(destinationBounds); } } diff --git a/libs/WindowManager/Shell/src/com/android/wm/shell/pip/tv/TvPipMenuView.java b/libs/WindowManager/Shell/src/com/android/wm/shell/pip/tv/TvPipMenuView.java index 3161b02d15454..9e2d130669615 100644 --- a/libs/WindowManager/Shell/src/com/android/wm/shell/pip/tv/TvPipMenuView.java +++ b/libs/WindowManager/Shell/src/com/android/wm/shell/pip/tv/TvPipMenuView.java @@ -44,8 +44,10 @@ import android.view.View; import android.view.ViewGroup; import android.view.ViewRootImpl; import android.widget.FrameLayout; +import android.widget.HorizontalScrollView; import android.widget.ImageView; import android.widget.LinearLayout; +import android.widget.ScrollView; import android.widget.TextView; import androidx.annotation.NonNull; @@ -90,15 +92,21 @@ public class TvPipMenuView extends FrameLayout implements View.OnClickListener { private final ImageView mArrowDown; private final ImageView mArrowLeft; - private final ViewGroup mScrollView; - private final ViewGroup mHorizontalScrollView; + private final ScrollView mScrollView; + private final HorizontalScrollView mHorizontalScrollView; + private View mFocusedButton; private Rect mCurrentPipBounds; + private boolean mMoveMenuIsVisible; + private boolean mButtonMenuIsVisible; private final TvPipMenuActionButton mExpandButton; private final TvPipMenuActionButton mCloseButton; + private boolean mSwitchingOrientation; + private final int mPipMenuFadeAnimationDuration; + private final int mResizeAnimationDuration; public TvPipMenuView(@NonNull Context context) { this(context, null); @@ -146,8 +154,11 @@ public class TvPipMenuView extends FrameLayout implements View.OnClickListener { mEduTextView = findViewById(R.id.tv_pip_menu_edu_text); mEduTextContainerView = findViewById(R.id.tv_pip_menu_edu_text_container); + mResizeAnimationDuration = context.getResources().getInteger( + R.integer.config_pipResizeAnimationDuration); mPipMenuFadeAnimationDuration = context.getResources() .getInteger(R.integer.pip_menu_fade_animation_duration); + mPipMenuOuterSpace = context.getResources() .getDimensionPixelSize(R.dimen.pip_menu_outer_space); mPipMenuBorderWidth = context.getResources() @@ -203,43 +214,152 @@ public class TvPipMenuView extends FrameLayout implements View.OnClickListener { heightAnimation.start(); } - void updateLayout(Rect updatedPipBounds) { + void onPipTransitionStarted(Rect finishBounds) { + final boolean vertical = finishBounds.height() > finishBounds.width(); + final boolean orientationChanged = + vertical != (mActionButtonsContainer.getOrientation() == LinearLayout.VERTICAL); ProtoLog.d(ShellProtoLogGroup.WM_SHELL_PICTURE_IN_PICTURE, - "%s: update menu layout: %s", TAG, updatedPipBounds.toShortString()); + "%s: onPipTransitionStarted(), orientation changed %b", TAG, orientationChanged); + if (!orientationChanged) { + return; + } - boolean previouslyVertical = - mCurrentPipBounds != null && mCurrentPipBounds.height() > mCurrentPipBounds.width(); - boolean vertical = updatedPipBounds.height() > updatedPipBounds.width(); + if (mButtonMenuIsVisible) { + mSwitchingOrientation = true; + mActionButtonsContainer.animate() + .alpha(0) + .setInterpolator(TvPipInterpolators.EXIT) + .setDuration(mResizeAnimationDuration / 2) + .withEndAction(() -> { + changeButtonScrollOrientation(finishBounds); + updateButtonGravity(finishBounds); + mActionButtonsContainer.animate() + .alpha(1) + .setInterpolator(TvPipInterpolators.ENTER) + .setDuration(mResizeAnimationDuration / 2); + }); + } else { + changeButtonScrollOrientation(finishBounds); + updateButtonGravity(finishBounds); + } + } - mCurrentPipBounds = updatedPipBounds; + void onPipTransitionFinished() { + ProtoLog.d(ShellProtoLogGroup.WM_SHELL_PICTURE_IN_PICTURE, + "%s: onPipTransitionFinished()", TAG); + if (!mSwitchingOrientation) { + refocusPreviousButton(); + } + mSwitchingOrientation = false; + } + + /** + * Also updates the button gravity. + */ + void updateBounds(Rect updatedBounds) { + ProtoLog.d(ShellProtoLogGroup.WM_SHELL_PICTURE_IN_PICTURE, + "%s: updateLayout, width: %s, height: %s", TAG, updatedBounds.width(), + updatedBounds.height()); + mCurrentPipBounds = updatedBounds; + if (!mSwitchingOrientation) { + updateButtonGravity(mCurrentPipBounds); + } updatePipFrameBounds(); + } - if (previouslyVertical == vertical) { - if (DEBUG) { - ProtoLog.d(ShellProtoLogGroup.WM_SHELL_PICTURE_IN_PICTURE, - "%s: no update for menu layout", TAG); - } - return; - } else { - if (DEBUG) { - ProtoLog.d(ShellProtoLogGroup.WM_SHELL_PICTURE_IN_PICTURE, - "%s: change menu layout to vertical: %b", TAG, vertical); + private void changeButtonScrollOrientation(Rect bounds) { + final boolean vertical = bounds.height() > bounds.width(); + + final ViewGroup oldScrollView = vertical ? mHorizontalScrollView : mScrollView; + final ViewGroup newScrollView = vertical ? mScrollView : mHorizontalScrollView; + + if (oldScrollView.getChildCount() == 1) { + ProtoLog.d(ShellProtoLogGroup.WM_SHELL_PICTURE_IN_PICTURE, + "%s: orientation changed", TAG); + oldScrollView.removeView(mActionButtonsContainer); + oldScrollView.setVisibility(GONE); + mActionButtonsContainer.setOrientation(vertical ? LinearLayout.VERTICAL + : LinearLayout.HORIZONTAL); + newScrollView.addView(mActionButtonsContainer); + newScrollView.setVisibility(VISIBLE); + if (mFocusedButton != null) { + mFocusedButton.requestFocus(); } } + } + + /** + * Change button gravity based on new dimensions + */ + private void updateButtonGravity(Rect bounds) { + final boolean vertical = bounds.height() > bounds.width(); + // Use Math.max since the possible orientation change might not have been applied yet. + final int buttonsSize = Math.max(mActionButtonsContainer.getHeight(), + mActionButtonsContainer.getWidth()); + + ProtoLog.d(ShellProtoLogGroup.WM_SHELL_PICTURE_IN_PICTURE, + "%s: buttons container width: %s, height: %s", TAG, + mActionButtonsContainer.getWidth(), mActionButtonsContainer.getHeight()); + + final boolean buttonsFit = + vertical ? buttonsSize < bounds.height() + : buttonsSize < bounds.width(); + final int buttonGravity = buttonsFit ? Gravity.CENTER + : (vertical ? Gravity.CENTER_HORIZONTAL : Gravity.CENTER_VERTICAL); + + final LayoutParams params = (LayoutParams) mActionButtonsContainer.getLayoutParams(); + params.gravity = buttonGravity; + mActionButtonsContainer.setLayoutParams(params); + + ProtoLog.d(ShellProtoLogGroup.WM_SHELL_PICTURE_IN_PICTURE, + "%s: vertical: %b, buttonsFit: %b, gravity: %s", TAG, vertical, buttonsFit, + Gravity.toString(buttonGravity)); + } + + private void refocusPreviousButton() { + if (mMoveMenuIsVisible || mCurrentPipBounds == null || mFocusedButton == null) { + return; + } + final boolean vertical = mCurrentPipBounds.height() > mCurrentPipBounds.width(); + + if (!mFocusedButton.hasFocus()) { + ProtoLog.d(ShellProtoLogGroup.WM_SHELL_PICTURE_IN_PICTURE, + "%s: request focus from: %s", TAG, mFocusedButton); + mFocusedButton.requestFocus(); + } else { + ProtoLog.d(ShellProtoLogGroup.WM_SHELL_PICTURE_IN_PICTURE, + "%s: already focused: %s", TAG, mFocusedButton); + } + + // Do we need to scroll? + final Rect buttonBounds = new Rect(); + final Rect scrollBounds = new Rect(); + if (vertical) { + mScrollView.getDrawingRect(scrollBounds); + } else { + mHorizontalScrollView.getDrawingRect(scrollBounds); + } + mFocusedButton.getHitRect(buttonBounds); + + if (scrollBounds.contains(buttonBounds)) { + // Button is already completely visible, don't scroll + ProtoLog.d(ShellProtoLogGroup.WM_SHELL_PICTURE_IN_PICTURE, + "%s: not scrolling", TAG); + return; + } + + // Scrolling so the button is visible to the user. + ProtoLog.d(ShellProtoLogGroup.WM_SHELL_PICTURE_IN_PICTURE, + "%s: scrolling to focused button", TAG); if (vertical) { - mHorizontalScrollView.removeView(mActionButtonsContainer); - mScrollView.addView(mActionButtonsContainer); + mScrollView.smoothScrollTo((int) mFocusedButton.getX(), + (int) mFocusedButton.getY()); } else { - mScrollView.removeView(mActionButtonsContainer); - mHorizontalScrollView.addView(mActionButtonsContainer); + mHorizontalScrollView.smoothScrollTo((int) mFocusedButton.getX(), + (int) mFocusedButton.getY()); } - mActionButtonsContainer.setOrientation(vertical ? LinearLayout.VERTICAL - : LinearLayout.HORIZONTAL); - - mScrollView.setVisibility(vertical ? VISIBLE : GONE); - mHorizontalScrollView.setVisibility(vertical ? GONE : VISIBLE); } Rect getPipMenuContainerBounds(Rect pipBounds) { @@ -300,6 +420,8 @@ public class TvPipMenuView extends FrameLayout implements View.OnClickListener { if (DEBUG) { ProtoLog.d(ShellProtoLogGroup.WM_SHELL_PICTURE_IN_PICTURE, "%s: showMoveMenu()", TAG); } + mButtonMenuIsVisible = false; + mMoveMenuIsVisible = true; showButtonsMenu(false); showMovementHints(gravity); setFrameHighlighted(true); @@ -310,19 +432,34 @@ public class TvPipMenuView extends FrameLayout implements View.OnClickListener { ProtoLog.d(ShellProtoLogGroup.WM_SHELL_PICTURE_IN_PICTURE, "%s: showButtonsMenu()", TAG); } + + mButtonMenuIsVisible = true; + mMoveMenuIsVisible = false; showButtonsMenu(true); hideMovementHints(); setFrameHighlighted(true); + + // Always focus on the first button when opening the menu, except directly after moving. + if (mFocusedButton == null) { + // Focus on first button (there is a Space at position 0) + mFocusedButton = mActionButtonsContainer.getChildAt(1); + // Reset scroll position. + mScrollView.scrollTo(0, 0); + mHorizontalScrollView.scrollTo( + isLayoutRtl() ? mActionButtonsContainer.getWidth() : 0, 0); + } + refocusPreviousButton(); } /** * Hides all menu views, including the menu frame. */ void hideAllUserControls() { - if (DEBUG) { - ProtoLog.d(ShellProtoLogGroup.WM_SHELL_PICTURE_IN_PICTURE, - "%s: hideAllUserControls()", TAG); - } + ProtoLog.d(ShellProtoLogGroup.WM_SHELL_PICTURE_IN_PICTURE, + "%s: hideAllUserControls()", TAG); + mFocusedButton = null; + mButtonMenuIsVisible = false; + mMoveMenuIsVisible = false; showButtonsMenu(false); hideMovementHints(); setFrameHighlighted(false); @@ -404,12 +541,21 @@ public class TvPipMenuView extends FrameLayout implements View.OnClickListener { } setActionForButton(action, button, mainHandler); } + + if (mCurrentPipBounds != null) { + updateButtonGravity(mCurrentPipBounds); + refocusPreviousButton(); + } } private void setActionForButton(RemoteAction action, TvPipMenuActionButton button, Handler mainHandler) { button.setVisibility(View.VISIBLE); // Ensure the button is visible. - button.setTextAndDescription(action.getContentDescription()); + if (action.getContentDescription().length() > 0) { + button.setTextAndDescription(action.getContentDescription()); + } else { + button.setTextAndDescription(action.getTitle()); + } button.setEnabled(action.isEnabled()); button.setTag(action); action.getIcon().loadDrawableAsync(mContext, button::setImageDrawable, mainHandler); @@ -460,12 +606,11 @@ public class TvPipMenuView extends FrameLayout implements View.OnClickListener { @Override public boolean dispatchKeyEvent(KeyEvent event) { - if (DEBUG) { - ProtoLog.d(ShellProtoLogGroup.WM_SHELL_PICTURE_IN_PICTURE, - "%s: dispatchKeyEvent, action: %d, keycode: %d", - TAG, event.getAction(), event.getKeyCode()); - } if (mListener != null && event.getAction() == ACTION_UP) { + if (!mMoveMenuIsVisible) { + mFocusedButton = mActionButtonsContainer.getFocusedChild(); + } + switch (event.getKeyCode()) { case KEYCODE_BACK: mListener.onBackPress(); @@ -527,6 +672,10 @@ public class TvPipMenuView extends FrameLayout implements View.OnClickListener { ProtoLog.d(ShellProtoLogGroup.WM_SHELL_PICTURE_IN_PICTURE, "%s: showUserActions: %b", TAG, show); } + if (show) { + mActionButtonsContainer.setVisibility(VISIBLE); + refocusPreviousButton(); + } animateAlphaTo(show ? 1 : 0, mActionButtonsContainer); }