Commit 372bfbd5 authored by dgn's avatar dgn Committed by Commit bot

[NTP Client] Refresh the visiblity of AllDismissed when items change

Adds a mocking method for Feature flags in SnippetsConfig

BUG=657319

Review-Url: https://codereview.chromium.org/2434263002
Cr-Commit-Position: refs/heads/master@{#427090}
parent 36a9ee3e
...@@ -31,6 +31,10 @@ public abstract class InnerNode extends ChildNode implements NodeParent { ...@@ -31,6 +31,10 @@ public abstract class InnerNode extends ChildNode implements NodeParent {
private int getStartingOffsetForChildIndex(int childIndex) { private int getStartingOffsetForChildIndex(int childIndex) {
List<TreeNode> children = getChildren(); List<TreeNode> children = getChildren();
if (childIndex < 0 || childIndex >= children.size()) {
throw new IndexOutOfBoundsException(childIndex + "/" + children.size());
}
int offset = 0; int offset = 0;
for (int i = 0; i < childIndex; i++) { for (int i = 0; i < childIndex; i++) {
offset += children.get(i).getItemCount(); offset += children.get(i).getItemCount();
......
...@@ -26,6 +26,7 @@ import org.chromium.chrome.browser.ntp.snippets.SectionHeaderViewHolder; ...@@ -26,6 +26,7 @@ import org.chromium.chrome.browser.ntp.snippets.SectionHeaderViewHolder;
import org.chromium.chrome.browser.ntp.snippets.SnippetArticle; import org.chromium.chrome.browser.ntp.snippets.SnippetArticle;
import org.chromium.chrome.browser.ntp.snippets.SnippetArticleViewHolder; 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.SnippetsConfig;
import org.chromium.chrome.browser.ntp.snippets.SuggestionsSource; import org.chromium.chrome.browser.ntp.snippets.SuggestionsSource;
import java.util.ArrayList; import java.util.ArrayList;
...@@ -138,6 +139,24 @@ public class NewTabPageAdapter ...@@ -138,6 +139,24 @@ public class NewTabPageAdapter
protected List<TreeNode> getChildren() { protected List<TreeNode> getChildren() {
return mChildren; return mChildren;
} }
@Override
public void onItemRangeChanged(TreeNode child, int index, int count) {
if (mChildren.isEmpty()) return; // The sections have not been initialised yet.
super.onItemRangeChanged(child, index, count);
}
@Override
public void onItemRangeInserted(TreeNode child, int index, int count) {
if (mChildren.isEmpty()) return; // The sections have not been initialised yet.
super.onItemRangeInserted(child, index, count);
}
@Override
public void onItemRangeRemoved(TreeNode child, int index, int count) {
if (mChildren.isEmpty()) return; // The sections have not been initialised yet.
super.onItemRangeRemoved(child, index, count);
}
}; };
mSigninPromo = new SignInPromo(mRoot, this); mSigninPromo = new SignInPromo(mRoot, this);
...@@ -155,6 +174,7 @@ public class NewTabPageAdapter ...@@ -155,6 +174,7 @@ public class NewTabPageAdapter
*/ */
public void resetSections(boolean alwaysAllowEmptySections) { public void resetSections(boolean alwaysAllowEmptySections) {
mSections.clear(); mSections.clear();
mChildren.clear();
SuggestionsSource suggestionsSource = mNewTabPageManager.getSuggestionsSource(); SuggestionsSource suggestionsSource = mNewTabPageManager.getSuggestionsSource();
int[] categories = suggestionsSource.getCategories(); int[] categories = suggestionsSource.getCategories();
...@@ -341,11 +361,7 @@ public class NewTabPageAdapter ...@@ -341,11 +361,7 @@ public class NewTabPageAdapter
} }
public int getFirstHeaderPosition() { public int getFirstHeaderPosition() {
int count = getItemCount(); return getFirstPositionForType(ItemViewType.HEADER);
for (int i = 0; i < count; i++) {
if (getItemViewType(i) == ItemViewType.HEADER) return i;
}
return RecyclerView.NO_POSITION;
} }
public int getFirstCardPosition() { public int getFirstCardPosition() {
...@@ -355,10 +371,6 @@ public class NewTabPageAdapter ...@@ -355,10 +371,6 @@ public class NewTabPageAdapter
return RecyclerView.NO_POSITION; return RecyclerView.NO_POSITION;
} }
public int getFooterPosition() {
return getChildPositionOffset(mFooter);
}
public int getBottomSpacerPosition() { public int getBottomSpacerPosition() {
return getChildPositionOffset(mBottomSpacer); return getChildPositionOffset(mBottomSpacer);
} }
...@@ -409,17 +421,27 @@ public class NewTabPageAdapter ...@@ -409,17 +421,27 @@ public class NewTabPageAdapter
notifyDataSetChanged(); notifyDataSetChanged();
} }
private void updateAllDismissedVisibility() {
boolean showAllDismissed = hasAllBeenDismissed();
if (showAllDismissed == mChildren.contains(mAllDismissed)) return;
if (showAllDismissed) {
assert mChildren.contains(mFooter);
mChildren.set(mChildren.indexOf(mFooter), mAllDismissed);
} else {
assert mChildren.contains(mAllDismissed);
mChildren.set(mChildren.indexOf(mAllDismissed), mFooter);
}
notifyItemChanged(getLastContentItemPosition());
}
private void removeSection(SuggestionsSection section) { private void removeSection(SuggestionsSection section) {
mSections.remove(section.getCategory()); mSections.remove(section.getCategory());
int startPos = getChildPositionOffset(section); int startPos = getChildPositionOffset(section);
mChildren.remove(section); mChildren.remove(section);
notifyItemRangeRemoved(startPos, section.getItemCount()); notifyItemRangeRemoved(startPos, section.getItemCount());
if (hasAllBeenDismissed()) { updateAllDismissedVisibility();
int footerPosition = getFooterPosition();
mChildren.set(mChildren.indexOf(mFooter), mAllDismissed);
notifyItemChanged(footerPosition);
}
notifyItemChanged(getBottomSpacerPosition()); notifyItemChanged(getBottomSpacerPosition());
} }
...@@ -427,24 +449,25 @@ public class NewTabPageAdapter ...@@ -427,24 +449,25 @@ public class NewTabPageAdapter
@Override @Override
public void onItemRangeChanged(TreeNode child, int itemPosition, int itemCount) { public void onItemRangeChanged(TreeNode child, int itemPosition, int itemCount) {
assert child == mRoot; assert child == mRoot;
if (mChildren.isEmpty()) return; // The sections have not been initialised yet.
notifyItemRangeChanged(itemPosition, itemCount); notifyItemRangeChanged(itemPosition, itemCount);
} }
@Override @Override
public void onItemRangeInserted(TreeNode child, int itemPosition, int itemCount) { public void onItemRangeInserted(TreeNode child, int itemPosition, int itemCount) {
assert child == mRoot; assert child == mRoot;
if (mChildren.isEmpty()) return; // The sections have not been initialised yet.
notifyItemRangeInserted(itemPosition, itemCount); notifyItemRangeInserted(itemPosition, itemCount);
notifyItemChanged(getItemCount() - 1); // Refresh the spacer too. notifyItemChanged(getItemCount() - 1); // Refresh the spacer too.
updateAllDismissedVisibility();
} }
@Override @Override
public void onItemRangeRemoved(TreeNode child, int itemPosition, int itemCount) { public void onItemRangeRemoved(TreeNode child, int itemPosition, int itemCount) {
assert child == mRoot; assert child == mRoot;
if (mChildren.isEmpty()) return; // The sections have not been initialised yet.
notifyItemRangeRemoved(itemPosition, itemCount); notifyItemRangeRemoved(itemPosition, itemCount);
notifyItemChanged(getItemCount() - 1); // Refresh the spacer too. notifyItemChanged(getItemCount() - 1); // Refresh the spacer too.
updateAllDismissedVisibility();
} }
@Override @Override
...@@ -492,6 +515,7 @@ public class NewTabPageAdapter ...@@ -492,6 +515,7 @@ public class NewTabPageAdapter
} }
private void dismissSection(SuggestionsSection section) { private void dismissSection(SuggestionsSection section) {
assert SnippetsConfig.isSectionDismissalEnabled();
mNewTabPageManager.getSuggestionsSource().dismissCategory(section.getCategory()); mNewTabPageManager.getSuggestionsSource().dismissCategory(section.getCategory());
removeSection(section); removeSection(section);
} }
...@@ -517,12 +541,6 @@ public class NewTabPageAdapter ...@@ -517,12 +541,6 @@ public class NewTabPageAdapter
private void dismissPromo() { private void dismissPromo() {
// TODO(dgn): accessibility announcement. // TODO(dgn): accessibility announcement.
mSigninPromo.dismiss(); mSigninPromo.dismiss();
if (hasAllBeenDismissed()) {
int footerPosition = getFooterPosition();
mChildren.set(mChildren.indexOf(mFooter), mAllDismissed);
notifyItemChanged(footerPosition);
}
} }
/** /**
...@@ -567,6 +585,15 @@ public class NewTabPageAdapter ...@@ -567,6 +585,15 @@ public class NewTabPageAdapter
return mRoot.getSuggestionAt(position); return mRoot.getSuggestionAt(position);
} }
@VisibleForTesting
int getFirstPositionForType(@ItemViewType int viewType) {
int count = getItemCount();
for (int i = 0; i < count; i++) {
if (getItemViewType(i) == viewType) return i;
}
return RecyclerView.NO_POSITION;
}
private void announceItemRemoved(String suggestionTitle) { private void announceItemRemoved(String suggestionTitle) {
// In tests the RecyclerView can be null. // In tests the RecyclerView can be null.
if (mRecyclerView == null) return; if (mRecyclerView == null) return;
......
...@@ -11,6 +11,7 @@ import android.support.annotation.StringRes; ...@@ -11,6 +11,7 @@ import android.support.annotation.StringRes;
import android.support.v7.widget.RecyclerView; import android.support.v7.widget.RecyclerView;
import org.chromium.base.ContextUtils; import org.chromium.base.ContextUtils;
import org.chromium.base.VisibleForTesting;
import org.chromium.base.metrics.RecordUserAction; import org.chromium.base.metrics.RecordUserAction;
import org.chromium.chrome.R; import org.chromium.chrome.R;
import org.chromium.chrome.browser.ntp.NewTabPage.DestructionObserver; import org.chromium.chrome.browser.ntp.NewTabPage.DestructionObserver;
...@@ -125,7 +126,7 @@ public class SignInPromo extends ChildNode implements StatusCardViewHolder.DataS ...@@ -125,7 +126,7 @@ public class SignInPromo extends ChildNode implements StatusCardViewHolder.DataS
} }
/** Attempts to show the sign in promo. If the user dismissed it before, it will not be shown.*/ /** Attempts to show the sign in promo. If the user dismissed it before, it will not be shown.*/
public void maybeShow() { private void maybeShow() {
if (mVisible) return; if (mVisible) return;
mVisible = true; mVisible = true;
...@@ -136,7 +137,7 @@ public class SignInPromo extends ChildNode implements StatusCardViewHolder.DataS ...@@ -136,7 +137,7 @@ public class SignInPromo extends ChildNode implements StatusCardViewHolder.DataS
} }
/** Hides the sign in promo. */ /** Hides the sign in promo. */
public void hide() { private void hide() {
if (!mVisible) return; if (!mVisible) return;
mVisible = false; mVisible = false;
...@@ -154,7 +155,8 @@ public class SignInPromo extends ChildNode implements StatusCardViewHolder.DataS ...@@ -154,7 +155,8 @@ public class SignInPromo extends ChildNode implements StatusCardViewHolder.DataS
mObserver.unregister(); mObserver.unregister();
} }
private class SigninObserver @VisibleForTesting
class SigninObserver
implements SignInStateObserver, SignInAllowedObserver, DestructionObserver { implements SignInStateObserver, SignInAllowedObserver, DestructionObserver {
private final SigninManager mSigninManager; private final SigninManager mSigninManager;
private final NewTabPageAdapter mAdapter; private final NewTabPageAdapter mAdapter;
......
...@@ -97,7 +97,7 @@ public class FakeSuggestionsSource implements SuggestionsSource { ...@@ -97,7 +97,7 @@ public class FakeSuggestionsSource implements SuggestionsSource {
@Override @Override
public void dismissCategory(@CategoryInt int category) { public void dismissCategory(@CategoryInt int category) {
throw new UnsupportedOperationException(); silentlyRemoveCategory(category);
} }
@Override @Override
......
...@@ -4,25 +4,44 @@ ...@@ -4,25 +4,44 @@
package org.chromium.chrome.browser.ntp.snippets; package org.chromium.chrome.browser.ntp.snippets;
import org.chromium.base.VisibleForTesting;
import org.chromium.chrome.browser.ChromeFeatureList; import org.chromium.chrome.browser.ChromeFeatureList;
import org.chromium.chrome.browser.offlinepages.OfflinePageBridge; import org.chromium.chrome.browser.offlinepages.OfflinePageBridge;
import java.util.Set;
/** /**
* Provides configuration details for NTP snippets. * Provides configuration details for NTP snippets.
*/ */
public final class SnippetsConfig { public final class SnippetsConfig {
/** Map that stores substitution feature flags for tests. */
private static Set<String> sTestEnabledFeatures;
private SnippetsConfig() {} private SnippetsConfig() {}
/**
* Sets the feature flags to use in JUnit tests, since native calls are not available there.
* Note: Always set it back to {@code null} at the end of the test!
*/
@VisibleForTesting
public static void setTestEnabledFeatures(Set<String> featureList) {
sTestEnabledFeatures = featureList;
}
public static boolean isEnabled() { public static boolean isEnabled() {
return ChromeFeatureList.isEnabled(ChromeFeatureList.NTP_SNIPPETS); return isFeatureEnabled(ChromeFeatureList.NTP_SNIPPETS);
} }
public static boolean isSaveToOfflineEnabled() { public static boolean isSaveToOfflineEnabled() {
return ChromeFeatureList.isEnabled(ChromeFeatureList.NTP_SNIPPETS_SAVE_TO_OFFLINE) return isFeatureEnabled(ChromeFeatureList.NTP_SNIPPETS_SAVE_TO_OFFLINE)
&& OfflinePageBridge.isBackgroundLoadingEnabled(); && OfflinePageBridge.isBackgroundLoadingEnabled();
} }
public static boolean isSectionDismissalEnabled() { public static boolean isSectionDismissalEnabled() {
return ChromeFeatureList.isEnabled(ChromeFeatureList.NTP_SUGGESTIONS_SECTION_DISMISSAL); return isFeatureEnabled(ChromeFeatureList.NTP_SUGGESTIONS_SECTION_DISMISSAL);
}
private static boolean isFeatureEnabled(String feature) {
if (sTestEnabledFeatures == null) return ChromeFeatureList.isEnabled(feature);
return sTestEnabledFeatures.contains(feature);
} }
} }
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