Commit 212feea3 authored by dgn's avatar dgn Committed by Commit bot

[NTP Client] Make status cards swipable

When the status card is swiped away, the entire section is dismissed and
stays that way on NTPs opened afterwards. If the status of the section
changes (new snippets loaded, user logged in, etc.) the section will be
added back.

Preview: https://goo.gl/photos/ooDFBuAVCLauo4bbA

BUG=638580

Review-Url: https://codereview.chromium.org/2340333002
Cr-Commit-Position: refs/heads/master@{#419170}
parent 1d3da332
...@@ -23,6 +23,7 @@ class ActionItem implements NewTabPageItem { ...@@ -23,6 +23,7 @@ class ActionItem implements NewTabPageItem {
// The position (index) of this item within its section, for logging purposes. // The position (index) of this item within its section, for logging purposes.
private int mPosition; private int mPosition;
private boolean mImpressionTracked = false; private boolean mImpressionTracked = false;
private boolean mDismissable;
public ActionItem(int category) { public ActionItem(int category) {
mCategory = category; mCategory = category;
...@@ -44,6 +45,7 @@ class ActionItem implements NewTabPageItem { ...@@ -44,6 +45,7 @@ class ActionItem implements NewTabPageItem {
public static class ViewHolder extends CardViewHolder { public static class ViewHolder extends CardViewHolder {
private ActionItem mActionListItem; private ActionItem mActionListItem;
public ViewHolder(NewTabPageRecyclerView recyclerView, final NewTabPageManager manager, public ViewHolder(NewTabPageRecyclerView recyclerView, final NewTabPageManager manager,
UiConfig uiConfig) { UiConfig uiConfig) {
super(R.layout.new_tab_page_action_card, recyclerView, uiConfig); super(R.layout.new_tab_page_action_card, recyclerView, uiConfig);
...@@ -81,6 +83,11 @@ class ActionItem implements NewTabPageItem { ...@@ -81,6 +83,11 @@ class ActionItem implements NewTabPageItem {
}); });
} }
@Override
public boolean isDismissable() {
return mActionListItem.mDismissable;
}
public void onBindViewHolder(ActionItem item) { public void onBindViewHolder(ActionItem item) {
mActionListItem = item; mActionListItem = item;
} }
...@@ -91,4 +98,9 @@ class ActionItem implements NewTabPageItem { ...@@ -91,4 +98,9 @@ class ActionItem implements NewTabPageItem {
assert holder instanceof ViewHolder; assert holder instanceof ViewHolder;
((ViewHolder) holder).onBindViewHolder(this); ((ViewHolder) holder).onBindViewHolder(this);
} }
/** Set whether this item can be dismissed.*/
public void setDismissable(boolean dismissable) {
this.mDismissable = dismissable;
}
} }
...@@ -36,7 +36,7 @@ import org.chromium.chrome.browser.util.ViewUtils; ...@@ -36,7 +36,7 @@ import org.chromium.chrome.browser.util.ViewUtils;
* - Cards will get some lateral margins when the viewport is sufficiently wide. * - Cards will get some lateral margins when the viewport is sufficiently wide.
* (see {@link UiConfig#DISPLAY_STYLE_WIDE}) * (see {@link UiConfig#DISPLAY_STYLE_WIDE})
* *
* Note: If a subclass overrides {@link #onBindViewHolder(NewTabPageItem)}, it should call the * Note: If a subclass overrides {@link #onBindViewHolder()}, it should call the
* parent implementation to reset the private state when a card is recycled. * parent implementation to reset the private state when a card is recycled.
*/ */
public class CardViewHolder extends NewTabPageViewHolder { public class CardViewHolder extends NewTabPageViewHolder {
......
...@@ -5,7 +5,6 @@ ...@@ -5,7 +5,6 @@
package org.chromium.chrome.browser.ntp.cards; package org.chromium.chrome.browser.ntp.cards;
import android.graphics.Canvas; import android.graphics.Canvas;
import android.support.v4.view.ViewCompat;
import android.support.v7.widget.RecyclerView; import android.support.v7.widget.RecyclerView;
import android.support.v7.widget.RecyclerView.Adapter; import android.support.v7.widget.RecyclerView.Adapter;
import android.support.v7.widget.RecyclerView.ViewHolder; import android.support.v7.widget.RecyclerView.ViewHolder;
...@@ -107,12 +106,24 @@ public class NewTabPageAdapter extends Adapter<NewTabPageViewHolder> ...@@ -107,12 +106,24 @@ public class NewTabPageAdapter extends Adapter<NewTabPageViewHolder>
float dX, float dY, int actionState, boolean isCurrentlyActive) { float dX, float dY, int actionState, boolean isCurrentlyActive) {
assert viewHolder instanceof NewTabPageViewHolder; assert viewHolder instanceof NewTabPageViewHolder;
// The item has already been removed. We have nothing more to do.
if (viewHolder.getAdapterPosition() == RecyclerView.NO_POSITION) return;
// We use our own implementation of the dismissal animation, so we don't call the
// parent implementation. (by default it changes the translation-X and elevation)
mRecyclerView.updateViewStateForDismiss(dX, viewHolder); mRecyclerView.updateViewStateForDismiss(dX, viewHolder);
// The super implementation performs animation and elevation, but only the animation is // If there is another item that should be animated at the same time, do the same to it.
// needed. int swipePos = viewHolder.getAdapterPosition();
super.onChildDraw(c, recyclerView, viewHolder, dX, dY, actionState, isCurrentlyActive); SuggestionsSection section = (SuggestionsSection) getGroup(swipePos);
ViewCompat.setElevation(viewHolder.itemView, 0f); int siblingPosDelta = section.getDismissSiblingPosDelta(getItems().get(swipePos));
if (siblingPosDelta == 0) return;
ViewHolder siblingViewHolder =
mRecyclerView.findViewHolderForAdapterPosition(siblingPosDelta + swipePos);
if (siblingViewHolder != null) {
mRecyclerView.updateViewStateForDismiss(dX, siblingViewHolder);
}
} }
} }
...@@ -353,7 +364,7 @@ public class NewTabPageAdapter extends Adapter<NewTabPageViewHolder> ...@@ -353,7 +364,7 @@ public class NewTabPageAdapter extends Adapter<NewTabPageViewHolder>
mGroups.add(mBottomSpacer); mGroups.add(mBottomSpacer);
} }
// TODO(bauerb): Notify about a smaller range. // TODO(bauerb): Notify about a smaller range: https://crbug.com/627512
notifyDataSetChanged(); notifyDataSetChanged();
} }
...@@ -372,13 +383,34 @@ public class NewTabPageAdapter extends Adapter<NewTabPageViewHolder> ...@@ -372,13 +383,34 @@ public class NewTabPageAdapter extends Adapter<NewTabPageViewHolder>
} }
public void dismissItem(int position) { public void dismissItem(int position) {
NewTabPageItem item = getItems().get(position);
if (item instanceof SnippetArticle) {
dismissSuggestion(position);
} else {
// We assume that the item being dismissed is the status card (and/or the more button,
// when it's displayed with the status card). In that case we just dismiss the section
// and it will show up on new NTPs after the status has changed.
ItemGroup group = getGroup(position);
assert group instanceof SuggestionsSection;
dismissSection((SuggestionsSection) group);
}
}
private void dismissSection(SuggestionsSection section) {
mSuggestionsSource.dismissCategory(section.getCategory());
mSections.remove(section.getCategory());
updateGroups();
}
private void dismissSuggestion(int position) {
SnippetArticle suggestion = (SnippetArticle) getItems().get(position); SnippetArticle suggestion = (SnippetArticle) getItems().get(position);
mSuggestionsSource.getSuggestionVisited(suggestion, new Callback<Boolean>() { mSuggestionsSource.getSuggestionVisited(suggestion, new Callback<Boolean>() {
@Override @Override
public void onResult(Boolean result) { public void onResult(Boolean result) {
NewTabPageUma.recordSnippetAction(result NewTabPageUma.recordSnippetAction(result
? NewTabPageUma.SNIPPETS_ACTION_DISMISSED_VISITED ? NewTabPageUma.SNIPPETS_ACTION_DISMISSED_VISITED
: NewTabPageUma.SNIPPETS_ACTION_DISMISSED_UNVISITED); : NewTabPageUma.SNIPPETS_ACTION_DISMISSED_UNVISITED);
} }
}); });
......
...@@ -459,13 +459,15 @@ public class NewTabPageRecyclerView extends RecyclerView { ...@@ -459,13 +459,15 @@ public class NewTabPageRecyclerView extends RecyclerView {
/** /**
* Update the view's state as it is being swiped away. Any changes to the animation here should * Update the view's state as it is being swiped away. Any changes to the animation here should
* be reflected also in {@link #dismissItemWithAnimation(SnippetArticle)} and reset in * be reflected also in {@link #dismissItemWithAnimation(SnippetArticle)} and reset in
* {@link CardViewHolder#onBindViewHolder(NewTabPageItem)}. * {@link CardViewHolder#onBindViewHolder()}.
* @param dX The amount of horizontal displacement caused by user's action. * @param dX The amount of horizontal displacement caused by user's action.
* @param viewHolder The view holder containing the view to be updated. * @param viewHolder The view holder containing the view to be updated.
*/ */
public void updateViewStateForDismiss(float dX, ViewHolder viewHolder) { public void updateViewStateForDismiss(float dX, ViewHolder viewHolder) {
if (!((NewTabPageViewHolder) viewHolder).isDismissable()) return; if (!((NewTabPageViewHolder) viewHolder).isDismissable()) return;
viewHolder.itemView.setTranslationX(dX);
float input = Math.abs(dX) / viewHolder.itemView.getMeasuredWidth(); float input = Math.abs(dX) / viewHolder.itemView.getMeasuredWidth();
float alpha = 1 - DISMISS_INTERPOLATOR.getInterpolation(input); float alpha = 1 - DISMISS_INTERPOLATOR.getInterpolation(input);
viewHolder.itemView.setAlpha(alpha); viewHolder.itemView.setAlpha(alpha);
......
...@@ -67,6 +67,11 @@ public abstract class StatusItem implements NewTabPageItem { ...@@ -67,6 +67,11 @@ public abstract class StatusItem implements NewTabPageItem {
mActionView.setVisibility(View.GONE); mActionView.setVisibility(View.GONE);
} }
} }
@Override
public boolean isDismissable() {
return true;
}
} }
private static class NoBookmarks extends StatusItem { private static class NoBookmarks extends StatusItem {
......
...@@ -4,6 +4,7 @@ ...@@ -4,6 +4,7 @@
package org.chromium.chrome.browser.ntp.cards; package org.chromium.chrome.browser.ntp.cards;
import org.chromium.base.VisibleForTesting;
import org.chromium.chrome.browser.ntp.cards.StatusItem.ActionDelegate; import org.chromium.chrome.browser.ntp.cards.StatusItem.ActionDelegate;
import org.chromium.chrome.browser.ntp.snippets.CategoryInt; import org.chromium.chrome.browser.ntp.snippets.CategoryInt;
import org.chromium.chrome.browser.ntp.snippets.CategoryStatus.CategoryStatusEnum; import org.chromium.chrome.browser.ntp.snippets.CategoryStatus.CategoryStatusEnum;
...@@ -63,6 +64,7 @@ public class SuggestionsSection implements ItemGroup { ...@@ -63,6 +64,7 @@ public class SuggestionsSection implements ItemGroup {
public void removeSuggestion(SnippetArticle suggestion) { public void removeSuggestion(SnippetArticle suggestion) {
mSuggestions.remove(suggestion); mSuggestions.remove(suggestion);
if (mMoreButton != null) mMoreButton.setDismissable(!hasSuggestions());
} }
public void removeSuggestionById(String suggestionId) { public void removeSuggestionById(String suggestionId) {
...@@ -116,4 +118,41 @@ public class SuggestionsSection implements ItemGroup { ...@@ -116,4 +118,41 @@ public class SuggestionsSection implements ItemGroup {
suggestion.setThumbnailBitmap(mSuggestions.get(index).getThumbnailBitmap()); suggestion.setThumbnailBitmap(mSuggestions.get(index).getThumbnailBitmap());
} }
} }
/**
* The dismiss sibling is an item that should be dismissed at the same time as the provided
* one. For example, if we want to dismiss a status card that has a More button attached, the
* button is the card's dismiss sibling. This function return the adapter position delta to
* apply to get to the sibling from the provided item. For the previous example, it would return
* {@code +1}, as the button comes right after the status card.
*
* @return a position delta to apply to the position of the provided item to get the adapter
* position of the item to animate. Returns {@code 0} if there is no dismiss sibling.
*/
public int getDismissSiblingPosDelta(NewTabPageItem item) {
// The only dismiss siblings we have so far are the More button and the status card.
// Exit early if there is no More button.
if (mMoreButton == null) return 0;
// When there are suggestions we won't have contiguous status and action items.
if (hasSuggestions()) return 0;
// The sibling of the more button is the status card, that should be right above.
if (item == mMoreButton) return -1;
// The sibling of the status card is the more button when it exists, should be right below.
if (item == mStatus) return 1;
return 0;
}
@VisibleForTesting
ActionItem getActionItem() {
return mMoreButton;
}
@VisibleForTesting
StatusItem getStatusItem() {
return mStatus;
};
} }
...@@ -96,8 +96,12 @@ public class FakeSuggestionsSource implements SuggestionsSource { ...@@ -96,8 +96,12 @@ public class FakeSuggestionsSource implements SuggestionsSource {
} }
@Override @Override
public void fetchSuggestionImage( public void dismissCategory(@CategoryInt int category) {
SnippetArticle suggestion, Callback<Bitmap> callback) { throw new UnsupportedOperationException();
}
@Override
public void fetchSuggestionImage(SnippetArticle suggestion, Callback<Bitmap> callback) {
if (mThumbnails.containsKey(suggestion.mId)) { if (mThumbnails.containsKey(suggestion.mId)) {
callback.onResult(mThumbnails.get(suggestion.mId)); callback.onResult(mThumbnails.get(suggestion.mId));
} }
......
...@@ -118,8 +118,13 @@ public class SnippetsBridge implements SuggestionsSource { ...@@ -118,8 +118,13 @@ public class SnippetsBridge implements SuggestionsSource {
} }
@Override @Override
public void getSuggestionVisited( public void dismissCategory(@CategoryInt int category) {
SnippetArticle suggestion, Callback<Boolean> callback) { assert mNativeSnippetsBridge != 0;
nativeDismissCategory(mNativeSnippetsBridge, category);
}
@Override
public void getSuggestionVisited(SnippetArticle suggestion, Callback<Boolean> callback) {
assert mNativeSnippetsBridge != 0; assert mNativeSnippetsBridge != 0;
nativeGetURLVisited(mNativeSnippetsBridge, callback, suggestion.mUrl); nativeGetURLVisited(mNativeSnippetsBridge, callback, suggestion.mUrl);
} }
...@@ -232,6 +237,7 @@ public class SnippetsBridge implements SuggestionsSource { ...@@ -232,6 +237,7 @@ public class SnippetsBridge implements SuggestionsSource {
private native void nativeFetchSuggestionImage( private native void nativeFetchSuggestionImage(
long nativeNTPSnippetsBridge, String suggestionId, Callback<Bitmap> callback); long nativeNTPSnippetsBridge, String suggestionId, Callback<Bitmap> callback);
private native void nativeDismissSuggestion(long nativeNTPSnippetsBridge, String suggestionId); private native void nativeDismissSuggestion(long nativeNTPSnippetsBridge, String suggestionId);
private native void nativeDismissCategory(long nativeNTPSnippetsBridge, int category);
private native void nativeGetURLVisited( private native void nativeGetURLVisited(
long nativeNTPSnippetsBridge, Callback<Boolean> callback, String url); long nativeNTPSnippetsBridge, Callback<Boolean> callback, String url);
private native void nativeOnPageShown( private native void nativeOnPageShown(
......
...@@ -69,6 +69,11 @@ public interface SuggestionsSource { ...@@ -69,6 +69,11 @@ public interface SuggestionsSource {
*/ */
void dismissSuggestion(SnippetArticle suggestion); void dismissSuggestion(SnippetArticle suggestion);
/**
* Tells the source to dismiss the category.
*/
void dismissCategory(@CategoryInt int category);
/** /**
* Checks whether a content suggestion has been visited. The callback is never called * Checks whether a content suggestion has been visited. The callback is never called
* synchronously. * synchronously.
......
...@@ -6,6 +6,8 @@ package org.chromium.chrome.browser.ntp.cards; ...@@ -6,6 +6,8 @@ package org.chromium.chrome.browser.ntp.cards;
import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue; import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail; import static org.junit.Assert.fail;
...@@ -655,6 +657,46 @@ public class NewTabPageAdapterTest { ...@@ -655,6 +657,46 @@ public class NewTabPageAdapterTest {
assertEquals(KnownCategories.DOWNLOADS, getCategory(groups.get(3))); assertEquals(KnownCategories.DOWNLOADS, getCategory(groups.get(3)));
} }
@Test
@Feature({"Ntp"})
public void testDismissSibling() {
List<SnippetArticle> snippets = createDummySuggestions(3);
SuggestionsSection section;
// Part 1: ShowMoreButton = true
section = new SuggestionsSection(42,
new SuggestionsCategoryInfo("", ContentSuggestionsCardLayout.FULL_CARD, true, true),
null);
section.setStatus(CategoryStatus.AVAILABLE);
assertNotNull(section.getActionItem());
// 1.1: Without snippets
assertEquals(-1, section.getDismissSiblingPosDelta(section.getActionItem()));
assertEquals(1, section.getDismissSiblingPosDelta(section.getStatusItem()));
// 1.2: With snippets
section.setSuggestions(snippets, CategoryStatus.AVAILABLE);
assertEquals(0, section.getDismissSiblingPosDelta(section.getActionItem()));
assertEquals(0, section.getDismissSiblingPosDelta(section.getStatusItem()));
assertEquals(0, section.getDismissSiblingPosDelta(snippets.get(0)));
// Part 2: ShowMoreButton = false
section = new SuggestionsSection(42,
new SuggestionsCategoryInfo("", ContentSuggestionsCardLayout.FULL_CARD, false,
true),
null);
section.setStatus(CategoryStatus.AVAILABLE);
assertNull(section.getActionItem());
// 2.1: Without snippets
assertEquals(0, section.getDismissSiblingPosDelta(section.getStatusItem()));
// 2.2: With snippets
section.setSuggestions(snippets, CategoryStatus.AVAILABLE);
assertEquals(0, section.getDismissSiblingPosDelta(section.getStatusItem()));
assertEquals(0, section.getDismissSiblingPosDelta(snippets.get(0)));
}
private List<SnippetArticle> createDummySuggestions(int count) { private List<SnippetArticle> createDummySuggestions(int count) {
List<SnippetArticle> suggestions = new ArrayList<>(); List<SnippetArticle> suggestions = new ArrayList<>();
for (int index = 0; index < count; index++) { for (int index = 0; index < count; index++) {
......
...@@ -215,6 +215,12 @@ void NTPSnippetsBridge::DismissSuggestion( ...@@ -215,6 +215,12 @@ void NTPSnippetsBridge::DismissSuggestion(
ConvertJavaStringToUTF8(env, suggestion_id)); ConvertJavaStringToUTF8(env, suggestion_id));
} }
void NTPSnippetsBridge::DismissCategory(JNIEnv* env,
const JavaParamRef<jobject>& obj,
jint category) {
content_suggestions_service_->DismissCategory(CategoryFromIDValue(category));
}
void NTPSnippetsBridge::GetURLVisited(JNIEnv* env, void NTPSnippetsBridge::GetURLVisited(JNIEnv* env,
const JavaParamRef<jobject>& obj, const JavaParamRef<jobject>& obj,
const JavaParamRef<jobject>& jcallback, const JavaParamRef<jobject>& jcallback,
......
...@@ -62,6 +62,10 @@ class NTPSnippetsBridge ...@@ -62,6 +62,10 @@ class NTPSnippetsBridge
const base::android::JavaParamRef<jobject>& obj, const base::android::JavaParamRef<jobject>& obj,
const base::android::JavaParamRef<jstring>& suggestion_id); const base::android::JavaParamRef<jstring>& suggestion_id);
void DismissCategory(JNIEnv* env,
const base::android::JavaParamRef<jobject>& obj,
jint category);
// Checks if the URL has been visited. The callback will not be called // Checks if the URL has been visited. The callback will not be called
// synchronously. // synchronously.
void GetURLVisited(JNIEnv* env, void GetURLVisited(JNIEnv* env,
......
...@@ -162,6 +162,16 @@ void ContentSuggestionsService::DismissSuggestion( ...@@ -162,6 +162,16 @@ void ContentSuggestionsService::DismissSuggestion(
<< " OnNewSuggestions in response to DismissSuggestion."; << " OnNewSuggestions in response to DismissSuggestion.";
} }
void ContentSuggestionsService::DismissCategory(Category category) {
auto providers_it = providers_by_category_.find(category);
if (providers_it == providers_by_category_.end())
return;
providers_by_category_.erase(providers_it);
categories_.erase(
std::find(categories_.begin(), categories_.end(), category));
}
void ContentSuggestionsService::AddObserver(Observer* observer) { void ContentSuggestionsService::AddObserver(Observer* observer) {
observers_.AddObserver(observer); observers_.AddObserver(observer);
} }
...@@ -219,12 +229,10 @@ void ContentSuggestionsService::OnCategoryStatusChanged( ...@@ -219,12 +229,10 @@ void ContentSuggestionsService::OnCategoryStatusChanged(
suggestions_by_category_.erase(category); suggestions_by_category_.erase(category);
} }
if (new_status == CategoryStatus::NOT_PROVIDED) { if (new_status == CategoryStatus::NOT_PROVIDED) {
auto providers_it = providers_by_category_.find(category); DCHECK(providers_by_category_.find(category) !=
DCHECK(providers_it != providers_by_category_.end()); providers_by_category_.end());
DCHECK_EQ(provider, providers_it->second); DCHECK_EQ(provider, providers_by_category_.find(category)->second);
providers_by_category_.erase(providers_it); DismissCategory(category);
categories_.erase(
std::find(categories_.begin(), categories_.end(), category));
} else { } else {
RegisterCategoryIfRequired(provider, category); RegisterCategoryIfRequired(provider, category);
DCHECK_EQ(new_status, provider->GetCategoryStatus(category)); DCHECK_EQ(new_status, provider->GetCategoryStatus(category));
......
...@@ -121,6 +121,10 @@ class ContentSuggestionsService : public KeyedService, ...@@ -121,6 +121,10 @@ class ContentSuggestionsService : public KeyedService,
// This will not trigger an update through the observers. // This will not trigger an update through the observers.
void DismissSuggestion(const std::string& suggestion_id); void DismissSuggestion(const std::string& suggestion_id);
// Dismisses the given |category|, if it exists.
// This will not trigger an update through the observers.
void DismissCategory(Category category);
// Observer accessors. // Observer accessors.
void AddObserver(Observer* observer); void AddObserver(Observer* observer);
void RemoveObserver(Observer* observer); void RemoveObserver(Observer* observer);
......
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