Commit 96c7e060 authored by Shakti Sahu's avatar Shakti Sahu Committed by Commit Bot

Read Later : Handle empty reading list

This CL
1 - Fixed a crash when there are no read/unread elements, and added
 more unit tests
2 - Adds IPH event when bookmark star icon pressed, and when
 bookmark folder is seen on bottom sheet

Bug: 1151034
Change-Id: I61d3d95590282b837aa35b9371811c389fcfe1ed
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2548390Reviewed-by: default avatarDavid Trainor <dtrainor@chromium.org>
Reviewed-by: default avatarXing Liu <xingliu@chromium.org>
Commit-Queue: Shakti Sahu <shaktisahu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#829972}
parent a34b20e3
......@@ -1505,6 +1505,9 @@ public abstract class ChromeActivity<C extends ChromeActivityComponent>
return;
}
TrackerFactory.getTrackerForProfile(Profile.getLastUsedRegularProfile())
.notifyEvent(EventConstants.APP_MENU_BOOKMARK_STAR_ICON_PRESSED);
// Note we get user bookmark ID over just a bookmark ID here: Managed bookmarks can't be
// edited. If the current URL is only bookmarked by managed bookmarks, this will return
// INVALID_ID, so the code below will fall back on adding a new bookmark instead.
......
......@@ -365,6 +365,8 @@ public class BookmarkUtils {
// Adds reading list as the first top level folder.
if (bookmarkId.getType() == BookmarkType.READING_LIST) {
topLevelFolders.add(bookmarkId);
TrackerFactory.getTrackerForProfile(Profile.getLastUsedRegularProfile())
.notifyEvent(EventConstants.READ_LATER_BOTTOM_SHEET_FOLDER_SEEN);
continue;
}
BookmarkId parent = bookmarkModel.getBookmarkById(bookmarkId).getParentId();
......
......@@ -15,16 +15,16 @@ import java.util.List;
/**
* Helper class to manage all the logic and UI behind adding the reading list section headers in the
* bookmark content UI.
* TODO(crbug/1147787): Add integration tests.
*/
class ReadingListSectionHeader {
/**
* Sorts the reading list and adds section headers if the list is a reading list.
* Noop, if the list isn't a reading list. The layout rows are shown in the following order :
* 1 - Section header with title "Unread"
* 2 - Unread reading list articles.
* 3 - Section header with title "Read"
* 4 - Read reading list articles.
* 1 - Any promo header
* 2 - Section header with title "Unread"
* 3 - Unread reading list articles.
* 4 - Section header with title "Read"
* 5 - Read reading list articles.
* @param listItems The list of bookmark items to be shown in the UI.
* @param context The associated activity context.
*/
......@@ -41,10 +41,11 @@ class ReadingListSectionHeader {
readingListStartIndex++;
}
assert readingListStartIndex < listItems.size();
// If we have no read/unread elements, exit.
if (readingListStartIndex == listItems.size()) return;
sort(listItems, readingListStartIndex);
// Add a section header at the top.
// Add a section header at the top. If it is for read, exit right away.
assert listItems.get(readingListStartIndex).getBookmarkItem().getId().getType()
== BookmarkType.READING_LIST;
boolean isRead = listItems.get(readingListStartIndex).getBookmarkItem().isRead();
......
......@@ -27,7 +27,7 @@ import java.util.List;
@Config(manifest = Config.NONE)
public class ReadingListSectionHeaderTest {
@Test
public void testAddReadingListSectionHeaders() {
public void testListWithReadUnreadItems() {
Context context = ContextUtils.getApplicationContext();
String titleRead = context.getString(org.chromium.chrome.R.string.reading_list_read);
String titleUnread = context.getString(org.chromium.chrome.R.string.reading_list_unread);
......@@ -53,6 +53,80 @@ public class ReadingListSectionHeaderTest {
"Expected a different item", 2, listItems.get(4).getBookmarkItem().getId().getId());
}
@Test
public void testListWithReadUnreadAndPromoItems() {
Context context = ContextUtils.getApplicationContext();
String titleRead = context.getString(org.chromium.chrome.R.string.reading_list_read);
List<BookmarkListEntry> listItems = new ArrayList<>();
listItems.add(BookmarkListEntry.createSyncPromoHeader(ViewType.PERSONALIZED_SIGNIN_PROMO));
listItems.add(createReadingListEntry(1, true));
listItems.add(createReadingListEntry(2, true));
ReadingListSectionHeader.maybeSortAndInsertSectionHeaders(listItems, context);
assertEquals("Incorrect number of items in the adapter", 4, listItems.size());
assertEquals("Expected promo section header", ViewType.PERSONALIZED_SIGNIN_PROMO,
listItems.get(0).getViewType());
assertEquals("Expected read section header", ViewType.SECTION_HEADER,
listItems.get(1).getViewType());
assertEquals("Expected read title text", titleRead, listItems.get(1).getHeaderTitle());
assertEquals(
"Expected a different item", 1, listItems.get(2).getBookmarkItem().getId().getId());
assertEquals(
"Expected a different item", 2, listItems.get(3).getBookmarkItem().getId().getId());
}
@Test
public void testEmptyList() {
Context context = ContextUtils.getApplicationContext();
List<BookmarkListEntry> listItems = new ArrayList<>();
ReadingListSectionHeader.maybeSortAndInsertSectionHeaders(listItems, context);
assertEquals("Incorrect number of items in the adapter", 0, listItems.size());
}
@Test
public void testNoReadUnreadItems() {
Context context = ContextUtils.getApplicationContext();
List<BookmarkListEntry> listItems = new ArrayList<>();
listItems.add(BookmarkListEntry.createSyncPromoHeader(ViewType.PERSONALIZED_SIGNIN_PROMO));
ReadingListSectionHeader.maybeSortAndInsertSectionHeaders(listItems, context);
assertEquals("Incorrect number of items in the adapter", 1, listItems.size());
assertEquals("Expected promo section header", ViewType.PERSONALIZED_SIGNIN_PROMO,
listItems.get(0).getViewType());
}
@Test
public void testUnreadItemsOnly() {
Context context = ContextUtils.getApplicationContext();
String titleUnread = context.getString(org.chromium.chrome.R.string.reading_list_unread);
List<BookmarkListEntry> listItems = new ArrayList<>();
listItems.add(createReadingListEntry(1, false));
listItems.add(createReadingListEntry(2, false));
ReadingListSectionHeader.maybeSortAndInsertSectionHeaders(listItems, context);
assertEquals("Incorrect number of items in the adapter", 3, listItems.size());
assertEquals(
"Expected section header", ViewType.SECTION_HEADER, listItems.get(0).getViewType());
assertEquals("Expected unread title text", titleUnread, listItems.get(0).getHeaderTitle());
}
@Test
public void testReadItemsOnly() {
Context context = ContextUtils.getApplicationContext();
String titleRead = context.getString(org.chromium.chrome.R.string.reading_list_read);
List<BookmarkListEntry> listItems = new ArrayList<>();
listItems.add(createReadingListEntry(1, true));
ReadingListSectionHeader.maybeSortAndInsertSectionHeaders(listItems, context);
assertEquals("Incorrect number of items in the adapter", 2, listItems.size());
assertEquals(
"Expected section header", ViewType.SECTION_HEADER, listItems.get(0).getViewType());
assertEquals("Expected read title text", titleRead, listItems.get(0).getHeaderTitle());
}
private BookmarkListEntry createReadingListEntry(long id, boolean read) {
BookmarkId bookmarkId = new BookmarkId(id, BookmarkType.READING_LIST);
BookmarkItem bookmarkItem =
......
......@@ -181,8 +181,12 @@ public final class EventConstants {
public static final String TAB_SWITCHER_BUTTON_CLICKED = "tab_switcher_button_clicked";
/** Read later related events. */
public static final String APP_MENU_BOOKMARK_STAR_ICON_PRESSED =
"app_menu_bookmark_star_icon_pressed";
public static final String READ_LATER_CONTEXT_MENU_TAPPED = "read_later_context_menu_tapped";
public static final String READ_LATER_ARTICLE_SAVED = "read_later_article_saved";
public static final String READ_LATER_BOTTOM_SHEET_FOLDER_SEEN =
"read_later_bottom_sheet_folder_seen";
public static final String READ_LATER_BOOKMARK_FOLDER_OPENED =
"read_later_bookmark_folder_opened";
......
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