Fix alignment of title/content on the OIB / Site Settings bubble.

Fix alignment of title/content on the OIB / Site Settings bubble.

r442683 adopted a dynamic inset for the title in bubble dialogs in order
to align with the content under Harmony. Shortly after, r442779 landed
and adopted the default bubble title view for the site settings bubble,
rather than providing a custom one. Title and content should always be
aligned on the site settings bubble (regardless of Harmony), so it now
needs to be aware of whether the title in the dialog is inset or not.

Harmony will align title and content by default. Otherwise, to fix
alignment for the site settings bubble, explicitly set the title margins
to match the content margins.

BUG=681081
Review-Url: https://codereview.chromium.org/2640593002
Cr-Commit-Position: refs/heads/master@{#444474}
Committed: https://chromium.googlesource.com/chromium/src/+/57b697aa2605b6bbcb97352a32ccdb28204739e9

[tapted, 2017-01-17 23:44:33]

The CQ bit was checked by tapted@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-17 23:45:12]

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

[tapted, 2017-01-18 17:03:53]

The CQ bit was checked by tapted@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-18 17:04:12]

Dry run: 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, 2017-01-18 17:54:53]

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

[commit-bot: I haz the power, 2017-01-18 17:54:55]

Dry run: This issue passed the CQ dry run.

[tapted, 2017-01-18 18:00:47]

Description was changed from

==========
Fix alignment of title/content on the OIB / Site Settings bubble.

BUG=681081
==========

to

==========
Fix alignment of title/content on the OIB / Site Settings bubble.

r442683 adopted a dynamic inset for the title in bubble dialogs in order
to align with the content under Harmony. Shortly after, r442779 landed
and adopted the default bubble title view for the site settings bubble,
rather than providing a custom one. Title and content should always be
aligned on the site settings bubble (regardless of Harmony), so it now
needs to be aware of  whether the title in the dialog is inset or not.

Harmony will align title and content by default. Otherwise, to fix
alignment for the site settings bubble, explicitly set the title margins
to match the content margins.

BUG=681081
==========

[tapted, 2017-01-18 18:00:47]

tapted@chromium.org changed reviewers:
+ msw@chromium.org

[tapted, 2017-01-18 18:01:12]

Hi mike, please take a look
(kylixrd cc FYI)

Thanks!

[tapted, 2017-01-18 18:01:39]

Description was changed from

==========
Fix alignment of title/content on the OIB / Site Settings bubble.

r442683 adopted a dynamic inset for the title in bubble dialogs in order
to align with the content under Harmony. Shortly after, r442779 landed
and adopted the default bubble title view for the site settings bubble,
rather than providing a custom one. Title and content should always be
aligned on the site settings bubble (regardless of Harmony), so it now
needs to be aware of  whether the title in the dialog is inset or not.

Harmony will align title and content by default. Otherwise, to fix
alignment for the site settings bubble, explicitly set the title margins
to match the content margins.

BUG=681081
==========

to

==========
Fix alignment of title/content on the OIB / Site Settings bubble.

r442683 adopted a dynamic inset for the title in bubble dialogs in order
to align with the content under Harmony. Shortly after, r442779 landed
and adopted the default bubble title view for the site settings bubble,
rather than providing a custom one. Title and content should always be
aligned on the site settings bubble (regardless of Harmony), so it now
needs to be aware of whether the title in the dialog is inset or not.

Harmony will align title and content by default. Otherwise, to fix
alignment for the site settings bubble, explicitly set the title margins
to match the content margins.

BUG=681081
==========

[msw, 2017-01-18 18:30:13]

lgtm with minor optional nits, thanks for the fix

https://codereview.chromium.org/2640593002/diff/40001/ui/views/bubble/bubble_...
File ui/views/bubble/bubble_dialog_delegate.cc (right):

https://codereview.chromium.org/2640593002/diff/40001/ui/views/bubble/bubble_...
ui/views/bubble/bubble_dialog_delegate.cc:107: BubbleFrameView* frame = new
BubbleFrameView(title_margins_, margins());
optional nit: use |margins_| or |title_margins()| for consistency

https://codereview.chromium.org/2640593002/diff/40001/ui/views/bubble/bubble_...
ui/views/bubble/bubble_dialog_delegate.cc:220: : gfx::Insets(kPanelVertMargin,
kPanelHorizMargin,
optional nit: use 2-arg ctor

[tapted, 2017-01-18 19:49:34]

Thanks Mike!

https://codereview.chromium.org/2640593002/diff/40001/ui/views/bubble/bubble_...
File ui/views/bubble/bubble_dialog_delegate.cc (right):

https://codereview.chromium.org/2640593002/diff/40001/ui/views/bubble/bubble_...
ui/views/bubble/bubble_dialog_delegate.cc:107: BubbleFrameView* frame = new
BubbleFrameView(title_margins_, margins());
On 2017/01/18 18:30:12, msw wrote:
> optional nit: use |margins_| or |title_margins()| for consistency

Done.

https://codereview.chromium.org/2640593002/diff/40001/ui/views/bubble/bubble_...
ui/views/bubble/bubble_dialog_delegate.cc:220: : gfx::Insets(kPanelVertMargin,
kPanelHorizMargin,
On 2017/01/18 18:30:12, msw wrote:
> optional nit: use 2-arg ctor

Done.

[tapted, 2017-01-18 19:50:10]

The CQ bit was checked by tapted@chromium.org

[tapted, 2017-01-18 19:50:10]

The patchset sent to the CQ was uploaded after l-g-t-m from msw@chromium.org
Link to the patchset: https://codereview.chromium.org/2640593002/#ps60001
(title: "respond to comments")

[commit-bot: I haz the power, 2017-01-18 19:51:05]

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, 2017-01-18 20:51:10]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1484769010568590,
"parent_rev": "c5b4bd26a37ac077a93699c1a575076299dc983a", "commit_rev":
"57b697aa2605b6bbcb97352a32ccdb28204739e9"}

[commit-bot: I haz the power, 2017-01-18 20:51:38]

Description was changed from

==========
Fix alignment of title/content on the OIB / Site Settings bubble.

r442683 adopted a dynamic inset for the title in bubble dialogs in order
to align with the content under Harmony. Shortly after, r442779 landed
and adopted the default bubble title view for the site settings bubble,
rather than providing a custom one. Title and content should always be
aligned on the site settings bubble (regardless of Harmony), so it now
needs to be aware of whether the title in the dialog is inset or not.

Harmony will align title and content by default. Otherwise, to fix
alignment for the site settings bubble, explicitly set the title margins
to match the content margins.

BUG=681081
==========

to

==========
Fix alignment of title/content on the OIB / Site Settings bubble.

r442683 adopted a dynamic inset for the title in bubble dialogs in order
to align with the content under Harmony. Shortly after, r442779 landed
and adopted the default bubble title view for the site settings bubble,
rather than providing a custom one. Title and content should always be
aligned on the site settings bubble (regardless of Harmony), so it now
needs to be aware of whether the title in the dialog is inset or not.

Harmony will align title and content by default. Otherwise, to fix
alignment for the site settings bubble, explicitly set the title margins
to match the content margins.

BUG=681081

Review-Url: https://codereview.chromium.org/2640593002
Cr-Commit-Position: refs/heads/master@{#444474}
Committed:
https://chromium.googlesource.com/chromium/src/+/57b697aa2605b6bbcb97352a32cc...
==========

[commit-bot: I haz the power, 2017-01-18 20:51:40]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/57b697aa2605b6bbcb97352a32cc...