Skip to content

Commit

Permalink
Revert "[Empty States] Add empty state illustrations to tab switcher UI"
Browse files Browse the repository at this point in the history
This reverts commit e108b3c.

Reason for revert: Caused issue where illustration would remain on screen on start surface. See https://crbug.com/1448283.

Original change's description:
> [Empty States] Add empty state illustrations to tab switcher UI
>
> - Add TabListEmptyCoordinator class to initialize empty state resources and use TabModelObserver and TabModelSelectorObserver to decide when to add /remove empty view.
>     Add empty state view when:
>     - a. On closing tab and normal tab count is going to be 0.
>     - b. On toggle tab model selector to normal tab model and normal tab count is 0.
>     - c On register tab switcher layout, add empty view when recycler is empty in TabListCoordinator.
>     Remove empty state view when:
>     - a. Adding a new tab.
>     - b. Undo a closed tab.
>     - c. On toggle tab model selector to go from normal tab model with 0 tab to incognito (We never apply empty view to incognito model).
>     - d. On unregister tab switcher layout.
>
> - Apply phone_tab_switcher_empty_state_illustration.xml to icon image view when form factor is phone.
> - Apply tablet_tab_switcher_empty_state_illustration.xml to icon image view when form factor is tablet.
>
> Phone demo: https://drive.google.com/file/d/17deVSRomQUQ1wj5Ml04FTM3tJ0ZXXKZo/view?usp=sharing
> Tablet demo: https://drive.google.com/file/d/1012QQcwxmBoqx7TUjOun7BlO6z2sP4o6/view?usp=sharing
>
> Change-Id: I43b89866ae612a05912b0256790cd538b38417f5
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4531843
> Reviewed-by: Theresa Sullivan <twellington@chromium.org>
> Commit-Queue: Theresa Sullivan <twellington@chromium.org>
> Code-Coverage: Findit <findit-for-me@appspot.gserviceaccount.com>
> Cr-Commit-Position: refs/heads/main@{#1147331}

(cherry picked from commit 900bddc)

Change-Id: I3e7f2bdbd89951c7e5b849fc41dd36d3b122ef30
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4557147
Commit-Queue: Neil Coronado <nemco@google.com>
Quick-Run: Neil Coronado <nemco@google.com>
Reviewed-by: Theresa Sullivan <twellington@chromium.org>
Commit-Queue: Theresa Sullivan <twellington@chromium.org>
Auto-Submit: Neil Coronado <nemco@google.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Original-Commit-Position: refs/heads/main@{#1148134}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4571340
Commit-Queue: Krishna Govind <govind@chromium.org>
Cr-Commit-Position: refs/branch-heads/5790@{#79}
Cr-Branched-From: 1d71a33-refs/heads/main@{#1148114}
  • Loading branch information
Neil Coronado authored and Chromium LUCI CQ committed May 26, 2023
1 parent 6d4d6a3 commit 26a3a02
Show file tree
Hide file tree
Showing 21 changed files with 9 additions and 242 deletions.
Expand Up @@ -100,8 +100,7 @@
Add({ChromeSwitches.DISABLE_FIRST_RUN_EXPERIENCE, "force-fieldtrials=Study/Group"})
@EnableFeatures({ChromeFeatureList.TAB_GRID_LAYOUT_ANDROID,
ChromeFeatureList.START_SURFACE_RETURN_TIME + "<Study,",
ChromeFeatureList.START_SURFACE_ANDROID + "<Study", ChromeFeatureList.INSTANT_START,
ChromeFeatureList.EMPTY_STATES})
ChromeFeatureList.START_SURFACE_ANDROID + "<Study", ChromeFeatureList.INSTANT_START})
@Restriction({Restriction.RESTRICTION_TYPE_NON_LOW_END_DEVICE,
UiRestriction.RESTRICTION_TYPE_PHONE})
@DoNotBatch(reason = "This test suite tests startup behaviours and thus can't be batched.")
Expand Down
Expand Up @@ -99,8 +99,7 @@
@CommandLineFlags.
Add({ChromeSwitches.DISABLE_FIRST_RUN_EXPERIENCE, "force-fieldtrials=Study/Group"})
@EnableFeatures({ChromeFeatureList.TAB_GRID_LAYOUT_ANDROID,
ChromeFeatureList.START_SURFACE_ANDROID, ChromeFeatureList.INSTANT_START,
ChromeFeatureList.EMPTY_STATES})
ChromeFeatureList.START_SURFACE_ANDROID, ChromeFeatureList.INSTANT_START})
@Restriction({Restriction.RESTRICTION_TYPE_NON_LOW_END_DEVICE,
UiRestriction.RESTRICTION_TYPE_PHONE})
@DoNotBatch(reason = "InstantStartTest tests startup behaviours and thus can't be batched.")
Expand Down
Expand Up @@ -78,8 +78,7 @@
@UseRunnerDelegate(ChromeJUnit4RunnerDelegate.class)
@Restriction(
{UiRestriction.RESTRICTION_TYPE_PHONE, Restriction.RESTRICTION_TYPE_NON_LOW_END_DEVICE})
@EnableFeatures(
{ChromeFeatureList.START_SURFACE_ANDROID + "<Study", ChromeFeatureList.EMPTY_STATES})
@EnableFeatures({ChromeFeatureList.START_SURFACE_ANDROID + "<Study"})
@DoNotBatch(reason = "StartSurface*Test tests startup behaviours and thus can't be batched.")
@CommandLineFlags.
Add({ChromeSwitches.DISABLE_FIRST_RUN_EXPERIENCE, "force-fieldtrials=Study/Group"})
Expand Down Expand Up @@ -122,6 +121,7 @@ public StartSurfaceBackButtonTest(boolean useInstantStart, boolean immediateRetu
@Before
public void setUp() throws IOException {
StartSurfaceTestUtils.setUpStartSurfaceTests(mImmediateReturn, mActivityTestRule);

mLayoutChangedCallbackHelper = new CallbackHelper();

if (isInstantReturn()) {
Expand Down
Expand Up @@ -77,8 +77,7 @@
@UseRunnerDelegate(ChromeJUnit4RunnerDelegate.class)
@Restriction(
{UiRestriction.RESTRICTION_TYPE_PHONE, Restriction.RESTRICTION_TYPE_NON_LOW_END_DEVICE})
@EnableFeatures(
{ChromeFeatureList.START_SURFACE_ANDROID + "<Study", ChromeFeatureList.EMPTY_STATES})
@EnableFeatures({ChromeFeatureList.START_SURFACE_ANDROID + "<Study"})
@DoNotBatch(reason = "StartSurface*Test tests startup behaviours and thus can't be batched.")
@CommandLineFlags.
Add({ChromeSwitches.DISABLE_FIRST_RUN_EXPERIENCE, "force-fieldtrials=Study/Group"})
Expand Down
Expand Up @@ -78,8 +78,7 @@
@UseRunnerDelegate(ChromeJUnit4RunnerDelegate.class)
@Restriction(
{UiRestriction.RESTRICTION_TYPE_PHONE, Restriction.RESTRICTION_TYPE_NON_LOW_END_DEVICE})
@EnableFeatures(
{ChromeFeatureList.START_SURFACE_ANDROID + "<Study", ChromeFeatureList.EMPTY_STATES})
@EnableFeatures({ChromeFeatureList.START_SURFACE_ANDROID + "<Study"})
@DoNotBatch(reason = "StartSurface*Test tests startup behaviours and thus can't be batched.")
@CommandLineFlags.
Add({ChromeSwitches.DISABLE_FIRST_RUN_EXPERIENCE, "force-fieldtrials=Study/Group"})
Expand Down
Expand Up @@ -116,8 +116,7 @@
@UseRunnerDelegate(ChromeJUnit4RunnerDelegate.class)
@Restriction(
{UiRestriction.RESTRICTION_TYPE_PHONE, Restriction.RESTRICTION_TYPE_NON_LOW_END_DEVICE})
@EnableFeatures(
{ChromeFeatureList.START_SURFACE_ANDROID + "<Study", ChromeFeatureList.EMPTY_STATES})
@EnableFeatures({ChromeFeatureList.START_SURFACE_ANDROID + "<Study"})
@CommandLineFlags.
Add({ChromeSwitches.DISABLE_FIRST_RUN_EXPERIENCE, "force-fieldtrials=Study/Group"})
@DoNotBatch(reason = "This test suite tests startup behaviors.")
Expand Down
2 changes: 0 additions & 2 deletions chrome/android/features/tab_ui/BUILD.gn
Expand Up @@ -41,7 +41,6 @@ android_resources("java_resources") {
"java/res/drawable/ic_select_all_24dp.xml",
"java/res/drawable/iph_drag_and_drop_animated_drawable.xml",
"java/res/drawable/iph_drag_and_drop_drawable.xml",
"java/res/drawable/phone_tab_switcher_empty_state_illustration.xml",
"java/res/drawable/price_card_background.xml",
"java/res/drawable/price_card_scrim.xml",
"java/res/drawable/selected_tab_background.xml",
Expand All @@ -55,7 +54,6 @@ android_resources("java_resources") {
"java/res/drawable/tab_strip_favicon_circle.xml",
"java/res/drawable/tab_strip_selected_ring.xml",
"java/res/drawable/tab_strip_selected_ring_incognito.xml",
"java/res/drawable/tablet_tab_switcher_empty_state_illustration.xml",
"java/res/drawable/tabstrip_favicon_background.xml",
"java/res/drawable/ungroup_bar_background.xml",
"java/res/layout/bottom_tab_grid_toolbar.xml",
Expand Down

This file was deleted.

This file was deleted.

Expand Up @@ -30,7 +30,6 @@

import org.chromium.base.Callback;
import org.chromium.base.TraceEvent;
import org.chromium.chrome.browser.flags.ChromeFeatureList;
import org.chromium.chrome.browser.lifecycle.DestroyObserver;
import org.chromium.chrome.browser.tab.Tab;
import org.chromium.chrome.browser.tab.TabUtils;
Expand Down Expand Up @@ -82,7 +81,6 @@ public class TabListCoordinator
static final int GRID_LAYOUT_SPAN_COUNT_LARGE = 4;
static final int MAX_SCREEN_WIDTH_COMPACT_DP = 600;
static final int MAX_SCREEN_WIDTH_MEDIUM_DP = 800;

private final TabListMediator mMediator;
private final TabListRecyclerView mRecyclerView;
private final SimpleRecyclerViewAdapter mAdapter;
Expand All @@ -99,7 +97,6 @@ public class TabListCoordinator
private @Nullable TabStripSnapshotter mTabStripSnapshotter;
private ItemTouchHelper mItemTouchHelper;
private OnItemTouchListener mOnItemTouchListener;
private TabListEmptyCoordinator mTabListEmptyCoordinator;

/**
* Construct a coordinator for UI that shows a list of tabs.
Expand Down Expand Up @@ -144,7 +141,6 @@ public class TabListCoordinator
mModel = new TabListModel();
mAdapter = new SimpleRecyclerViewAdapter(mModel);
mRootView = rootView;

RecyclerView.RecyclerListener recyclerListener = null;
if (mMode == TabListMode.GRID || mMode == TabListMode.CAROUSEL) {
mAdapter.registerType(UiType.SELECTABLE, parent -> {
Expand Down Expand Up @@ -223,10 +219,6 @@ public class TabListCoordinator
selectionDelegateProvider, gridCardOnClickListenerProvider, dialogHandler,
priceWelcomeMessageController, componentName, itemType);

if (ChromeFeatureList.sEmptyStates.isEnabled()) {
mTabListEmptyCoordinator = new TabListEmptyCoordinator(rootView, mModel);
}

try (TraceEvent e = TraceEvent.scoped("TabListCoordinator.setupRecyclerView")) {
if (!attachToParent) {
mRecyclerView = (TabListRecyclerView) LayoutInflater.from(context).inflate(
Expand Down Expand Up @@ -538,9 +530,6 @@ void postHiding() {
@Override
public void onDestroy() {
mMediator.destroy();
if (mTabListEmptyCoordinator != null) {
mTabListEmptyCoordinator.destroy();
}
if (mListLayoutListener != null) {
mRecyclerView.removeOnLayoutChangeListener(mListLayoutListener);
mLayoutListenerRegistered = false;
Expand Down

This file was deleted.

Expand Up @@ -148,7 +148,6 @@
@CommandLineFlags.Add({ChromeSwitches.DISABLE_FIRST_RUN_EXPERIENCE,
"force-fieldtrials=Study/Group"})
@EnableFeatures({ChromeFeatureList.TAB_GRID_LAYOUT_ANDROID + "<Study"})
@DisableFeatures({ChromeFeatureList.EMPTY_STATES})
@Restriction(
{UiRestriction.RESTRICTION_TYPE_PHONE, Restriction.RESTRICTION_TYPE_NON_LOW_END_DEVICE})
public class TabSwitcherAndStartSurfaceLayoutTest {
Expand Down
Expand Up @@ -88,8 +88,8 @@
@RunWith(ChromeJUnit4ClassRunner.class)
@CommandLineFlags.
Add({ChromeSwitches.DISABLE_FIRST_RUN_EXPERIENCE, "force-fieldtrials=Study/Group"})
@EnableFeatures({ChromeFeatureList.TAB_STRIP_REDESIGN, ChromeFeatureList.EMPTY_STATES})
@DisableFeatures({ChromeFeatureList.TAB_TO_GTS_ANIMATION})
@EnableFeatures({ChromeFeatureList.TAB_STRIP_REDESIGN})
@DisableFeatures(ChromeFeatureList.TAB_TO_GTS_ANIMATION)
@Restriction(
{Restriction.RESTRICTION_TYPE_NON_LOW_END_DEVICE, UiRestriction.RESTRICTION_TYPE_TABLET})
@Batch(Batch.PER_CLASS)
Expand Down
Expand Up @@ -67,7 +67,6 @@ internal_tab_management_java_sources = [
"//chrome/android/features/tab_ui/java/src/org/chromium/chrome/browser/tasks/tab_management/TabListContainerProperties.java",
"//chrome/android/features/tab_ui/java/src/org/chromium/chrome/browser/tasks/tab_management/TabListContainerViewBinder.java",
"//chrome/android/features/tab_ui/java/src/org/chromium/chrome/browser/tasks/tab_management/TabListCoordinator.java",
"//chrome/android/features/tab_ui/java/src/org/chromium/chrome/browser/tasks/tab_management/TabListEmptyCoordinator.java",
"//chrome/android/features/tab_ui/java/src/org/chromium/chrome/browser/tasks/tab_management/TabListMediator.java",
"//chrome/android/features/tab_ui/java/src/org/chromium/chrome/browser/tasks/tab_management/TabListModel.java",
"//chrome/android/features/tab_ui/java/src/org/chromium/chrome/browser/tasks/tab_management/TabListRecyclerView.java",
Expand Down
Expand Up @@ -68,7 +68,6 @@ public void cacheNativeFlags() {
CachedFeatureFlags.cacheNativeFlags(ChromeFeatureList.sFlagsCachedFullBrowser);
CachedFeatureFlags.cacheAdditionalNativeFlags();

//clang-format off
List<CachedFieldTrialParameter> fieldTrialsToCache = List.of(
BrandingController.BRANDING_CADENCE_MS,
BrandingController.MAX_BLANK_TOOLBAR_TIMEOUT_MS,
Expand Down
Expand Up @@ -529,7 +529,6 @@ public static Map<String, String> getFieldTrialParamsForFeature(String featureNa
new CachedFlag(DELAY_TEMP_STRIP_REMOVAL, true);
public static final CachedFlag sDiscoverMultiColumn = new CachedFlag(FEED_MULTI_COLUMN, true);
public static final CachedFlag sEarlyLibraryLoad = new CachedFlag(EARLY_LIBRARY_LOAD, true);
public static final CachedFlag sEmptyStates = new CachedFlag(EMPTY_STATES, true);
public static final CachedFlag sExperimentsForAgsa = new CachedFlag(EXPERIMENTS_FOR_AGSA, true);
public static final CachedFlag sFeedLoadingPlaceholder =
new CachedFlag(FEED_LOADING_PLACEHOLDER, false);
Expand Down Expand Up @@ -628,7 +627,6 @@ public static Map<String, String> getFieldTrialParamsForFeature(String featureNa
sDelayTempStripRemoval,
sDiscoverMultiColumn,
sEarlyLibraryLoad,
sEmptyStates,
sFeedLoadingPlaceholder,
sFoldableJankFix,
sHideNonDisplayableAccountEmail,
Expand Down

0 comments on commit 26a3a02

Please sign in to comment.