Commit deaf4305 authored by spdonghao's avatar spdonghao Committed by Commit Bot

Hide the placeholder of Feeds based on header status.

Bug: 1076139
Change-Id: I15ecbc7cbaeb6d0f1bcf316fd0c844d13452a5c7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2237715
Commit-Queue: Hao Dong <spdonghao@chromium.org>
Reviewed-by: default avatarHenrique Nakashima <hnakashima@chromium.org>
Reviewed-by: default avatarYaron Friedman <yfriedman@chromium.org>
Reviewed-by: default avatarCarlos Knippschild <carlosk@chromium.org>
Reviewed-by: default avatarWei-Yin Chen (陳威尹) <wychen@chromium.org>
Reviewed-by: default avatarXi Han <hanxi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#780048}
parent 1ee34734
......@@ -39,7 +39,7 @@
android:orientation="vertical"/>
<LinearLayout
android:id="@+id/images_layout"
android:id="@+id/placeholders_layout"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:orientation="vertical" />
......
......@@ -84,7 +84,7 @@ public class FeedLoadingLayout extends LinearLayout {
private void setPlaceholders() {
setPadding();
int currentOrientation = getResources().getConfiguration().orientation;
LinearLayout cardsParentView = (LinearLayout) findViewById(R.id.images_layout);
LinearLayout cardsParentView = (LinearLayout) findViewById(R.id.placeholders_layout);
setPlaceholders(cardsParentView, currentOrientation == Configuration.ORIENTATION_LANDSCAPE);
}
......
......@@ -10,8 +10,6 @@ import androidx.annotation.VisibleForTesting;
import org.chromium.base.ActivityState;
import org.chromium.base.ApplicationStatus;
import org.chromium.chrome.browser.ChromeActivity;
import org.chromium.chrome.browser.flags.CachedFeatureFlags;
import org.chromium.chrome.browser.flags.ChromeFeatureList;
import org.chromium.chrome.browser.tasks.TasksSurface;
import org.chromium.chrome.browser.tasks.TasksSurfaceProperties;
import org.chromium.chrome.browser.tasks.tab_management.TabManagementDelegate.TabSwitcherType;
......@@ -117,8 +115,7 @@ public class StartSurfaceCoordinator implements StartSurface {
StartSurfaceConfiguration.START_SURFACE_SHOW_STACK_TAB_SWITCHER.getValue());
// Show feed loading image.
if (mSurfaceMode == SurfaceMode.SINGLE_PANE
&& CachedFeatureFlags.isEnabled(ChromeFeatureList.INSTANT_START)) {
if (mStartSurfaceMediator.shouldShowFeedPlaceholder()) {
FeedLoadingCoordinator feedLoadingCoordinator = new FeedLoadingCoordinator(
mActivity, mTasksSurface.getBodyViewContainer(), false);
feedLoadingCoordinator.setUpLoadingView();
......
......@@ -494,7 +494,8 @@ class StartSurfaceMediator
&& mFeedSurfaceCreator != null) {
mPropertyModel.set(FEED_SURFACE_COORDINATOR,
mFeedSurfaceCreator.createFeedSurfaceCoordinator(
mNightModeStateProvider.isInNightMode(), getIsPlaceholderShown()));
mNightModeStateProvider.isInNightMode(),
shouldShowFeedPlaceholder()));
}
mTabModelSelector.addObserver(mTabModelSelectorObserver);
......@@ -611,9 +612,10 @@ class StartSurfaceMediator
setOverviewState(OverviewModeState.SHOWN_TABSWITCHER);
}
private boolean getIsPlaceholderShown() {
public boolean shouldShowFeedPlaceholder() {
return mSurfaceMode == SurfaceMode.SINGLE_PANE
&& CachedFeatureFlags.isEnabled(ChromeFeatureList.INSTANT_START);
&& CachedFeatureFlags.isEnabled(ChromeFeatureList.INSTANT_START)
&& StartSurfaceConfiguration.getFeedArticlesVisibility();
}
/** This interface builds the feed surface coordinator when showing if needed. */
......@@ -625,7 +627,7 @@ class StartSurfaceMediator
&& !mActivityStateChecker.isFinishingOrDestroyed()) {
mPropertyModel.set(FEED_SURFACE_COORDINATOR,
mFeedSurfaceCreator.createFeedSurfaceCoordinator(
mNightModeStateProvider.isInNightMode(), getIsPlaceholderShown()));
mNightModeStateProvider.isInNightMode(), shouldShowFeedPlaceholder()));
}
mPropertyModel.set(IS_EXPLORE_SURFACE_VISIBLE, isVisible);
......
......@@ -5,16 +5,21 @@
package org.chromium.chrome.features.start_surface;
import static androidx.test.espresso.Espresso.onView;
import static androidx.test.espresso.action.ViewActions.click;
import static androidx.test.espresso.assertion.ViewAssertions.doesNotExist;
import static androidx.test.espresso.assertion.ViewAssertions.matches;
import static androidx.test.espresso.matcher.ViewMatchers.isDisplayed;
import static androidx.test.espresso.matcher.ViewMatchers.withId;
import static androidx.test.espresso.matcher.ViewMatchers.withText;
import static com.google.common.truth.Truth.assertThat;
import static org.hamcrest.CoreMatchers.allOf;
import static org.hamcrest.Matchers.instanceOf;
import static org.chromium.chrome.browser.tabmodel.TestTabModelDirectory.M26_GOOGLE_COM;
import static org.chromium.chrome.test.util.ViewUtils.onViewWaiting;
import static org.chromium.chrome.test.util.ViewUtils.waitForView;
import android.content.Intent;
import android.graphics.Bitmap;
......@@ -23,9 +28,11 @@ import android.os.Build;
import android.text.TextUtils;
import android.util.Base64;
import android.view.View;
import android.view.ViewGroup;
import android.widget.ImageView;
import androidx.recyclerview.widget.RecyclerView;
import androidx.test.espresso.contrib.RecyclerViewActions;
import androidx.test.filters.SmallTest;
import org.hamcrest.core.AllOf;
......@@ -54,6 +61,8 @@ import org.chromium.chrome.browser.flags.ChromeFeatureList;
import org.chromium.chrome.browser.flags.ChromeSwitches;
import org.chromium.chrome.browser.homepage.HomepageManager;
import org.chromium.chrome.browser.ntp.cards.SignInPromo;
import org.chromium.chrome.browser.preferences.Pref;
import org.chromium.chrome.browser.preferences.PrefServiceBridge;
import org.chromium.chrome.browser.tab.TabState;
import org.chromium.chrome.browser.tabmodel.TabModelFilter;
import org.chromium.chrome.browser.tabmodel.TabPersistentStore;
......@@ -94,6 +103,7 @@ public class InstantStartTest {
// clang-format on
private static final String IMMEDIATE_RETURN_PARAMS = "force-fieldtrial-params=Study.Group:"
+ ReturnToChromeExperimentsUtil.TAB_SWITCHER_ON_RETURN_MS_PARAM + "/0";
private static final int ARTICLE_SECTION_HEADER_POSITION = 0;
private Bitmap mBitmap;
private int mThumbnailFetchCount;
......@@ -650,8 +660,104 @@ public class InstantStartTest {
startMainActivityFromLauncher();
Assert.assertFalse(mActivityTestRule.getActivity().isTablet());
Assert.assertTrue(CachedFeatureFlags.isEnabled(ChromeFeatureList.INSTANT_START));
onView(withId(org.chromium.chrome.start_surface.R.id.images_layout))
onView(withId(org.chromium.chrome.start_surface.R.id.placeholders_layout))
.check(matches(isDisplayed()));
Assert.assertFalse(LibraryLoader.getInstance().isInitialized());
}
@Test
@SmallTest
@Restriction({UiRestriction.RESTRICTION_TYPE_PHONE})
@EnableFeatures({ChromeFeatureList.TAB_SWITCHER_ON_RETURN + "<Study,",
ChromeFeatureList.START_SURFACE_ANDROID + "<Study"})
// clang-format off
@CommandLineFlags.Add({"force-fieldtrials=Study/Group",
IMMEDIATE_RETURN_PARAMS + "/start_surface_variation/single"})
public void testFeedPlaceholderVisibility() {
// clang-format on
startMainActivityFromLauncher();
// FEED_ARTICLES_LIST_VISIBLE should equal to ARTICLES_LIST_VISIBLE.
TestThreadUtils.runOnUiThreadBlocking(
()
-> Assert.assertEquals(PrefServiceBridge.getInstance().getBoolean(
Pref.ARTICLES_LIST_VISIBLE),
StartSurfaceConfiguration.getFeedArticlesVisibility()));
// Hide articles and verify that FEED_ARTICLES_LIST_VISIBLE and ARTICLES_LIST_VISIBLE are
// both false.
toggleHeader(false);
TestThreadUtils.runOnUiThreadBlocking(() -> {
boolean suggestionsVisible =
PrefServiceBridge.getInstance().getBoolean(Pref.ARTICLES_LIST_VISIBLE);
Assert.assertFalse(suggestionsVisible);
Assert.assertEquals(
suggestionsVisible, StartSurfaceConfiguration.getFeedArticlesVisibility());
});
// Show articles and verify that FEED_ARTICLES_LIST_VISIBLE and ARTICLES_LIST_VISIBLE are
// both true.
toggleHeader(true);
TestThreadUtils.runOnUiThreadBlocking(() -> {
boolean suggestionsVisible =
PrefServiceBridge.getInstance().getBoolean(Pref.ARTICLES_LIST_VISIBLE);
Assert.assertTrue(suggestionsVisible);
Assert.assertEquals(
suggestionsVisible, StartSurfaceConfiguration.getFeedArticlesVisibility());
});
}
@Test
@SmallTest
@Restriction({UiRestriction.RESTRICTION_TYPE_PHONE})
@EnableFeatures({ChromeFeatureList.TAB_SWITCHER_ON_RETURN + "<Study,",
ChromeFeatureList.START_SURFACE_ANDROID + "<Study"})
// clang-format off
@CommandLineFlags.Add({ChromeSwitches.DISABLE_NATIVE_INITIALIZATION,
"force-fieldtrials=Study/Group",
IMMEDIATE_RETURN_PARAMS + "/start_surface_variation/single"})
public void testHidePlaceholder() {
// clang-format on
StartSurfaceConfiguration.setFeedVisibilityForTesting(false);
startMainActivityFromLauncher();
onView(withId(org.chromium.chrome.start_surface.R.id.placeholders_layout))
.check(doesNotExist());
}
@Test
@SmallTest
@Restriction({UiRestriction.RESTRICTION_TYPE_PHONE})
@EnableFeatures({ChromeFeatureList.TAB_SWITCHER_ON_RETURN + "<Study,",
ChromeFeatureList.START_SURFACE_ANDROID + "<Study"})
// clang-format off
@CommandLineFlags.Add({ChromeSwitches.DISABLE_NATIVE_INITIALIZATION,
"force-fieldtrials=Study/Group",
IMMEDIATE_RETURN_PARAMS + "/start_surface_variation/single"})
public void testShowPlaceholder() {
// clang-format on
StartSurfaceConfiguration.setFeedVisibilityForTesting(true);
startMainActivityFromLauncher();
onView(withId(org.chromium.chrome.start_surface.R.id.placeholders_layout))
.check(matches(isDisplayed()));
}
/**
* Toggles the header and checks whether the header has the right status.
*
* @param expanded Whether the header should be expanded.
*/
private void toggleHeader(boolean expanded) {
onView(allOf(instanceOf(RecyclerView.class),
withId(org.chromium.chrome.feed.R.id.feed_stream_recycler_view)))
.perform(RecyclerViewActions.scrollToPosition(ARTICLE_SECTION_HEADER_POSITION),
RecyclerViewActions.actionOnItemAtPosition(
ARTICLE_SECTION_HEADER_POSITION, click()));
waitForView((ViewGroup) mActivityTestRule.getActivity().findViewById(
org.chromium.chrome.feed.R.id.feed_stream_recycler_view),
allOf(withId(org.chromium.chrome.R.id.header_status),
withText(expanded ? org.chromium.chrome.R.string.hide
: org.chromium.chrome.R.string.show)));
}
}
......@@ -4,12 +4,17 @@
package org.chromium.chrome.features.start_surface;
import androidx.annotation.VisibleForTesting;
import org.chromium.base.SysUtils;
import org.chromium.chrome.browser.flags.BooleanCachedFieldTrialParameter;
import org.chromium.chrome.browser.flags.CachedFeatureFlags;
import org.chromium.chrome.browser.flags.ChromeFeatureList;
import org.chromium.chrome.browser.flags.StringCachedFieldTrialParameter;
import org.chromium.chrome.browser.preferences.ChromePreferenceKeys;
import org.chromium.chrome.browser.preferences.Pref;
import org.chromium.chrome.browser.preferences.PrefChangeRegistrar;
import org.chromium.chrome.browser.preferences.PrefServiceBridge;
import org.chromium.chrome.browser.preferences.SharedPreferencesManager;
/**
......@@ -65,4 +70,35 @@ public class StartSurfaceConfiguration {
return isStartSurfaceSinglePaneEnabled()
&& START_SURFACE_SHOW_STACK_TAB_SWITCHER.getValue();
}
/**
* Add an observer to keep {@link ChromePreferenceKeys.FEED_ARTICLES_LIST_VISIBLE} consistent
* with {@link Pref.ARTICLES_LIST_VISIBLE}.
*/
public static void addFeedVisibilityObserver() {
updateFeedVisibility();
PrefChangeRegistrar prefChangeRegistrar = new PrefChangeRegistrar();
prefChangeRegistrar.addObserver(
Pref.ARTICLES_LIST_VISIBLE, StartSurfaceConfiguration::updateFeedVisibility);
}
private static void updateFeedVisibility() {
SharedPreferencesManager.getInstance().writeBoolean(
ChromePreferenceKeys.FEED_ARTICLES_LIST_VISIBLE,
PrefServiceBridge.getInstance().getBoolean(Pref.ARTICLES_LIST_VISIBLE));
}
/**
* @return Whether the Feed articles are visible.
*/
public static boolean getFeedArticlesVisibility() {
return SharedPreferencesManager.getInstance().readBoolean(
ChromePreferenceKeys.FEED_ARTICLES_LIST_VISIBLE, true);
}
@VisibleForTesting
static void setFeedVisibilityForTesting(boolean isVisible) {
SharedPreferencesManager.getInstance().writeBoolean(
ChromePreferenceKeys.FEED_ARTICLES_LIST_VISIBLE, isVisible);
}
}
......@@ -984,6 +984,7 @@ public class ChromeTabbedActivity extends ChromeActivity<ChromeActivityComponent
}
resetSavedInstanceState();
StartSurfaceConfiguration.addFeedVisibilityObserver();
}
@Override
......
......@@ -304,6 +304,12 @@ public final class ChromePreferenceKeys {
public static final String EXPLORE_OFFLINE_CONTENT_AVAILABILITY_STATUS =
"Chrome.NTPExploreOfflineCard.HasExploreOfflineContent";
/**
* The Feed articles visibility. This value is used as a pre-native cache and should be kept
* consistent with {@link Pref.ARTICLES_LIST_VISIBLE}.
*/
public static final String FEED_ARTICLES_LIST_VISIBLE = "Chrome.Feed.ArticlesListVisible";
public static final String FIRST_RUN_CACHED_TOS_ACCEPTED = "first_run_tos_accepted";
public static final String FIRST_RUN_FLOW_COMPLETE = "first_run_flow";
public static final String FIRST_RUN_FLOW_SIGNIN_ACCOUNT_NAME = "first_run_signin_account_name";
......@@ -772,6 +778,7 @@ public final class ChromePreferenceKeys {
DEFAULT_BROWSER_PROMO_IS_PROMOED,
DEFAULT_BROWSER_PROMO_SESSION_COUNT,
EXPLORE_OFFLINE_CONTENT_AVAILABILITY_STATUS,
FEED_ARTICLES_LIST_VISIBLE,
FLAGS_CACHED.pattern(),
FLAGS_CACHED_DUET_TABSTRIP_INTEGRATION_ANDROID_ENABLED,
FLAGS_FIELD_TRIAL_PARAM_CACHED.pattern(),
......
......@@ -13,6 +13,9 @@ namespace feed {
namespace prefs {
const char kEnableSnippets[] = "ntp_snippets.enable";
// A boolean pref set to true if Feed articles are visible.
// FEED_ARTICLES_LIST_VISIBLE in ChromePreferenceKeys.java is a pre-native cache
// and should be consistent with this pref.
const char kArticlesListVisible[] = "ntp_snippets.list_visible";
void RegisterFeedSharedProfilePrefs(PrefRegistrySimple* registry) {
......
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