When animating a content setting bubble smaller, hide the background later.

When animating a content setting bubble smaller, hide the background later.

We were hiding when the label size reached 0.  That winds up hiding the
background when the icon has both the internal spacing and the trailing padding
adjacent to it before the background ends.  Instead, wait to hide the background
until the internal spacing portion of that is collapsed, so it's just
<leading padding><icon><trailing padding>.

This isn't very noticeable when the internal spacing is 2 px.  It will be more
noticeable when I later increase the spacing (which is how I saw this).

This allows moving label layout in IconLabelBubbleView() downward since we no
longer care about sizing the label to 0 early.  I think the code reads more
clearly that way.

BUG=none
TEST=none

Committed: https://crrev.com/24c8b5c843dd95d3dd0753855d2de19d9b2d92a0
Cr-Commit-Position: refs/heads/master@{#383922}

[Peter Kasting, 2016-03-25 09:58:15]

Description was changed from

==========
When animating a content setting bubble smaller, hide the background later.

We were hiding when the label size reached 0.  That winds up hiding the
background when the icon has both the internal spacing and the trailing padding
adjacent to it before the background ends.  Instead, wait to hide the background
until the internal spacing portion of that is collapsed, so it's just
<leading padding><icon><trailing padding>.

This isn't very noticeable when the internal spacing is 2 px.  It will be more
noticeable when I later increase the spacing (which is how I saw this).

This allows moving label layout in IconLabelBubbleView() downward since we no
longer care about sizing the label to 0 early.  I think the code reads more
clearly that way.

BUG=none
TEST=none
==========

to

==========
When animating a content setting bubble smaller, hide the background later.

We were hiding when the label size reached 0.  That winds up hiding the
background when the icon has both the internal spacing and the trailing padding
adjacent to it before the background ends.  Instead, wait to hide the background
until the internal spacing portion of that is collapsed, so it's just
<leading padding><icon><trailing padding>.

This isn't very noticeable when the internal spacing is 2 px.  It will be more
noticeable when I later increase the spacing (which is how I saw this).

This allows moving label layout in IconLabelBubbleView() downward since we no
longer care about sizing the label to 0 early.  I think the code reads more
clearly that way.

BUG=none
TEST=none
==========

[Peter Kasting, 2016-03-25 09:58:16]

pkasting@chromium.org changed reviewers:
+ varkha@chromium.org

[Peter Kasting, 2016-03-25 10:13:05]

https://codereview.chromium.org/1830343003/diff/20001/chrome/browser/ui/views...
File chrome/browser/ui/views/location_bar/icon_label_bubble_view.cc (right):

https://codereview.chromium.org/1830343003/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/location_bar/icon_label_bubble_view.cc:163: std::max(0,
width() - label_x - bubble_trailing_padding);
Despite using |image_x| and |bubble_trailing_padding| in place of the old
GetOuterPadding() calls, this does the same thing; these values will only differ
from those in cases where the label isn't visible.

[varkha, 2016-03-30 03:43:20]

lgtm.

https://codereview.chromium.org/1830343003/diff/60001/chrome/browser/ui/views...
File chrome/browser/ui/views/location_bar/icon_label_bubble_view.cc (right):

https://codereview.chromium.org/1830343003/diff/60001/chrome/browser/ui/views...
chrome/browser/ui/views/location_bar/icon_label_bubble_view.cc:128:
!is_extension_icon_ || ui::MaterialDesignController::IsModeMaterial();
nit: Do you still need the check for IsModeMaterial() given that with MD
GetOuterPadding(true) = GetOuterPadding(false)?

[Peter Kasting, 2016-03-30 03:57:55]

https://codereview.chromium.org/1830343003/diff/60001/chrome/browser/ui/views...
File chrome/browser/ui/views/location_bar/icon_label_bubble_view.cc (right):

https://codereview.chromium.org/1830343003/diff/60001/chrome/browser/ui/views...
chrome/browser/ui/views/location_bar/icon_label_bubble_view.cc:128:
!is_extension_icon_ || ui::MaterialDesignController::IsModeMaterial();
On 2016/03/30 03:43:19, varkha wrote:
> nit: Do you still need the check for IsModeMaterial() given that with MD
> GetOuterPadding(true) = GetOuterPadding(false)?

Yes, because in the next change set that will no longer be true (due to
adjusting for built-in padding in the image with the leading value).

[Peter Kasting, 2016-03-30 06:49:29]

The CQ bit was checked by pkasting@chromium.org

[Peter Kasting, 2016-03-30 06:49:30]

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

[commit-bot: I haz the power, 2016-03-30 06:50:01]

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

[commit-bot: I haz the power, 2016-03-30 07:37:07]

Description was changed from

==========
When animating a content setting bubble smaller, hide the background later.

We were hiding when the label size reached 0.  That winds up hiding the
background when the icon has both the internal spacing and the trailing padding
adjacent to it before the background ends.  Instead, wait to hide the background
until the internal spacing portion of that is collapsed, so it's just
<leading padding><icon><trailing padding>.

This isn't very noticeable when the internal spacing is 2 px.  It will be more
noticeable when I later increase the spacing (which is how I saw this).

This allows moving label layout in IconLabelBubbleView() downward since we no
longer care about sizing the label to 0 early.  I think the code reads more
clearly that way.

BUG=none
TEST=none
==========

to

==========
When animating a content setting bubble smaller, hide the background later.

We were hiding when the label size reached 0.  That winds up hiding the
background when the icon has both the internal spacing and the trailing padding
adjacent to it before the background ends.  Instead, wait to hide the background
until the internal spacing portion of that is collapsed, so it's just
<leading padding><icon><trailing padding>.

This isn't very noticeable when the internal spacing is 2 px.  It will be more
noticeable when I later increase the spacing (which is how I saw this).

This allows moving label layout in IconLabelBubbleView() downward since we no
longer care about sizing the label to 0 early.  I think the code reads more
clearly that way.

BUG=none
TEST=none
==========

[commit-bot: I haz the power, 2016-03-30 07:37:08]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-03-30 07:39:05]

Description was changed from

==========
When animating a content setting bubble smaller, hide the background later.

We were hiding when the label size reached 0.  That winds up hiding the
background when the icon has both the internal spacing and the trailing padding
adjacent to it before the background ends.  Instead, wait to hide the background
until the internal spacing portion of that is collapsed, so it's just
<leading padding><icon><trailing padding>.

This isn't very noticeable when the internal spacing is 2 px.  It will be more
noticeable when I later increase the spacing (which is how I saw this).

This allows moving label layout in IconLabelBubbleView() downward since we no
longer care about sizing the label to 0 early.  I think the code reads more
clearly that way.

BUG=none
TEST=none
==========

to

==========
When animating a content setting bubble smaller, hide the background later.

We were hiding when the label size reached 0.  That winds up hiding the
background when the icon has both the internal spacing and the trailing padding
adjacent to it before the background ends.  Instead, wait to hide the background
until the internal spacing portion of that is collapsed, so it's just
<leading padding><icon><trailing padding>.

This isn't very noticeable when the internal spacing is 2 px.  It will be more
noticeable when I later increase the spacing (which is how I saw this).

This allows moving label layout in IconLabelBubbleView() downward since we no
longer care about sizing the label to 0 early.  I think the code reads more
clearly that way.

BUG=none
TEST=none

Committed: https://crrev.com/24c8b5c843dd95d3dd0753855d2de19d9b2d92a0
Cr-Commit-Position: refs/heads/master@{#383922}
==========

[commit-bot: I haz the power, 2016-03-30 07:39:06]

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