Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(556)

Issue 159346: Live resize of tabs as the window resizes.... (Closed)

Created:
11 years, 5 months ago by pink (ping after 24hrs)
Modified:
9 years, 7 months ago
CC:
chromium-reviews_googlegroups.com, John Grabowski, Ben Goodger (Google)
Visibility:
Public.

Description

Live resize of tabs as the window resizes. BUG=13713 TEST=dragging tabs still animates, new tabs still animate, resizing window live-resizes tabs. Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=21532

Patch Set 1 #

Patch Set 2 : '' #

Patch Set 3 : '' #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+35 lines, -7 lines) Patch
M chrome/browser/cocoa/tab_strip_controller.h View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/cocoa/tab_strip_controller.mm View 1 2 8 chunks +34 lines, -6 lines 2 comments Download

Messages

Total messages: 7 (0 generated)
pink (ping after 24hrs)
11 years, 5 months ago (2009-07-24 15:30:13 UTC) #1
viettrungluu-gmail
LGTM. Good catch on my missing removeObserver, and your method names are more Obj-C/Cocoa-ish. :-) ...
11 years, 5 months ago (2009-07-24 16:04:46 UTC) #2
pink (ping after 24hrs)
I think they should be low-cost in the non-animation case. I thought about wrapping them ...
11 years, 5 months ago (2009-07-24 16:08:00 UTC) #3
viettrungluu-gmail
On 2009/07/24 16:08:00, pink wrote: > I think they should be low-cost in the non-animation ...
11 years, 5 months ago (2009-07-24 16:09:49 UTC) #4
rohitrao (ping after 24h)
lgtm http://codereview.chromium.org/159346/diff/6/1008 File chrome/browser/cocoa/tab_strip_controller.mm (right): http://codereview.chromium.org/159346/diff/6/1008#newcode261 Line 261: // Move the current tab to the ...
11 years, 5 months ago (2009-07-24 16:11:22 UTC) #5
TVL
lgtm http://codereview.chromium.org/159346/diff/6/1008 File chrome/browser/cocoa/tab_strip_controller.mm (right): http://codereview.chromium.org/159346/diff/6/1008#newcode299 Line 299: id frameTarget = visible && animate ? ...
11 years, 5 months ago (2009-07-24 16:27:02 UTC) #6
TVL
11 years, 5 months ago (2009-07-24 16:29:16 UTC) #7
What about making a C++ stackbased class that wrapped this?  Have it take a bool
in the ctor to disable it, and it's get() can return nil when it's disabled so
the obj methods noop out?  then the code reads, and it could end the group when
it goes out of scope?  Seems like we might need this for other things when we
put animations back in?

TVL

On 2009/07/24 16:08:00, pink wrote:
> I think they should be low-cost in the non-animation case. I thought
> about wrapping them in if()s but thought it might make the code too
> hard to follow.
> 
> On Fri, Jul 24, 2009 at 12:04 PM, <viettrungluu@gmail.com> wrote:
> > LGTM. Good catch on my missing removeObserver, and your method names are
> > more Obj-C/Cocoa-ish. :-) Only question: The calls to
> > [NSAnimationContext beginContext/endContext/etc.] should be pretty
> > low-cost, right?
> >
> > http://codereview.chromium.org/159346
> >
> 
> 
> 
> -- 
> Mike Pinkerton
> Mac Weenie
> pinkerton@google.com

Powered by Google App Engine
This is Rietveld 408576698