Commit b5443388 authored by Maxime Charland's avatar Maxime Charland Committed by Commit Bot

Chrome Empties: Illustrated Empty State in Bookmarks root

The new illustrated empty state in Bookmarks root will happen when a user does not have any bookmarks or folder. This means that the Mobile Bookmarks folder will not be created when it is empty.

In addition to this change, a few other changes are needed :

- The Mobile Bookmarks TableViewItem should not be created if the folder is empty.

- When the state of the Sign In Promo is changed, the TableView background should be updated.

- When a bookmark is deleted, the root TableView should be updated to display an empty state if the user has deleted their last bookmark.

- Because the empty state is loaded right after Chrome has finished loading the user's bookmarks, there is a possibility that the spinnerView is no longer the background view when the spinnerView has finished its animation. Therefore, we should only remove the TableView background if the spinnerView is still the active background.

(Googlers only links)
Screenshot: https://drive.google.com/file/d/1gzEm1IazTYK23UispwRF4zNQIn6Dd3f-/view?usp=sharing
Design Doc for Chrome Empties: https://docs.google.com/document/d/1JM2sKT4oghkol51U7_3xv9sfaB26C89CCGa2uwYBgsI/edit?usp=sharing
Mocks of the new Chrome Empties look: https://docs.google.com/presentation/d/1x1lSTOSQ1zkJu87SoigiUhX2f9oHACglR22yYg-lSHo/edit?usp=sharing

Bug: 1098328
Change-Id: I05d7938d7193e6dfd34d075778eee0dd3b75003c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2304536
Commit-Queue: Maxime Charland <mcharland@google.com>
Reviewed-by: default avatarTommy Martino <tmartino@chromium.org>
Reviewed-by: default avatarSergio Collazos <sczs@chromium.org>
Cr-Commit-Position: refs/heads/master@{#792267}
parent f6d76fd4
...@@ -109,6 +109,8 @@ id<GREYMatcher> SearchIconButton(); ...@@ -109,6 +109,8 @@ id<GREYMatcher> SearchIconButton();
- (void)verifyEmptyBackgroundAppears; - (void)verifyEmptyBackgroundAppears;
- (void)verifyEmptyState;
- (void)verifyBookmarkFolderIsSeen:(NSString*)bookmarkFolder; - (void)verifyBookmarkFolderIsSeen:(NSString*)bookmarkFolder;
// Scroll the bookmarks to bottom. // Scroll the bookmarks to bottom.
......
...@@ -369,6 +369,20 @@ id<GREYMatcher> SearchIconButton() { ...@@ -369,6 +369,20 @@ id<GREYMatcher> SearchIconButton() {
assertWithMatcher:grey_sufficientlyVisible()]; assertWithMatcher:grey_sufficientlyVisible()];
} }
- (void)verifyEmptyState {
[self verifyEmptyBackgroundAppears];
id<GREYInteraction> searchBar =
[EarlGrey selectElementWithMatcher:grey_accessibilityTrait(
UIAccessibilityTraitSearchField)];
if (base::FeatureList::IsEnabled(kIllustratedEmptyStates)) {
// With the illustrated empty state, the search bar should be hidden.
[searchBar assertWithMatcher:grey_nil()];
} else {
[searchBar assertWithMatcher:grey_notNil()];
}
}
- (void)verifyBookmarkFolderIsSeen:(NSString*)bookmarkFolder { - (void)verifyBookmarkFolderIsSeen:(NSString*)bookmarkFolder {
[[EarlGrey [[EarlGrey
selectElementWithMatcher:grey_allOf( selectElementWithMatcher:grey_allOf(
......
...@@ -28,6 +28,7 @@ ...@@ -28,6 +28,7 @@
#import "ios/chrome/browser/ui/bookmarks/synced_bookmarks_bridge.h" #import "ios/chrome/browser/ui/bookmarks/synced_bookmarks_bridge.h"
#import "ios/chrome/browser/ui/table_view/cells/table_view_text_item.h" #import "ios/chrome/browser/ui/table_view/cells/table_view_text_item.h"
#import "ios/chrome/browser/ui/table_view/table_view_model.h" #import "ios/chrome/browser/ui/table_view/table_view_model.h"
#import "ios/chrome/browser/ui/ui_feature_flags.h"
#import "ios/chrome/common/ui/colors/UIColor+cr_semantic_colors.h" #import "ios/chrome/common/ui/colors/UIColor+cr_semantic_colors.h"
#import "ios/public/provider/chrome/browser/signin/signin_presenter.h" #import "ios/public/provider/chrome/browser/signin/signin_presenter.h"
...@@ -177,6 +178,12 @@ const int kMaxBookmarksSearchResults = 50; ...@@ -177,6 +178,12 @@ const int kMaxBookmarksSearchResults = 50;
// Generate the table view data when the current root node is the outermost // Generate the table view data when the current root node is the outermost
// root. // root.
- (void)generateTableViewDataForRootNode { - (void)generateTableViewDataForRootNode {
// If all the permanent nodes are empty, do not create items for any of them.
if (base::FeatureList::IsEnabled(kIllustratedEmptyStates) &&
![self hasBookmarksOrFolders]) {
return;
}
// Add "Mobile Bookmarks" to the table. // Add "Mobile Bookmarks" to the table.
const BookmarkNode* mobileNode = const BookmarkNode* mobileNode =
self.sharedState.bookmarkModel->mobile_node(); self.sharedState.bookmarkModel->mobile_node();
...@@ -276,6 +283,10 @@ const int kMaxBookmarksSearchResults = 50; ...@@ -276,6 +283,10 @@ const int kMaxBookmarksSearchResults = 50;
_syncedBookmarksObserver->IsPerformingInitialSync()) { _syncedBookmarksObserver->IsPerformingInitialSync()) {
[self.consumer [self.consumer
updateTableViewBackgroundStyle:BookmarkHomeBackgroundStyleLoading]; updateTableViewBackgroundStyle:BookmarkHomeBackgroundStyleLoading];
} else if (base::FeatureList::IsEnabled(kIllustratedEmptyStates) &&
![self hasBookmarksOrFolders]) {
[self.consumer
updateTableViewBackgroundStyle:BookmarkHomeBackgroundStyleEmpty];
} else { } else {
[self.consumer [self.consumer
updateTableViewBackgroundStyle:BookmarkHomeBackgroundStyleDefault]; updateTableViewBackgroundStyle:BookmarkHomeBackgroundStyleDefault];
...@@ -336,6 +347,11 @@ const int kMaxBookmarksSearchResults = 50; ...@@ -336,6 +347,11 @@ const int kMaxBookmarksSearchResults = 50;
removeSectionWithIdentifier:BookmarkHomeSectionIdentifierPromo]; removeSectionWithIdentifier:BookmarkHomeSectionIdentifierPromo];
} }
[self.sharedState.tableView reloadData]; [self.sharedState.tableView reloadData];
// Update the TabelView background to make sure the new state of the promo
// does not affect the background.
if (base::FeatureList::IsEnabled(kIllustratedEmptyStates)) {
[self updateTableViewBackground];
}
} }
#pragma mark - BookmarkModelBridgeObserver Callbacks #pragma mark - BookmarkModelBridgeObserver Callbacks
...@@ -504,6 +520,20 @@ const int kMaxBookmarksSearchResults = 50; ...@@ -504,6 +520,20 @@ const int kMaxBookmarksSearchResults = 50;
#pragma mark - Private Helpers #pragma mark - Private Helpers
- (BOOL)hasBookmarksOrFolders { - (BOOL)hasBookmarksOrFolders {
if (base::FeatureList::IsEnabled(kIllustratedEmptyStates) &&
self.sharedState.tableViewDisplayedRootNode ==
self.sharedState.bookmarkModel->root_node()) {
// The root node always has its permanent nodes. If all the permanent nodes
// are empty, we treat it as if the root itself is empty.
const auto& childrenOfRootNode =
self.sharedState.tableViewDisplayedRootNode->children();
for (const auto& child : childrenOfRootNode) {
if (!child->children().empty()) {
return YES;
}
}
return NO;
}
return self.sharedState.tableViewDisplayedRootNode && return self.sharedState.tableViewDisplayedRootNode &&
!self.sharedState.tableViewDisplayedRootNode->children().empty(); !self.sharedState.tableViewDisplayedRootNode->children().empty();
} }
......
...@@ -266,7 +266,7 @@ std::vector<GURL> GetUrlsToOpen(const std::vector<const BookmarkNode*>& nodes) { ...@@ -266,7 +266,7 @@ std::vector<GURL> GetUrlsToOpen(const std::vector<const BookmarkNode*>& nodes) {
// This method is only designed to be called for the view controller // This method is only designed to be called for the view controller
// associated with the root node. // associated with the root node.
DCHECK(self.bookmarks->loaded()); DCHECK(self.bookmarks->loaded());
DCHECK_EQ(_rootNode, self.bookmarks->root_node()); DCHECK([self isDisplayingBookmarkRoot]);
NSMutableArray<BookmarkHomeViewController*>* stack = [NSMutableArray array]; NSMutableArray<BookmarkHomeViewController*>* stack = [NSMutableArray array];
// Configure the root controller Navigationbar at this time when // Configure the root controller Navigationbar at this time when
...@@ -391,13 +391,20 @@ std::vector<GURL> GetUrlsToOpen(const std::vector<const BookmarkNode*>& nodes) { ...@@ -391,13 +391,20 @@ std::vector<GURL> GetUrlsToOpen(const std::vector<const BookmarkNode*>& nodes) {
// Hide the toolbar if we're displaying the root node. // Hide the toolbar if we're displaying the root node.
if (self.bookmarks->loaded() && if (self.bookmarks->loaded() &&
(_rootNode != self.bookmarks->root_node() || (![self isDisplayingBookmarkRoot] ||
self.sharedState.currentlyShowingSearchResults)) { self.sharedState.currentlyShowingSearchResults)) {
self.navigationController.toolbarHidden = NO; self.navigationController.toolbarHidden = NO;
} else { } else {
self.navigationController.toolbarHidden = YES; self.navigationController.toolbarHidden = YES;
} }
// If we navigate back to the root level, we need to make sure the root level
// folders are created or deleted if needed.
if (base::FeatureList::IsEnabled(kIllustratedEmptyStates) &&
[self isDisplayingBookmarkRoot]) {
[self refreshContents];
}
// Center search bar's cancel button vertically so it looks centered. // Center search bar's cancel button vertically so it looks centered.
// We change the cancel button proxy styles, so we will return it to // We change the cancel button proxy styles, so we will return it to
// default in viewDidDisappear. // default in viewDidDisappear.
...@@ -766,7 +773,7 @@ std::vector<GURL> GetUrlsToOpen(const std::vector<const BookmarkNode*>& nodes) { ...@@ -766,7 +773,7 @@ std::vector<GURL> GetUrlsToOpen(const std::vector<const BookmarkNode*>& nodes) {
// is set to regular size, which means the search bar is at same level at // is set to regular size, which means the search bar is at same level at
// beginning and end of animation. This controller will be replaced in // beginning and end of animation. This controller will be replaced in
// |stack| so there's no need to care about restoring this. // |stack| so there's no need to care about restoring this.
if (_rootNode == self.bookmarks->root_node()) { if ([self isDisplayingBookmarkRoot]) {
self.navigationItem.largeTitleDisplayMode = self.navigationItem.largeTitleDisplayMode =
UINavigationItemLargeTitleDisplayModeNever; UINavigationItemLargeTitleDisplayModeNever;
} }
...@@ -965,7 +972,12 @@ std::vector<GURL> GetUrlsToOpen(const std::vector<const BookmarkNode*>& nodes) { ...@@ -965,7 +972,12 @@ std::vector<GURL> GetUrlsToOpen(const std::vector<const BookmarkNode*>& nodes) {
strongSelf.spinnerView.alpha = 0.0; strongSelf.spinnerView.alpha = 0.0;
} }
completion:^(BOOL finished) { completion:^(BOOL finished) {
self.sharedState.tableView.backgroundView = nil; // By the time completion block is called, the backgroundView could be
// another view, like the empty view background. Only clear the
// background if it is still the spinner.
if (self.sharedState.tableView.backgroundView == self.spinnerView) {
self.sharedState.tableView.backgroundView = nil;
}
self.spinnerView = nil; self.spinnerView = nil;
}]; }];
[strongSelf loadBookmarkViews]; [strongSelf loadBookmarkViews];
...@@ -1007,6 +1019,10 @@ std::vector<GURL> GetUrlsToOpen(const std::vector<const BookmarkNode*>& nodes) { ...@@ -1007,6 +1019,10 @@ std::vector<GURL> GetUrlsToOpen(const std::vector<const BookmarkNode*>& nodes) {
#pragma mark - private #pragma mark - private
- (BOOL)isDisplayingBookmarkRoot {
return _rootNode == self.bookmarks->root_node();
}
// Check if any of our controller is presenting. We don't consider when this // Check if any of our controller is presenting. We don't consider when this
// controller is presenting the search controller. // controller is presenting the search controller.
// Note that when adding a controller that can present, it should be added in // Note that when adding a controller that can present, it should be added in
...@@ -1029,7 +1045,7 @@ std::vector<GURL> GetUrlsToOpen(const std::vector<const BookmarkNode*>& nodes) { ...@@ -1029,7 +1045,7 @@ std::vector<GURL> GetUrlsToOpen(const std::vector<const BookmarkNode*>& nodes) {
// Set up context bar for the new UI. // Set up context bar for the new UI.
- (void)setupContextBar { - (void)setupContextBar {
if (_rootNode != self.bookmarks->root_node() || if (![self isDisplayingBookmarkRoot] ||
self.sharedState.currentlyShowingSearchResults) { self.sharedState.currentlyShowingSearchResults) {
self.navigationController.toolbarHidden = NO; self.navigationController.toolbarHidden = NO;
[self setContextBarState:BookmarksContextBarDefault]; [self setContextBarState:BookmarksContextBarDefault];
...@@ -1223,9 +1239,7 @@ std::vector<GURL> GetUrlsToOpen(const std::vector<const BookmarkNode*>& nodes) { ...@@ -1223,9 +1239,7 @@ std::vector<GURL> GetUrlsToOpen(const std::vector<const BookmarkNode*>& nodes) {
- (int)topMostVisibleIndexPathRow { - (int)topMostVisibleIndexPathRow {
// If on root node screen, return 0. // If on root node screen, return 0.
if (self.sharedState.bookmarkModel && if (self.sharedState.bookmarkModel && [self isDisplayingBookmarkRoot]) {
self.sharedState.tableViewDisplayedRootNode ==
self.sharedState.bookmarkModel->root_node()) {
return 0; return 0;
} }
...@@ -1405,7 +1419,12 @@ std::vector<GURL> GetUrlsToOpen(const std::vector<const BookmarkNode*>& nodes) { ...@@ -1405,7 +1419,12 @@ std::vector<GURL> GetUrlsToOpen(const std::vector<const BookmarkNode*>& nodes) {
self.spinnerView.alpha = 0.0; self.spinnerView.alpha = 0.0;
} }
completion:^(BOOL finished) { completion:^(BOOL finished) {
self.sharedState.tableView.backgroundView = nil; // By the time completion block is called, the backgroundView could
// be another view, like the empty view background. Only clear the
// background if it is still the spinner.
if (self.sharedState.tableView.backgroundView == self.spinnerView) {
self.sharedState.tableView.backgroundView = nil;
}
self.spinnerView = nil; self.spinnerView = nil;
}]; }];
}]; }];
...@@ -1422,6 +1441,23 @@ std::vector<GURL> GetUrlsToOpen(const std::vector<const BookmarkNode*>& nodes) { ...@@ -1422,6 +1441,23 @@ std::vector<GURL> GetUrlsToOpen(const std::vector<const BookmarkNode*>& nodes) {
title:l10n_util::GetNSString(IDS_IOS_BOOKMARK_EMPTY_TITLE) title:l10n_util::GetNSString(IDS_IOS_BOOKMARK_EMPTY_TITLE)
subtitle:l10n_util::GetNSString(IDS_IOS_BOOKMARK_EMPTY_MESSAGE)]; subtitle:l10n_util::GetNSString(IDS_IOS_BOOKMARK_EMPTY_MESSAGE)];
} }
// If the Signin promo is visible on the root view, we have to shift the
// empty TableView background to make it fully visible on all devices.
if ([self isDisplayingBookmarkRoot]) {
self.navigationItem.largeTitleDisplayMode =
UINavigationItemLargeTitleDisplayModeNever;
if (self.sharedState.promoVisible &&
self.sharedState.tableView.visibleCells.count) {
CGFloat signinPromoHeight = self.sharedState.tableView.visibleCells
.firstObject.bounds.size.height;
self.emptyViewBackground.scrollViewContentInsets =
UIEdgeInsetsMake(signinPromoHeight, 0.0, 0.0, 0.0);
} else {
self.emptyViewBackground.scrollViewContentInsets =
self.view.safeAreaInsets;
}
}
self.sharedState.tableView.backgroundView = self.emptyViewBackground; self.sharedState.tableView.backgroundView = self.emptyViewBackground;
self.navigationItem.searchController = nil; self.navigationItem.searchController = nil;
} else { } else {
...@@ -1444,6 +1480,10 @@ std::vector<GURL> GetUrlsToOpen(const std::vector<const BookmarkNode*>& nodes) { ...@@ -1444,6 +1480,10 @@ std::vector<GURL> GetUrlsToOpen(const std::vector<const BookmarkNode*>& nodes) {
if (self.sharedState.tableView.backgroundView == self.emptyViewBackground) { if (self.sharedState.tableView.backgroundView == self.emptyViewBackground) {
self.sharedState.tableView.backgroundView = nil; self.sharedState.tableView.backgroundView = nil;
} }
if ([self isDisplayingBookmarkRoot]) {
self.navigationItem.largeTitleDisplayMode =
UINavigationItemLargeTitleDisplayModeAutomatic;
}
self.navigationItem.searchController = self.searchController; self.navigationItem.searchController = self.searchController;
} else { } else {
self.sharedState.tableView.backgroundView = nil; self.sharedState.tableView.backgroundView = nil;
...@@ -2013,7 +2053,7 @@ std::vector<GURL> GetUrlsToOpen(const std::vector<const BookmarkNode*>& nodes) { ...@@ -2013,7 +2053,7 @@ std::vector<GURL> GetUrlsToOpen(const std::vector<const BookmarkNode*>& nodes) {
// No reorering with filtered results or when displaying the top-most // No reorering with filtered results or when displaying the top-most
// Bookmarks node. // Bookmarks node.
if (self.sharedState.currentlyShowingSearchResults || if (self.sharedState.currentlyShowingSearchResults ||
_rootNode == self.bookmarks->root_node() || !self.tableView.editing) { [self isDisplayingBookmarkRoot] || !self.tableView.editing) {
return NO; return NO;
} }
TableViewItem* item = TableViewItem* item =
......
...@@ -849,16 +849,48 @@ using chrome_test_util::TappableBookmarkNodeWithLabel; ...@@ -849,16 +849,48 @@ using chrome_test_util::TappableBookmarkNodeWithLabel;
[[EarlGrey selectElementWithMatcher:grey_accessibilityID(@"Folder 1.1")] [[EarlGrey selectElementWithMatcher:grey_accessibilityID(@"Folder 1.1")]
performAction:grey_tap()]; performAction:grey_tap()];
// Empty TableView background should be visible // Empty TableView background should be visible.
[BookmarkEarlGreyUI verifyEmptyBackgroundAppears]; [BookmarkEarlGreyUI verifyEmptyState];
id<GREYInteraction> searchBar = }
[EarlGrey selectElementWithMatcher:grey_accessibilityTrait(
UIAccessibilityTraitSearchField)]; // Test to make sure the Mobile Bookmarks folder is not created if empty.
- (void)testRootEmptyState {
[BookmarkEarlGreyUI openBookmarks];
if (base::FeatureList::IsEnabled(kIllustratedEmptyStates)) {
// When the user has no bookmarks, the root view should be an empty state.
[BookmarkEarlGreyUI verifyEmptyState];
} else {
// Mobile Bookmark should be visible, even when empty.
[BookmarkEarlGreyUI verifyBookmarkFolderIsSeen:@"Mobile Bookmarks"];
}
}
// When deleting the last bookmark, the root view should be empty when
// navigating back.
- (void)testRootEmptyStateAfterAllBookmarkDeleted {
[BookmarkEarlGrey setupStandardBookmarks];
[BookmarkEarlGreyUI openBookmarks];
[BookmarkEarlGreyUI openMobileBookmarks];
// Delete all bookmarks and folders under Mobile Bookmarks.
[BookmarkEarlGrey removeBookmarkWithTitle:@"Folder 1.1"];
[BookmarkEarlGrey removeBookmarkWithTitle:@"Folder 1"];
[BookmarkEarlGrey removeBookmarkWithTitle:@"French URL"];
[BookmarkEarlGrey removeBookmarkWithTitle:@"Second URL"];
[BookmarkEarlGrey removeBookmarkWithTitle:@"First URL"];
// Navigate back to the root view.
[[EarlGrey selectElementWithMatcher:chrome_test_util::
BookmarksNavigationBarBackButton()]
performAction:grey_tap()];
if (base::FeatureList::IsEnabled(kIllustratedEmptyStates)) { if (base::FeatureList::IsEnabled(kIllustratedEmptyStates)) {
// With the illustrated background, the search bar should not be visible // When the user has no bookmarks, the root view should be an empty state.
[searchBar assertWithMatcher:grey_nil()]; [BookmarkEarlGreyUI verifyEmptyState];
} else { } else {
[searchBar assertWithMatcher:grey_notNil()]; // Mobile Bookmark should be visible, even when empty.
[BookmarkEarlGreyUI verifyBookmarkFolderIsSeen:@"Mobile Bookmarks"];
} }
} }
......
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