Commit 6686599e authored by Jeffrey Cohen's avatar Jeffrey Cohen Committed by Commit Bot

[ShareInToolbar] Minimum width driven via finch config and orientation

For the experimental share button in the toolbar:
  Add a requirement for devices to have a min width, and use finch param.
  Fix Issue with orientation, where button was not being updated on
    orientation change.
  Update Tests to respect device width.


Bug: 1036023
Change-Id: If99bb41b5ae9d5b35e416c338d5a8355ba91949e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2112753Reviewed-by: default avatarHenrique Nakashima <hnakashima@chromium.org>
Reviewed-by: default avatarTheresa  <twellington@chromium.org>
Reviewed-by: default avatarPatrick Noland <pnoland@chromium.org>
Reviewed-by: default avatarShakti Sahu <shaktisahu@chromium.org>
Commit-Queue: Jeffrey Cohen <jeffreycohen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#755012}
parent 0e66250e
......@@ -56,7 +56,6 @@ public class ChromeCachedFlags {
ChromeFeatureList.PAINT_PREVIEW_DEMO,
ChromeFeatureList.PRIORITIZE_BOOTSTRAP_TASKS,
ChromeFeatureList.INSTANT_START,
ChromeFeatureList.SHARE_BUTTON_IN_TOP_TOOLBAR,
ChromeFeatureList.START_SURFACE_ANDROID,
ChromeFeatureList.SWAP_PIXEL_FORMAT_TO_FIX_CONVERT_FROM_TRANSLUCENT,
ChromeFeatureList.TAB_GROUPS_CONTINUATION_ANDROID,
......
......@@ -4,7 +4,9 @@
package org.chromium.chrome.browser.share;
import android.content.ComponentCallbacks;
import android.content.Context;
import android.content.res.Configuration;
import android.view.View.OnClickListener;
import androidx.appcompat.content.res.AppCompatResources;
......@@ -14,7 +16,6 @@ import org.chromium.base.metrics.RecordUserAction;
import org.chromium.base.supplier.ObservableSupplier;
import org.chromium.chrome.R;
import org.chromium.chrome.browser.ActivityTabProvider;
import org.chromium.chrome.browser.flags.CachedFeatureFlags;
import org.chromium.chrome.browser.flags.ChromeFeatureList;
import org.chromium.chrome.browser.tab.Tab;
import org.chromium.chrome.browser.toolbar.ButtonData;
......@@ -26,6 +27,11 @@ import org.chromium.chrome.browser.toolbar.bottom.BottomToolbarVariationManager;
* whether NTP is shown).
*/
public class ShareButtonController implements ButtonDataProvider {
/**
* Default minimum width to show the share button.
*/
public static final int MIN_WIDTH_DP = 360;
// Context is used for fetching resources and launching preferences page.
private final Context mContext;
......@@ -33,6 +39,8 @@ public class ShareButtonController implements ButtonDataProvider {
private final ObservableSupplier<ShareDelegate> mShareDelegateSupplier;
private final ComponentCallbacks mComponentCallbacks;
// The activity tab provider.
private ActivityTabProvider mTabProvider;
......@@ -41,6 +49,11 @@ public class ShareButtonController implements ButtonDataProvider {
private final ObservableSupplier<Boolean> mBottomToolbarVisibilitySupplier;
private OnClickListener mOnClickListener;
private Integer mMinimumWidthDp;
private int mScreenWidthDp;
private int mCurrentOrientation;
/**
* Creates ShareButtonController object.
* @param context The Context for retrieving resources, etc.
......@@ -74,13 +87,32 @@ public class ShareButtonController implements ButtonDataProvider {
shareDelegate.share(tab, /*shareDirectly=*/false);
});
mComponentCallbacks = new ComponentCallbacks() {
@Override
public void onConfigurationChanged(Configuration configuration) {
int newOrientation = configuration.orientation;
if (newOrientation == mCurrentOrientation) return;
mCurrentOrientation = newOrientation;
mScreenWidthDp = configuration.screenWidthDp;
updateButtonVisibility(mTabProvider.get());
notifyObservers(mButtonData.canShow);
}
@Override
public void onLowMemory() {}
};
mContext.registerComponentCallbacks(mComponentCallbacks);
mButtonData = new ButtonData(false,
AppCompatResources.getDrawable(mContext, R.drawable.ic_toolbar_share_24dp),
mOnClickListener, R.string.share, true, null);
mScreenWidthDp = mContext.getResources().getConfiguration().screenWidthDp;
}
@Override
public void destroy() {}
public void destroy() {
mContext.unregisterComponentCallbacks(mComponentCallbacks);
}
@Override
public void addObserver(ButtonDataObserver obs) {
......@@ -94,16 +126,27 @@ public class ShareButtonController implements ButtonDataProvider {
@Override
public ButtonData get(Tab tab) {
updateButtonState(tab);
updateButtonVisibility(tab);
return mButtonData;
}
private void updateButtonState(Tab tab) {
// TODO(crbug.com/1036023) add width constraints.
if (!CachedFeatureFlags.isEnabled(ChromeFeatureList.SHARE_BUTTON_IN_TOP_TOOLBAR)
|| (mBottomToolbarVisibilitySupplier.get()
&& BottomToolbarVariationManager.isShareButtonOnBottom())
|| mShareDelegateSupplier.get() == null) {
private void updateButtonVisibility(Tab tab) {
if (tab == null || tab.getWebContents() == null
|| !ChromeFeatureList.isEnabled(ChromeFeatureList.SHARE_BUTTON_IN_TOP_TOOLBAR)) {
mButtonData.canShow = false;
return;
}
if (mMinimumWidthDp == null) {
mMinimumWidthDp = ChromeFeatureList.getFieldTrialParamByFeatureAsInt(
ChromeFeatureList.SHARE_BUTTON_IN_TOP_TOOLBAR, "minimum_width", MIN_WIDTH_DP);
}
boolean isDeviceWideEnough = mScreenWidthDp > mMinimumWidthDp;
if ((mBottomToolbarVisibilitySupplier.get()
&& BottomToolbarVariationManager.isShareButtonOnBottom())
|| mShareDelegateSupplier.get() == null || !isDeviceWideEnough) {
mButtonData.canShow = false;
return;
}
......
......@@ -44,10 +44,20 @@ public final class ShareButtonControllerTest {
@Rule
public ChromeTabbedActivityTestRule mActivityTestRule = new ChromeTabbedActivityTestRule();
private boolean mButtonExpected;
@Before
public void setUp() {
SigninTestUtil.setUpAuthForTest();
mActivityTestRule.startMainActivityOnBlankPage();
int minimumWidthDp = ChromeFeatureList.getFieldTrialParamByFeatureAsInt(
ChromeFeatureList.SHARE_BUTTON_IN_TOP_TOOLBAR, "minimum_width",
ShareButtonController.MIN_WIDTH_DP);
int deviceWidth =
mActivityTestRule.getActivity().getResources().getConfiguration().screenWidthDp;
mButtonExpected = deviceWidth > minimumWidthDp;
}
@After
......@@ -64,8 +74,13 @@ public final class ShareButtonControllerTest {
.getToolbarManager()
.getToolbarLayoutForTesting()
.getOptionalButtonView();
assertNotNull("experimental button not found", experimentalButton);
assertEquals(View.GONE, experimentalButton.getVisibility());
if (experimentalButton != null) {
String shareString =
mActivityTestRule.getActivity().getResources().getString(R.string.share);
assertTrue("Share button isnt showing",
(View.GONE == experimentalButton.getVisibility()
|| !shareString.equals(experimentalButton.getContentDescription())));
}
}
@Test
......@@ -76,12 +91,17 @@ public final class ShareButtonControllerTest {
.getToolbarLayoutForTesting()
.getOptionalButtonView();
assertNotNull("experimental button not found", experimentalButton);
assertEquals(View.VISIBLE, experimentalButton.getVisibility());
String shareString =
mActivityTestRule.getActivity().getResources().getString(R.string.share);
assertTrue(shareString.equals(experimentalButton.getContentDescription()));
if (!mButtonExpected) {
assertTrue(
experimentalButton == null || View.GONE == experimentalButton.getVisibility());
} else {
assertNotNull("experimental button not found", experimentalButton);
assertEquals(View.VISIBLE, experimentalButton.getVisibility());
String shareString =
mActivityTestRule.getActivity().getResources().getString(R.string.share);
assertTrue(shareString.equals(experimentalButton.getContentDescription()));
}
}
@Test
......@@ -106,12 +126,16 @@ public final class ShareButtonControllerTest {
.getToolbarManager()
.getToolbarLayoutForTesting()
.getOptionalButtonView();
assertNotNull("optional button not found", optionalButton);
if (!mButtonExpected) {
assertTrue(optionalButton == null || View.GONE == optionalButton.getVisibility());
} else {
assertNotNull("optional button not found", optionalButton);
String shareString =
mActivityTestRule.getActivity().getResources().getString(R.string.share);
String shareString =
mActivityTestRule.getActivity().getResources().getString(R.string.share);
assertEquals(shareString, optionalButton.getContentDescription());
assertEquals(shareString, optionalButton.getContentDescription());
}
}
@Test
......@@ -126,8 +150,13 @@ public final class ShareButtonControllerTest {
.getToolbarManager()
.getToolbarLayoutForTesting()
.getOptionalButtonView();
assertNotNull("experimental button not found", experimentalButton);
assertEquals(View.GONE, experimentalButton.getVisibility());
if (experimentalButton != null) {
String shareString =
mActivityTestRule.getActivity().getResources().getString(R.string.share);
assertTrue("Share button isnt showing",
(View.GONE == experimentalButton.getVisibility()
|| !shareString.equals(experimentalButton.getContentDescription())));
}
}
// TODO(crbug/1036023) Add a test that checks that expected intents are fired.
......
......@@ -65,7 +65,6 @@ public class CachedFeatureFlags {
put(ChromeFeatureList.TAB_GROUPS_ANDROID, false);
put(ChromeFeatureList.TAB_GROUPS_CONTINUATION_ANDROID, false);
put(ChromeFeatureList.DUET_TABSTRIP_INTEGRATION_ANDROID, false);
put(ChromeFeatureList.SHARE_BUTTON_IN_TOP_TOOLBAR, false);
put(ChromeFeatureList.CLOSE_TAB_SUGGESTIONS, false);
put(ChromeFeatureList.INSTANT_START, false);
put(ChromeFeatureList.TAB_GROUPS_CONTINUATION_ANDROID, false);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment