Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(104)

Issue 1941583002: [Mac][Material Design] Update bookmarks bar to Material Design. (Closed)

Created:
4 years, 7 months ago by shrike
Modified:
4 years, 7 months ago
CC:
chromium-reviews, tfarina, noyau+watch_chromium.org, oshima+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

[Mac][Material Design] Update bookmarks bar to Material Design. This cl changes the bookmarks bar folder icons, button sizes, and hover state to match the Material Design spec. R=avi@chromium.org,cpu@chromium.org BUG=589941 Committed: https://crrev.com/e757665e44c261568486d052ecf38d79256a522d Cr-Commit-Position: refs/heads/master@{#394165}

Patch Set 1 #

Patch Set 2 : Add 2x bookmarks folder images. #

Patch Set 3 : Checkpoint with semi-working hover state. #

Patch Set 4 : Remove hover code. #

Patch Set 5 : Ready for review. #

Total comments: 2

Patch Set 6 : Fix typo. #

Patch Set 7 : Fix compile problem. #

Patch Set 8 : Partial fix of tests. #

Patch Set 9 : Fix test and nit. #

Patch Set 10 : Fix tests, prepare for test debugging. #

Patch Set 11 : Fix tests, more test debugging. #

Patch Set 12 : Fix tests. #

Patch Set 13 : Fix tests. #

Patch Set 14 : Testing. #

Patch Set 15 : Fix tests. #

Patch Set 16 : Fix tests. #

Patch Set 17 : Convert bookmarks overflow button to MD. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+572 lines, -151 lines) Patch
A chrome/app/theme/default_100_percent/mac/bookmark_bar_folder_managed_white.png View 1 2 3 4 Binary file 0 comments Download
A chrome/app/theme/default_100_percent/mac/bookmark_bar_folder_supervised_white.png View 1 2 3 4 Binary file 0 comments Download
A chrome/app/theme/default_100_percent/mac/bookmark_bar_folder_white.png View 1 2 3 4 Binary file 0 comments Download
A chrome/app/theme/default_200_percent/mac/bookmark_bar_folder_managed_white.png View 1 2 3 4 Binary file 0 comments Download
A chrome/app/theme/default_200_percent/mac/bookmark_bar_folder_supervised_white.png View 1 2 3 4 Binary file 0 comments Download
A chrome/app/theme/default_200_percent/mac/bookmark_bar_folder_white.png View 1 2 3 4 Binary file 0 comments Download
A chrome/app/theme/material_100_percent/mac/bookmark_bar_folder.png View 1 2 3 4 Binary file 0 comments Download
A chrome/app/theme/material_100_percent/mac/bookmark_bar_folder_managed.png View 1 2 3 4 Binary file 0 comments Download
A chrome/app/theme/material_100_percent/mac/bookmark_bar_folder_supervised.png View 1 2 3 4 Binary file 0 comments Download
A chrome/app/theme/material_200_percent/mac/bookmark_bar_folder.png View 1 2 3 4 Binary file 0 comments Download
A chrome/app/theme/material_200_percent/mac/bookmark_bar_folder_managed.png View 1 2 3 4 Binary file 0 comments Download
A chrome/app/theme/material_200_percent/mac/bookmark_bar_folder_supervised.png View 1 2 3 4 Binary file 0 comments Download
M chrome/app/theme/theme_resources.grd View 1 chunk +5 lines, -0 lines 0 comments Download
M chrome/browser/ui/cocoa/bookmarks/bookmark_bar_constants.h View 1 2 3 4 1 chunk +6 lines, -0 lines 0 comments Download
M chrome/browser/ui/cocoa/bookmarks/bookmark_bar_controller.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 5 chunks +19 lines, -7 lines 0 comments Download
M chrome/browser/ui/cocoa/bookmarks/bookmark_bar_controller.mm View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 34 chunks +201 lines, -67 lines 0 comments Download
M chrome/browser/ui/cocoa/bookmarks/bookmark_bar_controller_unittest.mm View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 5 chunks +85 lines, -26 lines 0 comments Download
M chrome/browser/ui/cocoa/bookmarks/bookmark_bar_folder_button_cell_unittest.mm View 1 2 3 4 5 6 7 8 9 2 chunks +4 lines, -3 lines 0 comments Download
M chrome/browser/ui/cocoa/bookmarks/bookmark_bar_folder_controller.mm View 1 2 3 4 7 chunks +12 lines, -8 lines 0 comments Download
M chrome/browser/ui/cocoa/bookmarks/bookmark_bar_folder_controller_unittest.mm View 1 2 3 4 5 6 7 4 chunks +4 lines, -4 lines 0 comments Download
M chrome/browser/ui/cocoa/bookmarks/bookmark_button.mm View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 5 chunks +43 lines, -0 lines 0 comments Download
M chrome/browser/ui/cocoa/bookmarks/bookmark_button_cell.mm View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 5 chunks +98 lines, -14 lines 0 comments Download
M chrome/browser/ui/cocoa/gradient_button_cell.h View 1 2 3 4 2 chunks +4 lines, -0 lines 0 comments Download
M chrome/browser/ui/cocoa/gradient_button_cell.mm View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 9 chunks +91 lines, -22 lines 0 comments Download

Messages

Total messages: 38 (20 generated)
upolat
https://codereview.chromium.org/1941583002/diff/80001/chrome/browser/ui/cocoa/gradient_button_cell.mm File chrome/browser/ui/cocoa/gradient_button_cell.mm (right): https://codereview.chromium.org/1941583002/diff/80001/chrome/browser/ui/cocoa/gradient_button_cell.mm#newcode331 chrome/browser/ui/cocoa/gradient_button_cell.mm:331: // THe user has just clicked down in the ...
4 years, 7 months ago (2016-05-09 23:16:45 UTC) #3
shrike
PTAL avi: chrome/browser/ui/cocoa/bookmarks/bookmark_bar_constants.h chrome/browser/ui/cocoa/bookmarks/bookmark_bar_controller.h chrome/browser/ui/cocoa/bookmarks/bookmark_bar_controller.mm chrome/browser/ui/cocoa/bookmarks/bookmark_bar_controller_unittest.mm chrome/browser/ui/cocoa/bookmarks/bookmark_bar_folder_controller.mm chrome/browser/ui/cocoa/bookmarks/bookmark_bar_folder_controller_unittest.mm chrome/browser/ui/cocoa/bookmarks/bookmark_button.mm chrome/browser/ui/cocoa/bookmarks/bookmark_button_cell.mm chrome/browser/ui/cocoa/gradient_button_cell.h chrome/browser/ui/cocoa/gradient_button_cell.mm cpu: chrome/app/theme/default_100_percent/mac/bookmark_bar_folder_managed_white.png chrome/app/theme/default_100_percent/mac/bookmark_bar_folder_supervised_white.png ...
4 years, 7 months ago (2016-05-09 23:42:37 UTC) #7
Avi (use Gerrit)
lgtm
4 years, 7 months ago (2016-05-10 01:06:09 UTC) #8
shrike
cpu@ - would you please approve addition of these icons to Chrome? This is an ...
4 years, 7 months ago (2016-05-11 19:07:44 UTC) #9
shrike
sky@ - are you able to approve my PNG additions in this cl? I'm trying ...
4 years, 7 months ago (2016-05-11 22:19:47 UTC) #11
cpu_(ooo_6.6-7.5)
chrome/app/theme lgtm
4 years, 7 months ago (2016-05-12 00:27:14 UTC) #12
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1941583002/100001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1941583002/100001
4 years, 7 months ago (2016-05-12 00:38:45 UTC) #14
commit-bot: I haz the power
Try jobs failed on following builders: mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_ng/builds/226296)
4 years, 7 months ago (2016-05-12 00:52:18 UTC) #16
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1941583002/120001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1941583002/120001
4 years, 7 months ago (2016-05-12 02:49:00 UTC) #19
commit-bot: I haz the power
Try jobs failed on following builders: mac_chromium_gn_rel on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_gn_rel/builds/109888)
4 years, 7 months ago (2016-05-12 03:05:21 UTC) #21
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1941583002/160001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1941583002/160001
4 years, 7 months ago (2016-05-12 21:00:10 UTC) #24
commit-bot: I haz the power
Try jobs failed on following builders: mac_chromium_gn_rel on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_gn_rel/builds/110360)
4 years, 7 months ago (2016-05-12 21:48:59 UTC) #26
shrike
avi@ - will you take a look the changes I've made since your approval? Thank ...
4 years, 7 months ago (2016-05-17 16:53:53 UTC) #27
Avi (use Gerrit)
lgtm
4 years, 7 months ago (2016-05-17 16:57:51 UTC) #29
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1941583002/320001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1941583002/320001
4 years, 7 months ago (2016-05-17 16:59:19 UTC) #32
commit-bot: I haz the power
Committed patchset #17 (id:320001)
4 years, 7 months ago (2016-05-17 18:06:17 UTC) #34
commit-bot: I haz the power
Patchset 17 (id:??) landed as https://crrev.com/e757665e44c261568486d052ecf38d79256a522d Cr-Commit-Position: refs/heads/master@{#394165}
4 years, 7 months ago (2016-05-17 18:09:44 UTC) #36
qyearsley
4 years, 7 months ago (2016-05-17 23:14:52 UTC) #37
Message was sent while issue was closed.
A revert of this CL (patchset #17 id:320001) has been created in
https://codereview.chromium.org/1986963004/ by qyearsley@chromium.org.

The reason for reverting is: Appears to have caused failure in unit test
BookmarkBarControllerTest.LastBookmarkResizeBehavior on Mac 10.11

Logs:
https://build.chromium.org/p/chromium.mac/builders/Mac10.11%20Tests/builds/16...

Build:
https://build.chromium.org/p/chromium.mac/builders/Mac10.11%20Tests/builds/1621.

Powered by Google App Engine
This is Rietveld 408576698