Adjust toolbar height on Retina (Mac).

Adjust toolbar height on Retina (Mac).

On Retina the toolbar's controls appear to be 1pt too high within the
toolbar area. This cl reduces the height of the toolbar on Retina by
1pt so that the toolbar controls are vertically centered. This cl also
increases the height of the bookmarks bar by 1pt to compensate for the
lost space above it.

BUG=326245
Committed: https://crrev.com/76f2d447a74c6e8a33a6ea654b53a2059f0760f6
Cr-Commit-Position: refs/heads/master@{#377601}

[shrike, 2016-02-23 01:16:49]

Description was changed from

==========
Adjust toolbar height on Retina (Mac).

On Retina the toolbar's controls appear to be 1pt too high within the
toolbar area. This cl reduces the height of the toolbar on Retina by
1pt so that the toolbar controls are vertically centered. This cl also
increases the height of the bookmarks bar by 1pt to compensate for the
lost space above it.

BUG=326245
==========

to

==========
Adjust toolbar height on Retina (Mac).

On Retina the toolbar's controls appear to be 1pt too high within the
toolbar area. This cl reduces the height of the toolbar on Retina by
1pt so that the toolbar controls are vertically centered. This cl also
increases the height of the bookmarks bar by 1pt to compensate for the
lost space above it.

BUG=326245
==========

[shrike, 2016-02-23 01:16:49]

shrike@chromium.org changed reviewers:
+ asvitkine@chromium.org

[shrike, 2016-02-23 01:16:56]

PTAL

[Alexei Svitkine (slow), 2016-02-23 01:20:34]

lgtm

Please post screenshot w/ the fix on the crbug. Thanks!

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

https://codereview.chromium.org/1722883002/diff/1/chrome/browser/ui/cocoa/boo...
chrome/browser/ui/cocoa/bookmarks/bookmark_bar_controller.mm:1047: // appear in
the right place. See https://crbug.com/326245 .
Nit: Please make this comment refer to the change in toolbar_controller.mm and
make those comments point back here.

[shrike, 2016-02-23 01:23:54]

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

https://codereview.chromium.org/1722883002/diff/1/chrome/browser/ui/cocoa/boo...
chrome/browser/ui/cocoa/bookmarks/bookmark_bar_controller.mm:1047: // appear in
the right place. See https://crbug.com/326245 .
On 2016/02/23 01:20:34, Alexei Svitkine (slow) wrote:
> Nit: Please make this comment refer to the change in toolbar_controller.mm and
> make those comments point back here.

If I understand you correctly, you are saying that the comments in each file
should point back to each other?

[Alexei Svitkine (slow), 2016-02-23 01:29:58]

Yep. So if someone wants to update the logic, they know to also update the
other location.
On 22 Feb 2016 5:23 p.m., <shrike@chromium.org> wrote:

>
>
>
https://codereview.chromium.org/1722883002/diff/1/chrome/browser/ui/cocoa/boo...
> File chrome/browser/ui/cocoa/bookmarks/bookmark_bar_controller.mm
> (right):
>
>
>
https://codereview.chromium.org/1722883002/diff/1/chrome/browser/ui/cocoa/boo...
> chrome/browser/ui/cocoa/bookmarks/bookmark_bar_controller.mm:1047: //
> appear in the right place. See https://crbug.com/326245 .
> On 2016/02/23 01:20:34, Alexei Svitkine (slow) wrote:
> > Nit: Please make this comment refer to the change in
> toolbar_controller.mm and
> > make those comments point back here.
>
> If I understand you correctly, you are saying that the comments in each
> file should point back to each other?
>
> https://codereview.chromium.org/1722883002/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[shrike, 2016-02-25 02:00:03]

On 2016/02/23 01:29:58, Alexei Svitkine (slow) wrote:
> Yep. So if someone wants to update the logic, they know to also update the
> other location.

Makes sense (I just wanted to be sure you weren't saying something different
that that).

[shrike, 2016-02-25 16:47:33]

The CQ bit was checked by shrike@chromium.org

[shrike, 2016-02-25 16:47:33]

The patchset sent to the CQ was uploaded after l-g-t-m from
asvitkine@chromium.org
Link to the patchset: https://codereview.chromium.org/1722883002/#ps20001
(title: "Add more info to comments.")

[commit-bot: I haz the power, 2016-02-25 16:50:59]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1722883002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1722883002/20001

[commit-bot: I haz the power, 2016-02-25 17:31:49]

Description was changed from

==========
Adjust toolbar height on Retina (Mac).

On Retina the toolbar's controls appear to be 1pt too high within the
toolbar area. This cl reduces the height of the toolbar on Retina by
1pt so that the toolbar controls are vertically centered. This cl also
increases the height of the bookmarks bar by 1pt to compensate for the
lost space above it.

BUG=326245
==========

to

==========
Adjust toolbar height on Retina (Mac).

On Retina the toolbar's controls appear to be 1pt too high within the
toolbar area. This cl reduces the height of the toolbar on Retina by
1pt so that the toolbar controls are vertically centered. This cl also
increases the height of the bookmarks bar by 1pt to compensate for the
lost space above it.

BUG=326245
==========

[commit-bot: I haz the power, 2016-02-25 17:31:50]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-02-25 17:32:57]

Description was changed from

==========
Adjust toolbar height on Retina (Mac).

On Retina the toolbar's controls appear to be 1pt too high within the
toolbar area. This cl reduces the height of the toolbar on Retina by
1pt so that the toolbar controls are vertically centered. This cl also
increases the height of the bookmarks bar by 1pt to compensate for the
lost space above it.

BUG=326245
==========

to

==========
Adjust toolbar height on Retina (Mac).

On Retina the toolbar's controls appear to be 1pt too high within the
toolbar area. This cl reduces the height of the toolbar on Retina by
1pt so that the toolbar controls are vertically centered. This cl also
increases the height of the bookmarks bar by 1pt to compensate for the
lost space above it.

BUG=326245

Committed: https://crrev.com/76f2d447a74c6e8a33a6ea654b53a2059f0760f6
Cr-Commit-Position: refs/heads/master@{#377601}
==========

[commit-bot: I haz the power, 2016-02-25 17:32:58]

Patchset 2 (id:??) landed as
https://crrev.com/76f2d447a74c6e8a33a6ea654b53a2059f0760f6
Cr-Commit-Position: refs/heads/master@{#377601}