New issue
Advanced search Search tips

Issue 839460 link

Starred by 1 user
Status: Fixed
Owner:
sczs@chromium.org
Closed: May 2018
Cc:
sczs@chromium.org
thegreenfrog@chromium.org
rohitrao@chromium.org
ios-bugs-priority@chromium.org
ios-bugs@chromium.org
Components:
EstimatedDays: ----
NextAction: 2018-05-09
OS: iOS
Pri: 0
Type: Task
Q2

Blocking:
issue 839454



Sign in to add a comment

Bookmarks: Convert BookmarkEditViewController to use ChromeTableViewController

Project Member Reported by rohitrao@chromium.org, May 3 2018 
BookmarkEditViewController is currently built on top of CollectionViewController.  We'd like to convert it to use ChromeTableViewController instead.

This will involve writing UITableViewCell subclasses for the various pieces of UI in the edit screen.  It will also require ChromeTableViewController to support MDCAppBar.

The edit view uses BookmarkElevatedToolbar to display contextual actions in a bottom toolbar.  That will need to be ported to the new VC or rewritten to use UINavigationController's bottom toolbar instead.
Project Member

Comment 1 by bugdroid1@chromium.org, May 3 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6d4ada0604de844a1c5206e5f17e3584ce83eec8

commit 6d4ada0604de844a1c5206e5f17e3584ce83eec8
Author: Rohit Rao <rohitrao@chromium.org>
Date: Thu May 03 22:35:42 2018

[ios] Adds MDCAppBar support to ChromeTableViewController.

Initially appbar support was added to SettingsRootTableViewController,
as that was the only view controller which needed it.  But now, we
expect to support MDCAppBar in bookmarks screens as well, so this CL
moves it to a common base class.  We follow the pattern set by
CollectionViewController and include an appBarStyle parameter which
determines whether or not an MDCAppBar is created.

BUG= 839439 , 839450 , 839460 

Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: I057b15113987d91451ca3a104aba95dd17667cda
Reviewed-on: https://chromium-review.googlesource.com/1043006
Commit-Queue: Rohit Rao <rohitrao@chromium.org>
Reviewed-by: Sergio Collazos <sczs@chromium.org>
Cr-Commit-Position: refs/heads/master@{#555892}
[modify] https://crrev.com/6d4ada0604de844a1c5206e5f17e3584ce83eec8/ios/chrome/browser/ui/history/history_table_view_controller.h
[modify] https://crrev.com/6d4ada0604de844a1c5206e5f17e3584ce83eec8/ios/chrome/browser/ui/history/history_table_view_controller.mm
[modify] https://crrev.com/6d4ada0604de844a1c5206e5f17e3584ce83eec8/ios/chrome/browser/ui/popup_menu/popup_menu_coordinator.mm
[modify] https://crrev.com/6d4ada0604de844a1c5206e5f17e3584ce83eec8/ios/chrome/browser/ui/popup_menu/popup_menu_table_view_controller.h
[modify] https://crrev.com/6d4ada0604de844a1c5206e5f17e3584ce83eec8/ios/chrome/browser/ui/popup_menu/popup_menu_table_view_controller.mm
[modify] https://crrev.com/6d4ada0604de844a1c5206e5f17e3584ce83eec8/ios/chrome/browser/ui/recent_tabs/recent_tabs_table_view_controller.h
[modify] https://crrev.com/6d4ada0604de844a1c5206e5f17e3584ce83eec8/ios/chrome/browser/ui/recent_tabs/recent_tabs_table_view_controller.mm
[modify] https://crrev.com/6d4ada0604de844a1c5206e5f17e3584ce83eec8/ios/chrome/browser/ui/settings/clear_browsing_data_table_view_controller.h
[modify] https://crrev.com/6d4ada0604de844a1c5206e5f17e3584ce83eec8/ios/chrome/browser/ui/settings/clear_browsing_data_table_view_controller.mm
[modify] https://crrev.com/6d4ada0604de844a1c5206e5f17e3584ce83eec8/ios/chrome/browser/ui/settings/settings_root_table_view_controller.h
[modify] https://crrev.com/6d4ada0604de844a1c5206e5f17e3584ce83eec8/ios/chrome/browser/ui/settings/settings_root_table_view_controller.mm
[modify] https://crrev.com/6d4ada0604de844a1c5206e5f17e3584ce83eec8/ios/chrome/browser/ui/settings/table_cell_catalog_view_controller.h
[modify] https://crrev.com/6d4ada0604de844a1c5206e5f17e3584ce83eec8/ios/chrome/browser/ui/settings/table_cell_catalog_view_controller.mm
[modify] https://crrev.com/6d4ada0604de844a1c5206e5f17e3584ce83eec8/ios/chrome/browser/ui/table_view/BUILD.gn
[modify] https://crrev.com/6d4ada0604de844a1c5206e5f17e3584ce83eec8/ios/chrome/browser/ui/table_view/chrome_table_view_controller.h
[modify] https://crrev.com/6d4ada0604de844a1c5206e5f17e3584ce83eec8/ios/chrome/browser/ui/table_view/chrome_table_view_controller.mm
[modify] https://crrev.com/6d4ada0604de844a1c5206e5f17e3584ce83eec8/ios/chrome/browser/ui/table_view/chrome_table_view_controller_unittest.mm
Project Member

Comment 2 by bugdroid1@chromium.org, May 8 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e0cf7fc3149e673f93a9081cfc0e6f58c5dd00b5

commit e0cf7fc3149e673f93a9081cfc0e6f58c5dd00b5
Author: sczs <sczs@chromium.org>
Date: Tue May 08 00:45:29 2018

[ios] Converts BookmarkEditVC to TableVC

- BookmarkEditVC subclasses ChromeTableVC instead of CollectionVC.
- BookmarksEditVC, BookmarkFolderVC and BookmarksFolderEditor hide or show the toolbar on
viewWillAppear.

Screenshots:
Old: https://drive.google.com/open?id=18hYFEmdmXv5WemJbwmx3jcP8e4zZRRf_
New: https://drive.google.com/open?id=1cJbQeEGIwMWKiulLk-geEIBVn1_cB-wN

Bug:  839460 
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: I4ce83e551b45fbfce10399f39eadba74add5449b
Reviewed-on: https://chromium-review.googlesource.com/1045887
Commit-Queue: Sergio Collazos <sczs@chromium.org>
Reviewed-by: Rohit Rao <rohitrao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556628}
[modify] https://crrev.com/e0cf7fc3149e673f93a9081cfc0e6f58c5dd00b5/ios/chrome/browser/ui/bookmarks/BUILD.gn
[modify] https://crrev.com/e0cf7fc3149e673f93a9081cfc0e6f58c5dd00b5/ios/chrome/browser/ui/bookmarks/bookmark_edit_view_controller.h
[modify] https://crrev.com/e0cf7fc3149e673f93a9081cfc0e6f58c5dd00b5/ios/chrome/browser/ui/bookmarks/bookmark_edit_view_controller.mm
[modify] https://crrev.com/e0cf7fc3149e673f93a9081cfc0e6f58c5dd00b5/ios/chrome/browser/ui/bookmarks/bookmark_folder_editor_view_controller.h
[modify] https://crrev.com/e0cf7fc3149e673f93a9081cfc0e6f58c5dd00b5/ios/chrome/browser/ui/bookmarks/bookmark_folder_editor_view_controller.mm
[modify] https://crrev.com/e0cf7fc3149e673f93a9081cfc0e6f58c5dd00b5/ios/chrome/browser/ui/bookmarks/bookmark_folder_view_controller.mm
[modify] https://crrev.com/e0cf7fc3149e673f93a9081cfc0e6f58c5dd00b5/ios/chrome/browser/ui/bookmarks/bookmarks_egtest.mm

Comment 3 by rohitrao@chromium.org, May 9 2018

Cc: rohitrao@chromium.org
NextAction: 2018-05-09
Owner: sczs@chromium.org
Status: Started (was: Assigned)
Anything left here, or can we close the bug?

Comment 4 by sczs@chromium.org, May 9 2018

Status: Fixed (was: Started)
There's a cleanup CL in flight, but this task is completed.
►

Sign in to add a comment