[Mac][Material Design] Adjust bookmark spacing and folder menu drawing.

[Mac][Material Design] Adjust bookmark spacing and folder menu drawing.

This cl adjusts the bookmark bar item spacing and bookmark folder menu
item drawing per sgabriel@.

R=tapted@chromium.org
BUG=631275
Committed: https://crrev.com/5f11f9261224faead4686ba6c33403ac4fca2e9b
Cr-Commit-Position: refs/heads/master@{#413593}

[shrike, 2016-08-03 21:11:39]

PTAL

[tapted, 2016-08-04 02:00:55]

https://codereview.chromium.org/2200303004/diff/1/chrome/browser/ui/cocoa/boo...
File chrome/browser/ui/cocoa/bookmarks/bookmark_bar_folder_controller.mm
(right):

https://codereview.chromium.org/2200303004/diff/1/chrome/browser/ui/cocoa/boo...
chrome/browser/ui/cocoa/bookmarks/bookmark_bar_folder_controller.mm:339:
[scrollView_ setFrame:scrollViewFrame];
I think has the unwanted side-effect of causing the menu NSWindows to lose their
rounded corners -- see http://i.imgur.com/os613QH.png - trimming the scroll view
from top and bottom (as well) may be able to fix it, but that will probably
affect some offsets elsewhere.

[shrike, 2016-08-04 20:54:47]

PTAL

https://codereview.chromium.org/2200303004/diff/1/chrome/browser/ui/cocoa/boo...
File chrome/browser/ui/cocoa/bookmarks/bookmark_bar_folder_controller.mm
(right):

https://codereview.chromium.org/2200303004/diff/1/chrome/browser/ui/cocoa/boo...
chrome/browser/ui/cocoa/bookmarks/bookmark_bar_folder_controller.mm:339:
[scrollView_ setFrame:scrollViewFrame];
On 2016/08/04 02:00:55, tapted wrote:
> I think has the unwanted side-effect of causing the menu NSWindows to lose
their
> rounded corners -- see http://i.imgur.com/os613QH.png - trimming the scroll
view
> from top and bottom (as well) may be able to fix it, but that will probably
> affect some offsets elsewhere.

Thanks for catching that. This new patchset should address the issue.

[tapted, 2016-08-05 06:30:00]

lgtm, although there is one minor visual nit - maybe good for a follow-up since
it's not very noticeable and might be a bit tricky.

the problem is that -[BookmarkBarFolderWindowContentView drawRect] is now
showing through at the top and bottom of the menus. This is drawing a horizontal
gradient (center is lighter than the side edges). So it wasn't noticeable when
the non-gradient MD buttons painted over the light center, but now the light
center peeks out at the top and bottom.

I think that drawRect needs an MD update - maybe just a constant, or borrowing
something from -[GradientButtonCell drawBorderAndFillForTheme:..]

[shrike, 2016-08-08 20:20:15]

On 2016/08/05 06:30:00, tapted wrote:
> lgtm, although there is one minor visual nit - maybe good for a follow-up
since
> it's not very noticeable and might be a bit tricky.
> 
> the problem is that -[BookmarkBarFolderWindowContentView drawRect] is now
> showing through at the top and bottom of the menus. This is drawing a
horizontal
> gradient (center is lighter than the side edges). So it wasn't noticeable when
> the non-gradient MD buttons painted over the light center, but now the light
> center peeks out at the top and bottom.
> 
> I think that drawRect needs an MD update - maybe just a constant, or borrowing
> something from -[GradientButtonCell drawBorderAndFillForTheme:..]

Thanks for catching that one also.

The bookmark folder window on MD does not actually show a gradient because the
bookmark buttons completely cover the background. Now that I've moved the
buttons, the content view gradient is showing. I have changed the folder content
view code to draw the same flat color the buttons do.

PTAL

[tapted, 2016-08-09 02:52:03]

lgtm

https://codereview.chromium.org/2200303004/diff/60001/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/bookmarks/bookmark_bar_folder_window.mm (right):

https://codereview.chromium.org/2200303004/diff/60001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/bookmarks/bookmark_bar_folder_window.mm:22: 
nit: remove extra blank line

(also technically the anonymous namespace is redundant, since `const` also gives
internal linkage, but opinions differ - I don't think this is consistently
applied)

[shrike, 2016-08-09 16:28:50]

https://codereview.chromium.org/2200303004/diff/60001/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/bookmarks/bookmark_bar_folder_window.mm (right):

https://codereview.chromium.org/2200303004/diff/60001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/bookmarks/bookmark_bar_folder_window.mm:22: 
On 2016/08/09 02:52:03, tapted wrote:
> nit: remove extra blank line
> 
> (also technically the anonymous namespace is redundant, since `const` also
gives
> internal linkage, but opinions differ - I don't think this is consistently
> applied)

Done. And I will leave the namespace as is.

[shrike, 2016-08-09 16:28:55]

The CQ bit was checked by shrike@chromium.org

[shrike, 2016-08-09 16:28:56]

The patchset sent to the CQ was uploaded after l-g-t-m from tapted@chromium.org
Link to the patchset: https://codereview.chromium.org/2200303004/#ps80001
(title: "Fix nit.")

[commit-bot: I haz the power, 2016-08-09 16:29:35]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-09 17:34:56]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-09 17:34:57]

Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[shrike, 2016-08-22 23:12:35]

The CQ bit was checked by shrike@chromium.org

[shrike, 2016-08-22 23:12:35]

The patchset sent to the CQ was uploaded after l-g-t-m from tapted@chromium.org
Link to the patchset: https://codereview.chromium.org/2200303004/#ps100001
(title: "Fix tests, changes for icon-only items.")

[commit-bot: I haz the power, 2016-08-22 23:13:41]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-23 00:13:25]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-08-23 00:17:22]

Description was changed from

==========
[Mac][Material Design] Adjust bookmark spacing and folder menu drawing.

This cl adjusts the bookmark bar item spacing and bookmark folder menu
item drawing per sgabriel@.

R=tapted@chromium.org
BUG=631275
==========

to

==========
[Mac][Material Design] Adjust bookmark spacing and folder menu drawing.

This cl adjusts the bookmark bar item spacing and bookmark folder menu
item drawing per sgabriel@.

R=tapted@chromium.org
BUG=631275

Committed: https://crrev.com/5f11f9261224faead4686ba6c33403ac4fca2e9b
Cr-Commit-Position: refs/heads/master@{#413593}
==========

[commit-bot: I haz the power, 2016-08-23 00:17:23]

Patchset 6 (id:??) landed as
https://crrev.com/5f11f9261224faead4686ba6c33403ac4fca2e9b
Cr-Commit-Position: refs/heads/master@{#413593}