Cancel previous new tab "submarine" animations when starting the animation fo...

Animate the new tab button when closing a tab.
BUG=14919
TEST=new tab button is in the correct location.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=28851

[pink (ping after 24hrs), 2009-10-13 16:20:08]

changed to only move the NTB on close tab, removed everything else. please take
another look.

[Mark Mentovai, 2009-10-13 16:26:35]

At first I wasn't sure about the animation but it's already started to grow on
me.

However, I can get the New Tab Button to disappear with this patch when I close
and open tabs rapidly (command-W command-T).  That's no good.

[pink (ping after 24hrs), 2009-10-13 16:32:29]

I can't reproduce this. Any tips?

On Tue, Oct 13, 2009 at 12:26 PM,  <mark@chromium.org> wrote:
> At first I wasn't sure about the animation but it's already started to gr=
ow
> on
> me.
>
> However, I can get the New Tab Button to disappear with this patch when I
> close
> and open tabs rapidly (command-W command-T). =A0That's no good.
>
> http://codereview.chromium.org/243049
>



--=20
Mike Pinkerton
Mac Weenie
pinkerton@google.com

[rohitrao (ping after 24h), 2009-10-13 16:55:04]

http://codereview.chromium.org/243049/diff/7001/7002
File chrome/browser/cocoa/tab_strip_controller.mm (right):

http://codereview.chromium.org/243049/diff/7001/7002#newcode450
Line 450: // if we only have 1 tab.
Is this code supposed to be in this CL?

http://codereview.chromium.org/243049/diff/7001/7002#newcode453
Line 453: [[tab view] setFrame:NSOffsetRect(tabFrame, 0, -NSHeight(tabFrame))];
Should we be looking at shouldAnimate below as well?  I don't quite follow the
animation logic for when there's only one tab.

[pink (ping after 24hrs), 2009-10-13 16:58:57]

http://codereview.chromium.org/243049/diff/7001/7002
File chrome/browser/cocoa/tab_strip_controller.mm (right):

http://codereview.chromium.org/243049/diff/7001/7002#newcode450
Line 450: // if we only have 1 tab.
On 2009/10/13 16:55:04, rohitrao wrote:
> Is this code supposed to be in this CL?

yes, it makes it so that we size the width of the tab correctly before we
animate it so we're not animating both offset and width.

http://codereview.chromium.org/243049/diff/7001/7002#newcode453
Line 453: [[tab view] setFrame:NSOffsetRect(tabFrame, 0, -NSHeight(tabFrame))];
On 2009/10/13 16:55:04, rohitrao wrote:
> Should we be looking at shouldAnimate below as well?  I don't quite follow the
> animation logic for when there's only one tab.

No, because when we go from 2 tabs to 1 tab, we want the NTB to still animate.

[rohitrao (ping after 24h), 2009-10-13 17:09:29]

LGTM, assuming mark doesn't find another way to break it.

[Mark Mentovai, 2009-10-13 17:30:23]

LGTM

http://codereview.chromium.org/243049/diff/13001/13002
File chrome/browser/cocoa/tab_strip_controller.mm (right):

http://codereview.chromium.org/243049/diff/13001/13002#newcode365
Line 365: const float kInstantly = 0.000001;
Can you #include <limits> and use the std::numeric_limits<NSTimeInterval>::min()
trick that I used in status_bubble_mac.mm?  Also, this should be an
NSTimeInterval, not a float.

http://codereview.chromium.org/243049/diff/13001/13002#newcode423
Line 423: [[NSAnimationContext currentContext] setDuration:kInstantly];
Seems kind of funny to say "set duration to instantly" - since the only thing we
use the constant for is "set duration," we might as well make it grammatically
correct.

http://codereview.chromium.org/243049/diff/13001/13002#newcode503
Line 503: [NSAnimationContext beginGrouping];
Adding this is what fixed the bug I was seeing earlier with the disappearing new
tab button.

[pink (ping after 24hrs), 2009-10-13 17:49:11]

All of these fixed.

On Tue, Oct 13, 2009 at 1:30 PM,  <mark@chromium.org> wrote:
> LGTM
>
>
> http://codereview.chromium.org/243049/diff/13001/13002
> File chrome/browser/cocoa/tab_strip_controller.mm (right):
>
> http://codereview.chromium.org/243049/diff/13001/13002#newcode365
> Line 365: const float kInstantly =3D 0.000001;
> Can you #include <limits> and use the
> std::numeric_limits<NSTimeInterval>::min() trick that I used in
> status_bubble_mac.mm? =A0Also, this should be an NSTimeInterval, not a
> float.
>
> http://codereview.chromium.org/243049/diff/13001/13002#newcode423
> Line 423: [[NSAnimationContext currentContext] setDuration:kInstantly];
> Seems kind of funny to say "set duration to instantly" - since the only
> thing we use the constant for is "set duration," we might as well make
> it grammatically correct.
>
> http://codereview.chromium.org/243049/diff/13001/13002#newcode503
> Line 503: [NSAnimationContext beginGrouping];
> Adding this is what fixed the bug I was seeing earlier with the
> disappearing new tab button.
>
> http://codereview.chromium.org/243049
>



--=20
Mike Pinkerton
Mac Weenie
pinkerton@google.com