Skip to content

Commit

Permalink
[Merge M111][GTS][Cleanup] Remove unused code in Toolbar Tablet
Browse files Browse the repository at this point in the history 
Bug: 1408352

Change-Id: I4cc6b555adad96afdd1797e156db8e39391adc57
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4213318
Commit-Queue: Sirisha Kavuluru <skavuluru@google.com>
Reviewed-by: Theresa Sullivan <twellington@chromium.org>
Cr-Commit-Position: refs/branch-heads/5563@{#125}
Cr-Branched-From: 3ac59a6-refs/heads/main@{#1097615}
  • Loading branch information
Sirisha Kavuluru authored and Chromium LUCI CQ committed Feb 2, 2023
1 parent 85d8bd5 commit ccc169a
Show file tree
Hide file tree
Showing 5 changed files with 16 additions and 190 deletions.
2 changes: 1 addition & 1 deletion chrome/browser/ui/android/toolbar/java/res/layout/toolbar_tablet.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ found in the LICENSE file.
android:paddingStart="8dp"
android:layout_gravity="top"
android:contentDescription="@string/accessibility_toolbar_btn_tabswitcher_toggle_default"
android:visibility="gone" />
android:visibility="visible" />

<include layout="@layout/menu_button"/>
</LinearLayout>
Expand Down
6 changes: 0 additions & 6 deletions ...er/ui/android/toolbar/java/src/org/chromium/chrome/browser/toolbar/top/ToolbarLayout.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -523,12 +523,6 @@ void updateReloadButtonVisibility(boolean isReloading) {}
*/
void updateBookmarkButton(boolean isBookmarked, boolean editingAllowed) {}

/**
* Gives inheriting classes the chance to respond to accessibility state changes.
* @param enabled Whether or not accessibility is enabled.
*/
void onAccessibilityStatusChanged(boolean enabled) {}

/**
* Gives inheriting classes the chance to do the necessary UI operations after Chrome is
* restored to a previously saved state.
Expand Down
119 changes: 15 additions & 104 deletions ...er/ui/android/toolbar/java/src/org/chromium/chrome/browser/toolbar/top/ToolbarTablet.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
import android.animation.Animator;
import android.animation.AnimatorListenerAdapter;
import android.animation.AnimatorSet;
import android.animation.ObjectAnimator;
import android.annotation.SuppressLint;
import android.content.Context;
import android.content.res.ColorStateList;
Expand Down Expand Up @@ -52,10 +51,7 @@
import org.chromium.chrome.browser.toolbar.menu_button.MenuButtonCoordinator;
import org.chromium.chrome.browser.toolbar.top.NavigationPopup.HistoryDelegate;
import org.chromium.chrome.browser.ui.theme.BrandedColorScheme;
import org.chromium.chrome.browser.util.ChromeAccessibilityUtil;
import org.chromium.components.browser_ui.styles.ChromeColors;
import org.chromium.components.browser_ui.widget.animation.CancelAwareAnimatorListener;
import org.chromium.components.browser_ui.widget.animation.Interpolators;
import org.chromium.ui.UiUtils;
import org.chromium.ui.base.DeviceFormFactor;
import org.chromium.ui.widget.Toast;
Expand All @@ -70,8 +66,6 @@
@SuppressLint("Instantiatable")
public class ToolbarTablet
extends ToolbarLayout implements OnClickListener, View.OnLongClickListener {
private ObjectAnimator mTabSwitcherModeAnimation;

/**
* Downloads page for offline access.
*/
Expand All @@ -93,13 +87,11 @@ public interface OfflineDownloader {
private ImageButton mReloadButton;
private ImageButton mBookmarkButton;
private ImageButton mSaveOfflineButton;
private ToggleTabStackButton mAccessibilitySwitcherButton;
private ToggleTabStackButton mSwitcherButton;

private OnClickListener mBookmarkListener;

private boolean mIsInTabSwitcherMode;

private boolean mShowTabStack;
private boolean mToolbarButtonsVisible;
private ImageButton[] mToolbarButtons;
private ImageButton mOptionalButton;
Expand Down Expand Up @@ -164,15 +156,9 @@ public void onFinishInflate() {
getContext(), R.drawable.btn_close, R.color.default_icon_color_tint_list);
reloadIcon.addLevel(stopLevel, stopLevel, stopLevelDrawable);
mReloadButton.setImageDrawable(reloadIcon);
mShowTabStack = (ChromeAccessibilityUtil.get().isAccessibilityEnabled()
&& isAccessibilityTabSwitcherPreferenceEnabled())
|| isGridTabSwitcherEnabled();

mAccessibilitySwitcherButton = findViewById(R.id.tab_switcher_button);
updateSwitcherButtonVisibility(mShowTabStack);

mSwitcherButton = findViewById(R.id.tab_switcher_button);
mBookmarkButton = findViewById(R.id.bookmark_button);

mSaveOfflineButton = findViewById(R.id.save_offline_button);

// Initialize values needed for showing/hiding toolbar buttons when the activity size
Expand Down Expand Up @@ -406,11 +392,6 @@ public boolean onLongClick(View v) {
return Toast.showAnchoredToast(context, v, description);
}

private void updateSwitcherButtonVisibility(boolean enabled) {
mAccessibilitySwitcherButton.setVisibility(
mShowTabStack || enabled ? View.VISIBLE : View.GONE);
}

/**
* Update the visibility of the toolbar shadow.
*/
Expand Down Expand Up @@ -450,7 +431,7 @@ public void onTintChanged(ColorStateList tint, @BrandedColorScheme int brandedCo
ImageViewCompat.setImageTintList(mForwardButton, tint);
ImageViewCompat.setImageTintList(mSaveOfflineButton, tint);
ImageViewCompat.setImageTintList(mReloadButton, tint);
mAccessibilitySwitcherButton.setBrandedColorScheme(brandedColorScheme);
mSwitcherButton.setBrandedColorScheme(brandedColorScheme);

if (mOptionalButton != null && mOptionalButtonUsesTint) {
ImageViewCompat.setImageTintList(mOptionalButton, tint);
Expand Down Expand Up @@ -543,70 +524,17 @@ void updateBookmarkButton(boolean isBookmarked, boolean editingAllowed) {
@Override
void setTabSwitcherMode(boolean inTabSwitcherMode, boolean showToolbar, boolean delayAnimation,
MenuButtonCoordinator menuButtonCoordinator) {
boolean isInTabSwitcherMode = mShowTabStack && inTabSwitcherMode;
mIsInTabSwitcherMode = isInTabSwitcherMode;
mAccessibilitySwitcherButton.setClickable(!isInTabSwitcherMode);

if (isTabletGridTabSwitcherPolishEnabled()) {
int importantForAccessibility = inTabSwitcherMode
? View.IMPORTANT_FOR_ACCESSIBILITY_NO_HIDE_DESCENDANTS
: View.IMPORTANT_FOR_ACCESSIBILITY_AUTO;

mLocationBar.setUrlBarFocusable(!mIsInTabSwitcherMode);
if (getImportantForAccessibility() != importantForAccessibility) {
setImportantForAccessibility(importantForAccessibility);
sendAccessibilityEvent(AccessibilityEvent.TYPE_WINDOW_CONTENT_CHANGED);
}
} else if (isGridTabSwitcherEnabled()) {
setTabSwitcherModeWithAnimation(delayAnimation);
} else {
if (mIsInTabSwitcherMode) {
mBackButton.setEnabled(false);
mForwardButton.setEnabled(false);
mReloadButton.setEnabled(false);
mLocationBar.getContainerView().setVisibility(View.INVISIBLE);
menuButtonCoordinator.setAppMenuUpdateBadgeSuppressed(true);
} else {
mLocationBar.getContainerView().setVisibility(View.VISIBLE);
menuButtonCoordinator.setAppMenuUpdateBadgeSuppressed(false);
}
}
}

private void setTabSwitcherModeWithAnimation(boolean delayAnimation) {
assert !delayAnimation : "Animation delay should be false when switcher is toggled.";

float startAlpha = mIsInTabSwitcherMode ? 1f : 0f;
float endAlpha = mIsInTabSwitcherMode ? 0f : 1f;

if (mTabSwitcherModeAnimation != null) {
mTabSwitcherModeAnimation.cancel();
}

if (mIsInTabSwitcherMode) {
mLocationBar.setUrlBarFocusable(false);
updateShadowVisibility();
mIsInTabSwitcherMode = inTabSwitcherMode;
mSwitcherButton.setClickable(!inTabSwitcherMode);
int importantForAccessibility = inTabSwitcherMode
? View.IMPORTANT_FOR_ACCESSIBILITY_NO_HIDE_DESCENDANTS
: View.IMPORTANT_FOR_ACCESSIBILITY_AUTO;

mLocationBar.setUrlBarFocusable(!mIsInTabSwitcherMode);
if (getImportantForAccessibility() != importantForAccessibility) {
setImportantForAccessibility(importantForAccessibility);
sendAccessibilityEvent(AccessibilityEvent.TYPE_WINDOW_CONTENT_CHANGED);
}
setVisibility(View.VISIBLE);
setAlpha(startAlpha);

mTabSwitcherModeAnimation = ObjectAnimator.ofFloat(this, View.ALPHA, startAlpha, endAlpha);
mTabSwitcherModeAnimation.setDuration(
TopToolbarCoordinator.TAB_SWITCHER_MODE_NORMAL_ANIMATION_DURATION_MS);
mTabSwitcherModeAnimation.setInterpolator(Interpolators.LINEAR_INTERPOLATOR);
mTabSwitcherModeAnimation.addListener(new CancelAwareAnimatorListener() {
@Override
public void onEnd(Animator animator) {
int endVisibility = mIsInTabSwitcherMode ? View.GONE : View.VISIBLE;
setVisibility(endVisibility);
if (!mIsInTabSwitcherMode) {
mLocationBar.setUrlBarFocusable(true);
updateShadowVisibility();
}
}
});

mTabSwitcherModeAnimation.start();
}

@Override
Expand All @@ -633,19 +561,7 @@ public void destroy() {

@Override
void setTabCountProvider(TabCountProvider tabCountProvider) {
mAccessibilitySwitcherButton.setTabCountProvider(tabCountProvider);
}

@Override
void onAccessibilityStatusChanged(boolean enabled) {
enableTabStackButton(enabled);
}

@VisibleForTesting
void enableTabStackButton(boolean enabled) {
mShowTabStack = (enabled && isAccessibilityTabSwitcherPreferenceEnabled())
|| isGridTabSwitcherEnabled();
updateSwitcherButtonVisibility(mShowTabStack);
mSwitcherButton.setTabCountProvider(tabCountProvider);
}

@Override
Expand All @@ -655,7 +571,7 @@ void setBookmarkClickHandler(OnClickListener listener) {

@Override
void setOnTabSwitcherClickHandler(OnClickListener listener) {
mAccessibilitySwitcherButton.setOnTabSwitcherClickHandler(listener);
mSwitcherButton.setOnTabSwitcherClickHandler(listener);
}

@Override
Expand Down Expand Up @@ -881,11 +797,6 @@ ImageButton[] getToolbarButtons() {
return mToolbarButtons;
}

@VisibleForTesting
ObjectAnimator getTabSwitcherModeAnimation() {
return mTabSwitcherModeAnimation;
}

@VisibleForTesting
void enableButtonVisibilityChangeAnimationForTesting() {
mShouldAnimateButtonVisibilityChange = true;
Expand Down
78 changes: 0 additions & 78 deletions ...droid/toolbar/java/src/org/chromium/chrome/browser/toolbar/top/ToolbarTabletUnitTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@
import org.chromium.chrome.browser.omnibox.status.StatusCoordinator;
import org.chromium.chrome.browser.toolbar.menu_button.MenuButtonCoordinator;
import org.chromium.chrome.test.util.browser.Features;
import org.chromium.chrome.test.util.browser.Features.DisableFeatures;
import org.chromium.chrome.test.util.browser.Features.EnableFeatures;

import java.util.ArrayList;
Expand Down Expand Up @@ -148,49 +147,6 @@ public void onMeasureLargeWidthWithAnimation_showsToolbarButtons() {
}
}

@EnableFeatures(ChromeFeatureList.GRID_TAB_SWITCHER_FOR_TABLETS)
@Test
public void testSetTabSwitcherModeOn_hidesToolbar() {
assertEquals("Initial Toolbar visibility is not as expected", View.VISIBLE,
mToolbarTablet.getVisibility());
// Call
mToolbarTablet.setTabSwitcherMode(true, false, false, mMenuButtonCoordinator);
mToolbarTablet.getTabSwitcherModeAnimation().end();
assertEquals(
"Toolbar visibility is not as expected", View.GONE, mToolbarTablet.getVisibility());
verify(mLocationBar).setUrlBarFocusable(false);
}

@EnableFeatures(ChromeFeatureList.GRID_TAB_SWITCHER_FOR_TABLETS)
@Test
public void testSetTabSwitcherModeOn_tabSwitcherButtonDisabledInGTSMode() {
assertEquals("Initial Toolbar visibility is not as expected", View.VISIBLE,
mToolbarTablet.getVisibility());
// Call to enter GTS Mode
mToolbarTablet.setTabSwitcherMode(true, false, false, mMenuButtonCoordinator);
mToolbarTablet.getTabSwitcherModeAnimation().end();
assertFalse("Button should not be enabled in GTS Mode",
mToolbarTablet.findViewById(R.id.tab_switcher_button).isClickable());

// Leave GTS Mode
mToolbarTablet.setTabSwitcherMode(false, false, false, mMenuButtonCoordinator);
assertTrue("Button should be re-enabled after leaving GTS Mode",
mToolbarTablet.findViewById(R.id.tab_switcher_button).isClickable());
}

@EnableFeatures(ChromeFeatureList.GRID_TAB_SWITCHER_FOR_TABLETS)
@Test
public void testSetTabSwitcherModeOff_showsToolbar() {
// Hide toolbar as initial state.
mToolbarTablet.setVisibility(View.GONE);
// Call
mToolbarTablet.setTabSwitcherMode(false, false, false, mMenuButtonCoordinator);
mToolbarTablet.getTabSwitcherModeAnimation().end();
assertEquals("Toolbar visibility is not as expected", View.VISIBLE,
mToolbarTablet.getVisibility());
verify(mLocationBar).setUrlBarFocusable(true);
}

@EnableFeatures(ChromeFeatureList.GRID_TAB_SWITCHER_FOR_TABLETS)
@Test
public void testSetTabSwitcherPolishModeOff_toolbarStillVisible() {
Expand All @@ -217,40 +173,6 @@ public void testSetTabSwitcherPolishModeOn_toolbarStillVisible() {
verify(mLocationBar).setUrlBarFocusable(false);
}

@DisableFeatures(ChromeFeatureList.GRID_TAB_SWITCHER_FOR_TABLETS)
@Test
public void testSetTabSwitcherOnGTSDisabled_hidesViews() {
// Enable tab stack button when GTS is disabled
mToolbarTablet.enableTabStackButton(true);
when(mLocationBar.getContainerView()).thenReturn(mContainerView);
assertEquals("Initial Toolbar visibility is not as expected", View.VISIBLE,
mToolbarTablet.getVisibility());
// Call
mToolbarTablet.setTabSwitcherMode(true, false, false, mMenuButtonCoordinator);
assertFalse("Button should not be enabled",
mToolbarTablet.findViewById(R.id.back_button).isEnabled());
assertFalse("Button should not be enabled",
mToolbarTablet.findViewById(R.id.forward_button).isEnabled());
assertFalse("Button should not be enabled",
mToolbarTablet.findViewById(R.id.refresh_button).isEnabled());
verify(mContainerView).setVisibility(View.INVISIBLE);
verify(mMenuButtonCoordinator).setAppMenuUpdateBadgeSuppressed(true);
}

@DisableFeatures(ChromeFeatureList.GRID_TAB_SWITCHER_FOR_TABLETS)
@Test
public void testSetTabSwitcherOffGTSDisabled_showsViews() {
// Enable tab stack button when GTS is disabled
mToolbarTablet.enableTabStackButton(true);
when(mLocationBar.getContainerView()).thenReturn(mContainerView);
assertEquals("Initial Toolbar visibility is not as expected", View.VISIBLE,
mToolbarTablet.getVisibility());
// Call
mToolbarTablet.setTabSwitcherMode(false, false, false, mMenuButtonCoordinator);
verify(mContainerView).setVisibility(View.VISIBLE);
verify(mMenuButtonCoordinator).setAppMenuUpdateBadgeSuppressed(false);
}

@Test
public void testOnLongClick() {
longClickAndVerifyToast(R.id.refresh_button, R.string.refresh);
Expand Down
1 change: 0 additions & 1 deletion ...droid/toolbar/java/src/org/chromium/chrome/browser/toolbar/top/TopToolbarCoordinator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -482,7 +482,6 @@ public void updateBookmarkButton(boolean isBookmarked, boolean editingAllowed) {
* @param enabled Whether or not accessibility is enabled.
*/
public void onAccessibilityStatusChanged(boolean enabled) {
mToolbarLayout.onAccessibilityStatusChanged(enabled);
if (mTabSwitcherModeCoordinator != null) {
mTabSwitcherModeCoordinator.onAccessibilityStatusChanged(enabled);
} else if (mStartSurfaceToolbarCoordinator != null) {
Expand Down

0 comments on commit ccc169a

Please sign in to comment.