Commit 5f389b81 authored by Nicolas Dossou-gbete's avatar Nicolas Dossou-gbete Committed by Commit Bot

📰 Show placeholder card on loads from empty sections

Preview: https://photos.app.goo.gl/2NJzRQ31Js9ltKeC3
Bug: 762932
Change-Id: I47d47928cf4f1708bfc1ea96689e81362e166ca8
Reviewed-on: https://chromium-review.googlesource.com/664578Reviewed-by: default avatarBernhard Bauer <bauerb@chromium.org>
Reviewed-by: default avatarJan Krcal <jkrcal@chromium.org>
Commit-Queue: Nicolas Dossou-Gbété <dgn@chromium.org>
Cr-Commit-Position: refs/heads/master@{#504407}
parent 5b51043c
...@@ -175,7 +175,7 @@ public abstract class CardViewHolder ...@@ -175,7 +175,7 @@ public abstract class CardViewHolder
* with data. * with data.
*/ */
@CallSuper @CallSuper
protected void onBindViewHolder() { public void onBindViewHolder() {
// Reset the transparency and translation in case a dismissed card is being recycled. // Reset the transparency and translation in case a dismissed card is being recycled.
itemView.setAlpha(1f); itemView.setAlpha(1f);
itemView.setTranslationX(0f); itemView.setTranslationX(0f);
......
...@@ -20,7 +20,7 @@ import java.lang.annotation.RetentionPolicy; ...@@ -20,7 +20,7 @@ import java.lang.annotation.RetentionPolicy;
@IntDef({ItemViewType.ABOVE_THE_FOLD, ItemViewType.SITE_SECTION, ItemViewType.HEADER, @IntDef({ItemViewType.ABOVE_THE_FOLD, ItemViewType.SITE_SECTION, ItemViewType.HEADER,
ItemViewType.SNIPPET, ItemViewType.SPACING, ItemViewType.STATUS, ItemViewType.PROGRESS, ItemViewType.SNIPPET, ItemViewType.SPACING, ItemViewType.STATUS, ItemViewType.PROGRESS,
ItemViewType.ACTION, ItemViewType.FOOTER, ItemViewType.PROMO, ItemViewType.ALL_DISMISSED, ItemViewType.ACTION, ItemViewType.FOOTER, ItemViewType.PROMO, ItemViewType.ALL_DISMISSED,
ItemViewType.CAROUSEL}) ItemViewType.CAROUSEL, ItemViewType.PLACEHOLDER_CARD})
@Retention(RetentionPolicy.SOURCE) @Retention(RetentionPolicy.SOURCE)
public @interface ItemViewType { public @interface ItemViewType {
/** /**
...@@ -96,4 +96,11 @@ public @interface ItemViewType { ...@@ -96,4 +96,11 @@ public @interface ItemViewType {
* @see Adapter#getItemViewType(int) * @see Adapter#getItemViewType(int)
*/ */
int CAROUSEL = 12; int CAROUSEL = 12;
/**
* View type for a {@link org.chromium.chrome.browser.suggestions.ContentSuggestionPlaceholder}.
*
* @see Adapter#getItemViewType(int)
*/
int PLACEHOLDER_CARD = 13;
} }
...@@ -23,6 +23,7 @@ import org.chromium.chrome.browser.ntp.snippets.SnippetArticleViewHolder; ...@@ -23,6 +23,7 @@ import org.chromium.chrome.browser.ntp.snippets.SnippetArticleViewHolder;
import org.chromium.chrome.browser.ntp.snippets.SnippetsBridge; import org.chromium.chrome.browser.ntp.snippets.SnippetsBridge;
import org.chromium.chrome.browser.ntp.snippets.SuggestionsSource; import org.chromium.chrome.browser.ntp.snippets.SuggestionsSource;
import org.chromium.chrome.browser.offlinepages.OfflinePageBridge; import org.chromium.chrome.browser.offlinepages.OfflinePageBridge;
import org.chromium.chrome.browser.suggestions.ContentSuggestionPlaceholder;
import org.chromium.chrome.browser.suggestions.DestructionObserver; import org.chromium.chrome.browser.suggestions.DestructionObserver;
import org.chromium.chrome.browser.suggestions.SiteSection; import org.chromium.chrome.browser.suggestions.SiteSection;
import org.chromium.chrome.browser.suggestions.SuggestionsCarousel; import org.chromium.chrome.browser.suggestions.SuggestionsCarousel;
...@@ -185,6 +186,10 @@ public class NewTabPageAdapter extends Adapter<NewTabPageViewHolder> implements ...@@ -185,6 +186,10 @@ public class NewTabPageAdapter extends Adapter<NewTabPageViewHolder> implements
case ItemViewType.CAROUSEL: case ItemViewType.CAROUSEL:
return new SuggestionsCarousel.ViewHolder(mRecyclerView); return new SuggestionsCarousel.ViewHolder(mRecyclerView);
case ItemViewType.PLACEHOLDER_CARD:
return new ContentSuggestionPlaceholder.ViewHolder(
mRecyclerView, mUiConfig, mContextMenuManager);
} }
assert false : viewType; assert false : viewType;
...@@ -347,7 +352,7 @@ public class NewTabPageAdapter extends Adapter<NewTabPageViewHolder> implements ...@@ -347,7 +352,7 @@ public class NewTabPageAdapter extends Adapter<NewTabPageViewHolder> implements
// In the modern layout, we only consider articles. // In the modern layout, we only consider articles.
SuggestionsSection suggestions = mSections.getSection(KnownCategories.ARTICLES); SuggestionsSection suggestions = mSections.getSection(KnownCategories.ARTICLES);
return suggestions == null || !suggestions.hasSuggestions(); return suggestions == null || !suggestions.hasCards();
} }
private int getChildPositionOffset(TreeNode child) { private int getChildPositionOffset(TreeNode child) {
......
...@@ -74,4 +74,9 @@ public class NodeVisitor { ...@@ -74,4 +74,9 @@ public class NodeVisitor {
* @param adapter The adapter that holds all information displayed in the carousel. * @param adapter The adapter that holds all information displayed in the carousel.
*/ */
public void visitCarouselItem(SuggestionsCarouselAdapter adapter) {} public void visitCarouselItem(SuggestionsCarouselAdapter adapter) {}
/**
* Visits a card placeholder item.
*/
public void visitPlaceholderItem() {}
} }
...@@ -13,7 +13,6 @@ import org.chromium.chrome.browser.ntp.snippets.SnippetArticle; ...@@ -13,7 +13,6 @@ import org.chromium.chrome.browser.ntp.snippets.SnippetArticle;
import org.chromium.chrome.browser.ntp.snippets.SnippetsBridge; import org.chromium.chrome.browser.ntp.snippets.SnippetsBridge;
import org.chromium.chrome.browser.ntp.snippets.SuggestionsSource; import org.chromium.chrome.browser.ntp.snippets.SuggestionsSource;
import org.chromium.chrome.browser.offlinepages.OfflinePageBridge; import org.chromium.chrome.browser.offlinepages.OfflinePageBridge;
import org.chromium.chrome.browser.suggestions.DestructionObserver;
import org.chromium.chrome.browser.suggestions.SuggestionsRanker; import org.chromium.chrome.browser.suggestions.SuggestionsRanker;
import org.chromium.chrome.browser.suggestions.SuggestionsUiDelegate; import org.chromium.chrome.browser.suggestions.SuggestionsUiDelegate;
import org.chromium.chrome.browser.util.FeatureUtilities; import org.chromium.chrome.browser.util.FeatureUtilities;
...@@ -47,12 +46,7 @@ public class SectionList ...@@ -47,12 +46,7 @@ public class SectionList
mUiDelegate.getSuggestionsSource().addObserver(this); mUiDelegate.getSuggestionsSource().addObserver(this);
mOfflinePageBridge = offlinePageBridge; mOfflinePageBridge = offlinePageBridge;
mUiDelegate.addDestructionObserver(new DestructionObserver() { mUiDelegate.addDestructionObserver(this::removeAllSections);
@Override
public void onDestroy() {
removeAllSections();
}
});
} }
/** /**
...@@ -94,7 +88,8 @@ public class SectionList ...@@ -94,7 +88,8 @@ public class SectionList
SuggestionsSection section = mSections.get(category); SuggestionsSection section = mSections.get(category);
// Do not show an empty section if not allowed. // Do not show an empty section if not allowed.
if (suggestions.isEmpty() && !info.showIfEmpty() && !alwaysAllowEmptySections) { if (suggestions.isEmpty() && !info.showIfEmpty() && !alwaysAllowEmptySections
&& !SnippetsBridge.isCategoryLoading(categoryStatus)) {
mBlacklistedCategories.add(category); mBlacklistedCategories.add(category);
if (section != null) removeSection(section); if (section != null) removeSection(section);
return; return;
...@@ -116,7 +111,9 @@ public class SectionList ...@@ -116,7 +111,9 @@ public class SectionList
// Set the new suggestions. // Set the new suggestions.
section.setStatus(categoryStatus); section.setStatus(categoryStatus);
section.appendSuggestions(suggestions, /* keepSectionSize = */ true); if (!section.isLoading()) {
section.appendSuggestions(suggestions, /* keepSectionSize = */ true);
}
} }
@Override @Override
......
...@@ -336,7 +336,7 @@ public class SignInPromo extends OptionalLeaf implements ImpressionTracker.Liste ...@@ -336,7 +336,7 @@ public class SignInPromo extends OptionalLeaf implements ImpressionTracker.Liste
} }
@Override @Override
protected void onBindViewHolder() { public void onBindViewHolder() {
super.onBindViewHolder(); super.onBindViewHolder();
updatePersonalizedSigninPromo(); updatePersonalizedSigninPromo();
} }
......
...@@ -6,6 +6,7 @@ package org.chromium.chrome.browser.ntp.cards; ...@@ -6,6 +6,7 @@ package org.chromium.chrome.browser.ntp.cards;
import android.support.annotation.CallSuper; import android.support.annotation.CallSuper;
import android.support.annotation.NonNull; import android.support.annotation.NonNull;
import android.support.annotation.Nullable;
import android.text.TextUtils; import android.text.TextUtils;
import org.chromium.base.Callback; import org.chromium.base.Callback;
...@@ -20,6 +21,7 @@ import org.chromium.chrome.browser.ntp.snippets.SnippetsBridge; ...@@ -20,6 +21,7 @@ import org.chromium.chrome.browser.ntp.snippets.SnippetsBridge;
import org.chromium.chrome.browser.ntp.snippets.SuggestionsSource; import org.chromium.chrome.browser.ntp.snippets.SuggestionsSource;
import org.chromium.chrome.browser.offlinepages.OfflinePageBridge; import org.chromium.chrome.browser.offlinepages.OfflinePageBridge;
import org.chromium.chrome.browser.offlinepages.OfflinePageItem; import org.chromium.chrome.browser.offlinepages.OfflinePageItem;
import org.chromium.chrome.browser.suggestions.ContentSuggestionPlaceholder;
import org.chromium.chrome.browser.suggestions.SuggestionsOfflineModelObserver; import org.chromium.chrome.browser.suggestions.SuggestionsOfflineModelObserver;
import org.chromium.chrome.browser.suggestions.SuggestionsRanker; import org.chromium.chrome.browser.suggestions.SuggestionsRanker;
import org.chromium.chrome.browser.suggestions.SuggestionsUiDelegate; import org.chromium.chrome.browser.suggestions.SuggestionsUiDelegate;
...@@ -47,8 +49,9 @@ public class SuggestionsSection extends InnerNode { ...@@ -47,8 +49,9 @@ public class SuggestionsSection extends InnerNode {
// Children // Children
private final SectionHeader mHeader; private final SectionHeader mHeader;
private final @Nullable ContentSuggestionPlaceholder mPlaceholder;
private final SuggestionsList mSuggestionsList; private final SuggestionsList mSuggestionsList;
private final StatusItem mStatus; private final @Nullable StatusItem mStatus;
private final ActionItem mMoreButton; private final ActionItem mMoreButton;
/** /**
...@@ -97,17 +100,16 @@ public class SuggestionsSection extends InnerNode { ...@@ -97,17 +100,16 @@ public class SuggestionsSection extends InnerNode {
mHeader = new SectionHeader(info.getTitle()); mHeader = new SectionHeader(info.getTitle());
mSuggestionsList = new SuggestionsList(mSuggestionsSource, ranker, info); mSuggestionsList = new SuggestionsList(mSuggestionsSource, ranker, info);
mMoreButton = new ActionItem(this, ranker);
boolean isChromeHomeEnabled = FeatureUtilities.isChromeHomeEnabled(); boolean isChromeHomeEnabled = FeatureUtilities.isChromeHomeEnabled();
if (isChromeHomeEnabled) { if (isChromeHomeEnabled) {
mStatus = null; mStatus = null;
mPlaceholder = new ContentSuggestionPlaceholder();
addChildren(mHeader, mPlaceholder, mSuggestionsList, mMoreButton);
} else { } else {
mStatus = StatusItem.createNoSuggestionsItem(info); mStatus = StatusItem.createNoSuggestionsItem(info);
} mPlaceholder = null;
mMoreButton = new ActionItem(this, ranker);
if (isChromeHomeEnabled) {
addChildren(mHeader, mSuggestionsList, mMoreButton);
} else {
addChildren(mHeader, mSuggestionsList, mStatus, mMoreButton); addChildren(mHeader, mSuggestionsList, mStatus, mMoreButton);
} }
...@@ -252,9 +254,14 @@ public class SuggestionsSection extends InnerNode { ...@@ -252,9 +254,14 @@ public class SuggestionsSection extends InnerNode {
if ((newSuggestionsCount == 0) == (oldSuggestionsCount == 0)) return; if ((newSuggestionsCount == 0) == (oldSuggestionsCount == 0)) return;
if (!FeatureUtilities.isChromeHomeEnabled()) { if (!FeatureUtilities.isChromeHomeEnabled()) {
mStatus.setVisible(newSuggestionsCount == 0); mStatus.setVisible(!hasSuggestions());
} }
// A manual fetch can complete after the placeholder is shown, thus we can have both a
// placeholder and some suggestions present with no status change notification. We need to
// update its visibility here to avoid that.
updatePlaceholderVisibility();
// When the ActionItem stops being dismissable, it is possible that it was being // When the ActionItem stops being dismissable, it is possible that it was being
// interacted with. We need to reset the view's related property changes. // interacted with. We need to reset the view's related property changes.
if (mMoreButton.isVisible()) { if (mMoreButton.isVisible()) {
...@@ -359,7 +366,7 @@ public class SuggestionsSection extends InnerNode { ...@@ -359,7 +366,7 @@ public class SuggestionsSection extends InnerNode {
} }
} }
public boolean hasSuggestions() { private boolean hasSuggestions() {
return mSuggestionsList.getItemCount() != 0; return mSuggestionsList.getItemCount() != 0;
} }
...@@ -379,10 +386,20 @@ public class SuggestionsSection extends InnerNode { ...@@ -379,10 +386,20 @@ public class SuggestionsSection extends InnerNode {
return mIsDataStale; return mIsDataStale;
} }
/** Whether the section is waiting for content to be loaded. */
public boolean isLoading() { public boolean isLoading() {
return mMoreButton.getState() == ActionItem.State.LOADING; return mMoreButton.getState() == ActionItem.State.LOADING;
} }
/**
* @return Whether the section is showing content cards. The placeholder is included in this
* check, as it's standing for content, but the status card is not.
*/
public boolean hasCards() {
return hasSuggestions()
|| (FeatureUtilities.isChromeHomeEnabled() && mPlaceholder.isVisible());
}
/** /**
* Returns whether content has been inserted in the section since last time this method was * Returns whether content has been inserted in the section since last time this method was
* called. * called.
...@@ -401,6 +418,11 @@ public class SuggestionsSection extends InnerNode { ...@@ -401,6 +418,11 @@ public class SuggestionsSection extends InnerNode {
return suggestionIds; return suggestionIds;
} }
private void updatePlaceholderVisibility() {
if (!FeatureUtilities.isChromeHomeEnabled()) return;
mPlaceholder.setVisible(isLoading() && !hasSuggestions());
}
/** /**
* Requests the section to update itself. If possible, it will retrieve suggestions from the * Requests the section to update itself. If possible, it will retrieve suggestions from the
* backend and use them to replace the current ones. This call may have no or only partial * backend and use them to replace the current ones. This call may have no or only partial
...@@ -540,8 +562,9 @@ public class SuggestionsSection extends InnerNode { ...@@ -540,8 +562,9 @@ public class SuggestionsSection extends InnerNode {
Log.d(TAG, "setStatus: unavailable status, cleared suggestions."); Log.d(TAG, "setStatus: unavailable status, cleared suggestions.");
} }
mMoreButton.updateState(SnippetsBridge.isCategoryLoading(status) ? ActionItem.State.LOADING boolean isLoading = SnippetsBridge.isCategoryLoading(status);
: ActionItem.State.BUTTON); mMoreButton.updateState(isLoading ? ActionItem.State.LOADING : ActionItem.State.BUTTON);
updatePlaceholderVisibility();
} }
/** Clears the suggestions and related data, resetting the state of the section. */ /** Clears the suggestions and related data, resetting the state of the section. */
......
// Copyright 2017 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
package org.chromium.chrome.browser.suggestions;
import android.support.annotation.ColorRes;
import org.chromium.chrome.R;
import org.chromium.chrome.browser.ntp.ContextMenuManager;
import org.chromium.chrome.browser.ntp.cards.CardViewHolder;
import org.chromium.chrome.browser.ntp.cards.ItemViewType;
import org.chromium.chrome.browser.ntp.cards.NewTabPageViewHolder;
import org.chromium.chrome.browser.ntp.cards.NodeVisitor;
import org.chromium.chrome.browser.ntp.cards.OptionalLeaf;
import org.chromium.chrome.browser.widget.displaystyle.UiConfig;
/**
* Optional node describing an empty content suggestion card.
*/
public class ContentSuggestionPlaceholder extends OptionalLeaf {
/** Color used to fill highlighted empty UI elements of the placeholder. */
private static final @ColorRes int HIGHLIGHT_COLOR = R.color.black_alpha_20;
public void setVisible(boolean visible) {
setVisibilityInternal(visible);
}
@Override
protected void onBindViewHolder(NewTabPageViewHolder holder) {
((ViewHolder) holder).onBindViewHolder();
}
@Override
protected int getItemViewType() {
return ItemViewType.PLACEHOLDER_CARD;
}
@Override
protected void visitOptionalItem(NodeVisitor visitor) {
visitor.visitPlaceholderItem();
}
/**
* ViewHolder for {@link ContentSuggestionPlaceholder}. Holds an empty modern card, where the
* main fields (thumbnail, title and publisher) are filled with some plain color instead of
* actual data.
*/
public static class ViewHolder extends CardViewHolder {
public ViewHolder(SuggestionsRecyclerView recyclerView, UiConfig uiConfig,
ContextMenuManager contextMenuManager) {
super(R.layout.content_suggestions_card_modern, recyclerView, uiConfig,
contextMenuManager);
itemView.findViewById(R.id.article_thumbnail).setBackgroundResource(HIGHLIGHT_COLOR);
}
@Override
public boolean isDismissable() {
return false;
}
@Override
public boolean isItemSupported(int menuItemId) {
return false;
}
}
}
...@@ -134,6 +134,13 @@ public class SuggestionsBottomSheetContent implements BottomSheet.BottomSheetCon ...@@ -134,6 +134,13 @@ public class SuggestionsBottomSheetContent implements BottomSheet.BottomSheetCon
mBottomSheetObserver = new SuggestionsSheetVisibilityChangeObserver(this, activity) { mBottomSheetObserver = new SuggestionsSheetVisibilityChangeObserver(this, activity) {
@Override @Override
public void onContentShown(boolean isFirstShown) { public void onContentShown(boolean isFirstShown) {
// TODO(dgn): Temporary workaround to trigger an event in the backend when the
// sheet is opened following inactivity. See https://crbug.com/760974. Should be
// moved back to the "new opening of the sheet" path once we are able to trigger it
// in that case.
mSuggestionsUiDelegate.getEventReporter().onSurfaceOpened();
SuggestionsMetrics.recordSurfaceVisible();
if (isFirstShown) { if (isFirstShown) {
adapter.refreshSuggestions(); adapter.refreshSuggestions();
...@@ -147,13 +154,7 @@ public class SuggestionsBottomSheetContent implements BottomSheet.BottomSheetCon ...@@ -147,13 +154,7 @@ public class SuggestionsBottomSheetContent implements BottomSheet.BottomSheetCon
mRecyclerView.getScrollEventReporter().reset(); mRecyclerView.getScrollEventReporter().reset();
} }
// TODO(dgn): Temporary workaround to trigger an event in the backend when the
// sheet is opened following inactivity. See https://crbug.com/760974. Should be
// moved back to the "new opening of the sheet" path once we are able to trigger it
// in that case.
mSuggestionsUiDelegate.getEventReporter().onSurfaceOpened();
SuggestionsMetrics.recordSurfaceVisible();
} }
@Override @Override
......
...@@ -1077,6 +1077,7 @@ chrome_java_sources = [ ...@@ -1077,6 +1077,7 @@ chrome_java_sources = [
"java/src/org/chromium/chrome/browser/suggestions/SuggestionsEventReporterBridge.java", "java/src/org/chromium/chrome/browser/suggestions/SuggestionsEventReporterBridge.java",
"java/src/org/chromium/chrome/browser/suggestions/SuggestionsMetrics.java", "java/src/org/chromium/chrome/browser/suggestions/SuggestionsMetrics.java",
"java/src/org/chromium/chrome/browser/suggestions/SuggestionsRanker.java", "java/src/org/chromium/chrome/browser/suggestions/SuggestionsRanker.java",
"java/src/org/chromium/chrome/browser/suggestions/ContentSuggestionPlaceholder.java",
"java/src/org/chromium/chrome/browser/suggestions/SuggestionsSheetVisibilityChangeObserver.java", "java/src/org/chromium/chrome/browser/suggestions/SuggestionsSheetVisibilityChangeObserver.java",
"java/src/org/chromium/chrome/browser/superviseduser/SupervisedUserContentProvider.java", "java/src/org/chromium/chrome/browser/superviseduser/SupervisedUserContentProvider.java",
"java/src/org/chromium/chrome/browser/sync/GmsCoreSyncListener.java", "java/src/org/chromium/chrome/browser/sync/GmsCoreSyncListener.java",
......
...@@ -127,6 +127,7 @@ public class NewTabPageAdapterTest { ...@@ -127,6 +127,7 @@ public class NewTabPageAdapterTest {
public boolean mViewAllButton; public boolean mViewAllButton;
public boolean mFetchButton; public boolean mFetchButton;
public boolean mProgressItem; public boolean mProgressItem;
public boolean mPlaceholder;
public SectionDescriptor() {} public SectionDescriptor() {}
...@@ -172,6 +173,11 @@ public class NewTabPageAdapterTest { ...@@ -172,6 +173,11 @@ public class NewTabPageAdapterTest {
mStatusCard = true; mStatusCard = true;
return this; return this;
} }
public SectionDescriptor withPlaceholder() {
mPlaceholder = true;
return this;
}
} }
/** /**
...@@ -214,6 +220,10 @@ public class NewTabPageAdapterTest { ...@@ -214,6 +220,10 @@ public class NewTabPageAdapterTest {
mInOrder.verify(mVisitor, mVerification).visitHeader(); mInOrder.verify(mVisitor, mVerification).visitHeader();
} }
if (descriptor.mPlaceholder) {
mInOrder.verify(mVisitor, mVerification).visitPlaceholderItem();
}
for (SnippetArticle suggestion : descriptor.mSuggestions) { for (SnippetArticle suggestion : descriptor.mSuggestions) {
mInOrder.verify(mVisitor, mVerification).visitSuggestion(eq(suggestion)); mInOrder.verify(mVisitor, mVerification).visitSuggestion(eq(suggestion));
} }
...@@ -273,7 +283,7 @@ public class NewTabPageAdapterTest { ...@@ -273,7 +283,7 @@ public class NewTabPageAdapterTest {
FeatureUtilities.resetChromeHomeEnabledForTests(); FeatureUtilities.resetChromeHomeEnabledForTests();
// Set empty variation params for the test. // Set empty variation params for the test.
CardsVariationParameters.setTestVariationParams(new HashMap<String, String>()); CardsVariationParameters.setTestVariationParams(new HashMap<>());
// Initialise the sign in state. We will be signed in by default in the tests. // Initialise the sign in state. We will be signed in by default in the tests.
assertFalse( assertFalse(
...@@ -546,7 +556,7 @@ public class NewTabPageAdapterTest { ...@@ -546,7 +556,7 @@ public class NewTabPageAdapterTest {
// Part 2: VisibleIfEmpty = false // Part 2: VisibleIfEmpty = false
suggestionsSource = new FakeSuggestionsSource(); suggestionsSource = new FakeSuggestionsSource();
suggestionsSource.setStatusForCategory(TEST_CATEGORY, CategoryStatus.INITIALIZING); suggestionsSource.setStatusForCategory(TEST_CATEGORY, CategoryStatus.AVAILABLE);
suggestionsSource.setInfoForCategory( suggestionsSource.setInfoForCategory(
TEST_CATEGORY, new CategoryInfoBuilder(TEST_CATEGORY).build()); TEST_CATEGORY, new CategoryInfoBuilder(TEST_CATEGORY).build());
...@@ -966,7 +976,7 @@ public class NewTabPageAdapterTest { ...@@ -966,7 +976,7 @@ public class NewTabPageAdapterTest {
@Test @Test
@Feature({"Ntp"}) @Feature({"Ntp"})
@Features({ @Features.Register(ChromeFeatureList.CHROME_HOME) }) @Features(@Features.Register(ChromeFeatureList.CHROME_HOME))
public void testSigninPromoModern() { public void testSigninPromoModern() {
when(mMockSigninManager.isSignInAllowed()).thenReturn(true); when(mMockSigninManager.isSignInAllowed()).thenReturn(true);
when(mMockSigninManager.isSignedInOnNative()).thenReturn(false); when(mMockSigninManager.isSignedInOnNative()).thenReturn(false);
...@@ -974,7 +984,7 @@ public class NewTabPageAdapterTest { ...@@ -974,7 +984,7 @@ public class NewTabPageAdapterTest {
reloadNtp(); reloadNtp();
// Special case of the modern layout: the signin promo comes before the content suggestions. // Special case of the modern layout: the signin promo comes before the content suggestions.
assertItemsFor(signinPromo(), emptySection().withProgress()); assertItemsFor(signinPromo(), emptySection().withPlaceholder().withProgress());
} }
@Test @Test
...@@ -1133,18 +1143,19 @@ public class NewTabPageAdapterTest { ...@@ -1133,18 +1143,19 @@ public class NewTabPageAdapterTest {
@Test @Test
@Feature({"Ntp"}) @Feature({"Ntp"})
@Features({ @Features.Register(value = ChromeFeatureList.CHROME_HOME) }) @Features(@Features.Register(ChromeFeatureList.CHROME_HOME))
public void testAllDismissedModern() { public void testAllDismissedModern() {
when(mUiDelegate.isVisible()).thenReturn(true); when(mUiDelegate.isVisible()).thenReturn(true);
@CategoryInt
int category = KnownCategories.ARTICLES;
mSource.setStatusForCategory(TEST_CATEGORY, CategoryStatus.NOT_PROVIDED); mSource.setStatusForCategory(TEST_CATEGORY, CategoryStatus.NOT_PROVIDED);
mSource.setInfoForCategory(KnownCategories.ARTICLES, mSource.setInfoForCategory(
new CategoryInfoBuilder(TEST_CATEGORY).showIfEmpty().build()); category, new CategoryInfoBuilder(category).showIfEmpty().build());
mSource.setStatusForCategory(KnownCategories.ARTICLES, CategoryStatus.AVAILABLE); mSource.setStatusForCategory(category, CategoryStatus.AVAILABLE);
final int numSuggestions = 3; final int numSuggestions = 3;
List<SnippetArticle> suggestions = List<SnippetArticle> suggestions = createDummySuggestions(numSuggestions, category);
createDummySuggestions(numSuggestions, KnownCategories.ARTICLES); mSource.setSuggestionsForCategory(category, suggestions);
mSource.setSuggestionsForCategory(KnownCategories.ARTICLES, suggestions);
reloadNtp(); reloadNtp();
assertItemsForChromeHome(section(suggestions).withoutHeader()); assertItemsForChromeHome(section(suggestions).withoutHeader());
...@@ -1155,7 +1166,42 @@ public class NewTabPageAdapterTest { ...@@ -1155,7 +1166,42 @@ public class NewTabPageAdapterTest {
verify(itemRemovedCallback).onResult(anyString()); verify(itemRemovedCallback).onResult(anyString());
} }
assertItemsForChromeHome(emptySection().withoutHeader()); assertItemsForEmptyChromeHome(emptySection().withoutHeader());
}
/** Tests whether a section stays visible if empty, if required. */
@Test
@Feature({"Ntp"})
@Features(@Features.Register(ChromeFeatureList.CHROME_HOME))
public void testPlaceholderModern() {
// Need to use ARTICLES for Home features to work properly, e.g. All Dismissed visibility.
int category = KnownCategories.ARTICLES;
mSource.setStatusForCategory(TEST_CATEGORY, CategoryStatus.NOT_PROVIDED);
mSource.setInfoForCategory(
category, new CategoryInfoBuilder(category).showIfEmpty().build());
mSource.setStatusForCategory(category, CategoryStatus.INITIALIZING);
reloadNtp();
// By default we have an initializing section. It should show a placeholder in modern.
assertItemsForChromeHome(emptySection().withoutHeader().withPlaceholder().withProgress());
// We stop loading (timeout?), we should remove the placeholder.
mSource.setStatusForCategory(category, CategoryStatus.AVAILABLE);
assertItemsForEmptyChromeHome(emptySection().withoutHeader());
// Loading suggestions show a placeholder when they are empty
mSource.setStatusForCategory(category, CategoryStatus.AVAILABLE_LOADING);
assertItemsForChromeHome(emptySection().withoutHeader().withPlaceholder().withProgress());
// Simulating an update from the component (timeout or success), we are notified about a
// status changes and that we should fetch the new suggestions.
mSource.setStatusForCategory(category, CategoryStatus.AVAILABLE);
List<SnippetArticle> suggestions = mSource.createAndSetSuggestions(2, category);
assertItemsForChromeHome(section(suggestions).withoutHeader());
// Loading suggestions should not show a placeholder when they are not empty
mSource.setStatusForCategory(category, CategoryStatus.AVAILABLE_LOADING);
assertItemsForChromeHome(section(suggestions).withoutHeader().withProgress());
} }
/** /**
...@@ -1208,9 +1254,19 @@ public class NewTabPageAdapterTest { ...@@ -1208,9 +1254,19 @@ public class NewTabPageAdapterTest {
// TODO(bauerb): Remove above-the-fold from test setup in Chrome Home. // TODO(bauerb): Remove above-the-fold from test setup in Chrome Home.
matcher.expectAboveTheFoldItem(); matcher.expectAboveTheFoldItem();
if (section.mSuggestions.isEmpty()) matcher.expectAllDismissedItem();
matcher.expectSection(section); matcher.expectSection(section);
if (!section.mSuggestions.isEmpty()) matcher.expectFooter(); matcher.expectFooter(); // TODO(dgn): Handle scroll to reload with removes the footer
matcher.expectSpacingItem();
matcher.expectEnd();
}
private void assertItemsForEmptyChromeHome(SectionDescriptor section) {
ItemsMatcher matcher = new ItemsMatcher(mAdapter.getRootForTesting());
// TODO(bauerb): Remove above-the-fold from test setup in Chrome Home.
matcher.expectAboveTheFoldItem();
matcher.expectAllDismissedItem();
matcher.expectSection(section);
matcher.expectSpacingItem(); matcher.expectSpacingItem();
matcher.expectEnd(); matcher.expectEnd();
} }
...@@ -1240,7 +1296,7 @@ public class NewTabPageAdapterTest { ...@@ -1240,7 +1296,7 @@ public class NewTabPageAdapterTest {
*/ */
private SectionDescriptor sectionWithStatusCard() { private SectionDescriptor sectionWithStatusCard() {
assertFalse(FeatureUtilities.isChromeHomeEnabled()); assertFalse(FeatureUtilities.isChromeHomeEnabled());
return new SectionDescriptor(Collections.<SnippetArticle>emptyList()).withStatusCard(); return new SectionDescriptor(Collections.emptyList()).withStatusCard();
} }
/** /**
...@@ -1251,7 +1307,7 @@ public class NewTabPageAdapterTest { ...@@ -1251,7 +1307,7 @@ public class NewTabPageAdapterTest {
*/ */
private SectionDescriptor emptySection() { private SectionDescriptor emptySection() {
assertTrue(FeatureUtilities.isChromeHomeEnabled()); assertTrue(FeatureUtilities.isChromeHomeEnabled());
return new SectionDescriptor(Collections.<SnippetArticle>emptyList()); return new SectionDescriptor(Collections.emptyList());
} }
private void resetUiDelegate() { private void resetUiDelegate() {
......
...@@ -32,9 +32,11 @@ import org.mockito.ArgumentCaptor; ...@@ -32,9 +32,11 @@ import org.mockito.ArgumentCaptor;
import org.mockito.InOrder; import org.mockito.InOrder;
import org.mockito.Mock; import org.mockito.Mock;
import org.mockito.MockitoAnnotations; import org.mockito.MockitoAnnotations;
import org.robolectric.RuntimeEnvironment;
import org.robolectric.annotation.Config; import org.robolectric.annotation.Config;
import org.chromium.base.Callback; import org.chromium.base.Callback;
import org.chromium.base.ContextUtils;
import org.chromium.base.test.util.Feature; import org.chromium.base.test.util.Feature;
import org.chromium.chrome.browser.ChromeFeatureList; import org.chromium.chrome.browser.ChromeFeatureList;
import org.chromium.chrome.browser.DisableHistogramsRule; import org.chromium.chrome.browser.DisableHistogramsRule;
...@@ -48,6 +50,7 @@ import org.chromium.chrome.browser.suggestions.DestructionObserver; ...@@ -48,6 +50,7 @@ import org.chromium.chrome.browser.suggestions.DestructionObserver;
import org.chromium.chrome.browser.suggestions.SuggestionsEventReporter; import org.chromium.chrome.browser.suggestions.SuggestionsEventReporter;
import org.chromium.chrome.browser.suggestions.SuggestionsRanker; import org.chromium.chrome.browser.suggestions.SuggestionsRanker;
import org.chromium.chrome.browser.suggestions.SuggestionsUiDelegate; import org.chromium.chrome.browser.suggestions.SuggestionsUiDelegate;
import org.chromium.chrome.browser.util.FeatureUtilities;
import org.chromium.chrome.test.util.browser.Features; import org.chromium.chrome.test.util.browser.Features;
import org.chromium.chrome.test.util.browser.suggestions.ContentSuggestionsTestUtils.CategoryInfoBuilder; import org.chromium.chrome.test.util.browser.suggestions.ContentSuggestionsTestUtils.CategoryInfoBuilder;
import org.chromium.chrome.test.util.browser.suggestions.FakeSuggestionsSource; import org.chromium.chrome.test.util.browser.suggestions.FakeSuggestionsSource;
...@@ -83,7 +86,10 @@ public class SectionListTest { ...@@ -83,7 +86,10 @@ public class SectionListTest {
@Before @Before
public void setUp() { public void setUp() {
CardsVariationParameters.setTestVariationParams(new HashMap<String, String>()); ContextUtils.initApplicationContextForTests(RuntimeEnvironment.application);
FeatureUtilities.resetChromeHomeEnabledForTests();
CardsVariationParameters.setTestVariationParams(new HashMap<>());
MockitoAnnotations.initMocks(this); MockitoAnnotations.initMocks(this);
mSuggestionSource = spy(new FakeSuggestionsSource()); mSuggestionSource = spy(new FakeSuggestionsSource());
...@@ -440,7 +446,7 @@ public class SectionListTest { ...@@ -440,7 +446,7 @@ public class SectionListTest {
assertFalse(sectionList.categoriesChanged(mSuggestionSource.getCategories())); assertFalse(sectionList.categoriesChanged(mSuggestionSource.getCategories()));
assertFalse(sectionList.categoriesChanged(new int[] {CATEGORY1})); assertFalse(sectionList.categoriesChanged(new int[] {CATEGORY1}));
mSuggestionSource.setStatusForCategory(CATEGORY2, CategoryStatus.AVAILABLE_LOADING); mSuggestionSource.setStatusForCategory(CATEGORY2, CategoryStatus.AVAILABLE);
// After notifying of a change for the category, it stops being ignored. // After notifying of a change for the category, it stops being ignored.
assertTrue(sectionList.categoriesChanged(mSuggestionSource.getCategories())); assertTrue(sectionList.categoriesChanged(mSuggestionSource.getCategories()));
......
...@@ -197,6 +197,11 @@ public final class ContentSuggestionsTestUtils { ...@@ -197,6 +197,11 @@ public final class ContentSuggestionsTestUtils {
describeItem("SITE_SECTION"); describeItem("SITE_SECTION");
} }
@Override
public void visitPlaceholderItem() {
describeItem("PLACEHOLDER_CARD");
}
private void describeItem(String description) { private void describeItem(String description) {
stringBuilder.append( stringBuilder.append(
String.format(Locale.US, "%s - %s%n", mPosition++, description)); String.format(Locale.US, "%s - %s%n", mPosition++, description));
......
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