New issue
Advanced search Search tips

Issue 839435 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: May 2018
Cc:
Components:
EstimatedDays: ----
NextAction: 2018-05-23
OS: iOS
Pri: 0
Type: Task
Q2

Blocking:
issue 805166



Sign in to add a comment

Bookmarks: Add an abstraction layer for BookmarkContextBar and BookmarkElevatedToolbar

Project Member Reported by rohitrao@chromium.org, May 3 2018

Issue description

These two toolbars are owned by the various bookmarks VCs.  The new UI will use the standard bottom toolbar that is provided by UINavigationController, so we will need to handle these existing bars in one of two ways:

1) Remove these classes and instead use the UINavigationController bottom bar.  This will visibly change the existing bookmarks UI, but that might be ok for one milestone.

2) Add a delegate that abstracts away the specific toolbar implementation and moves the toolbars themselves into a higher-level container.  This would be similar to the proposed TableViewContaining protocol from https://chromium-review.googlesource.com/c/chromium/src/+/1015333.
 
Cc: rohitrao@chromium.org
NextAction: 2018-05-09
Owner: sczs@chromium.org
We're converging on approach #1.  Sczs is investigating how hard it would be to tweak the style of the UIToolbar to look more like the original bars.

He's also added some fixes to ensure that the toolbar is properly shown and hidden as needed, based on whether or not it has any visible items.  We'll keep an eye on this to make sure we didn't miss any cases.
Status: Started (was: Assigned)
Project Member

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

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

commit 2370cef0b55b23a0758273c42c7ffc5458847cc9
Author: sczs <sczs@chromium.org>
Date: Thu May 10 18:30:34 2018

[ios] Updates BookmarkEditorVCs Toolbar UI

- Makes some small changes in order to make BookmarkEditVC and BookmarkFolderEditorVC navigation
toolbars appeareance more similar to the ElevatedToolbar they were using before.

Screenshots:
Old: https://drive.google.com/open?id=1uagROckekOJXPjwxu6HdhGfXUEoZtZ7v
New: https://drive.google.com/open?id=1btpW_JxZ-Kad0U0fgPtGuYrD03FceXQq

Bug:  839435 
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: Ifbfc7ca44dad96bad6d6fc55788ecb1c670580a3
Reviewed-on: https://chromium-review.googlesource.com/1053390
Commit-Queue: Sergio Collazos <sczs@chromium.org>
Reviewed-by: Rohit Rao <rohitrao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#557595}
[modify] https://crrev.com/2370cef0b55b23a0758273c42c7ffc5458847cc9/ios/chrome/browser/ui/bookmarks/bookmark_edit_view_controller.mm
[modify] https://crrev.com/2370cef0b55b23a0758273c42c7ffc5458847cc9/ios/chrome/browser/ui/bookmarks/bookmark_folder_editor_view_controller.mm

Project Member

Comment 4 by bugdroid1@chromium.org, May 12 2018

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

commit c52143f5c6363dbc4685c437edaa8ab8475ac976
Author: sczs <sczs@chromium.org>
Date: Sat May 12 04:49:46 2018

[ios] Deletes BookmarkContextBar and uses UIToolbar instead

- Deletes BookmarkContextBar files and usage from BookmarkHomeVC.
- Uses BookmarkHomeVC navigationController UIToolbar instead.
- Creates accessibilityIdentifiers for toolbar buttons.
- Changes the BookmarkTableCell margins since these were giving the effect of shrinking row height.

Screenshots
Old: https://drive.google.com/open?id=1v3GQRiiPiIzJNZ7Lxd-pgpaaqP15b0PR
New: https://drive.google.com/open?id=1z5mOT3p1NBj62EaMKfR2P5WSD2irQmyX

Bug:  839435 
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: I155365b6f9520010153ce97187ef1f6a2a8514f7
Reviewed-on: https://chromium-review.googlesource.com/1054123
Commit-Queue: Sergio Collazos <sczs@chromium.org>
Reviewed-by: Rohit Rao <rohitrao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#558114}
[modify] https://crrev.com/c52143f5c6363dbc4685c437edaa8ab8475ac976/ios/chrome/browser/ui/bookmarks/BUILD.gn
[delete] https://crrev.com/a242af0746e1d24e778692cffdaeb07821dd2527/ios/chrome/browser/ui/bookmarks/bars/BUILD.gn
[delete] https://crrev.com/a242af0746e1d24e778692cffdaeb07821dd2527/ios/chrome/browser/ui/bookmarks/bars/bookmark_context_bar.h
[delete] https://crrev.com/a242af0746e1d24e778692cffdaeb07821dd2527/ios/chrome/browser/ui/bookmarks/bars/bookmark_context_bar.mm
[modify] https://crrev.com/c52143f5c6363dbc4685c437edaa8ab8475ac976/ios/chrome/browser/ui/bookmarks/bookmark_home_view_controller.h
[modify] https://crrev.com/c52143f5c6363dbc4685c437edaa8ab8475ac976/ios/chrome/browser/ui/bookmarks/bookmark_home_view_controller.mm
[modify] https://crrev.com/c52143f5c6363dbc4685c437edaa8ab8475ac976/ios/chrome/browser/ui/bookmarks/bookmark_home_view_controller_unittest.mm
[modify] https://crrev.com/c52143f5c6363dbc4685c437edaa8ab8475ac976/ios/chrome/browser/ui/bookmarks/bookmark_ui_constants.h
[modify] https://crrev.com/c52143f5c6363dbc4685c437edaa8ab8475ac976/ios/chrome/browser/ui/bookmarks/bookmark_ui_constants.mm
[modify] https://crrev.com/c52143f5c6363dbc4685c437edaa8ab8475ac976/ios/chrome/browser/ui/bookmarks/bookmarks_egtest.mm
[modify] https://crrev.com/c52143f5c6363dbc4685c437edaa8ab8475ac976/ios/chrome/browser/ui/bookmarks/cells/bookmark_table_cell.mm

Status: Fixed (was: Started)
These classes are now gone and have been replaced by UIToolbar.
NextAction: 2018-05-23
The NextAction date has arrived: 2018-05-23

Sign in to add a comment