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

Issue 6621056: mac: UI tweaks to tab overview mode (Closed)

Created:
9 years, 9 months ago by Nico
Modified:
9 years, 7 months ago
Reviewers:
viettrungluu
CC:
chromium-reviews, pam+watch_chromium.org
Visibility:
Public.

Description

mac: UI tweaks to tab overview mode BUG=50307 TEST=tab overview mode looks different Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=77227

Patch Set 1 #

Patch Set 2 : . #

Total comments: 2

Patch Set 3 : . #

Patch Set 4 : . #

Patch Set 5 : . #

Total comments: 9
Unified diffs Side-by-side diffs Delta from patch set Stats (+46 lines, -10 lines) Patch
M chrome/browser/ui/cocoa/tabpose_window.h View 1 2 1 chunk +3 lines, -0 lines 0 comments Download
M chrome/browser/ui/cocoa/tabpose_window.mm View 1 2 3 4 6 chunks +43 lines, -10 lines 9 comments Download

Messages

Total messages: 7 (0 generated)
Nico
9 years, 9 months ago (2011-03-08 00:57:15 UTC) #1
viettrungluu
LJBGETM with nits. http://codereview.chromium.org/6621056/diff/4/chrome/browser/ui/cocoa/tabpose_window.mm File chrome/browser/ui/cocoa/tabpose_window.mm (right): http://codereview.chromium.org/6621056/diff/4/chrome/browser/ui/cocoa/tabpose_window.mm#newcode65 chrome/browser/ui/cocoa/tabpose_window.mm:65: CGPoint topLeft = CGPointMake(0.0, self.bounds.size.height); NSHeight? ...
9 years, 9 months ago (2011-03-08 01:02:32 UTC) #2
Nico
On Mon, Mar 7, 2011 at 5:02 PM, <viettrungluu@chromium.org> wrote: > LJBGETM with nits. > ...
9 years, 9 months ago (2011-03-08 01:03:47 UTC) #3
Nico
Hey look, I uploaded the wrong CL for review. PTAL? The code is nicer _and_ ...
9 years, 9 months ago (2011-03-08 01:22:10 UTC) #4
viettrungluu
Still LJBGETM with nits. http://codereview.chromium.org/6621056/diff/4007/chrome/browser/ui/cocoa/tabpose_window.mm File chrome/browser/ui/cocoa/tabpose_window.mm (right): http://codereview.chromium.org/6621056/diff/4007/chrome/browser/ui/cocoa/tabpose_window.mm#newcode40 chrome/browser/ui/cocoa/tabpose_window.mm:40: const int kBottomGradientHeight = 50; ...
9 years, 9 months ago (2011-03-08 01:33:40 UTC) #5
viettrungluu
http://codereview.chromium.org/6621056/diff/4007/chrome/browser/ui/cocoa/tabpose_window.mm File chrome/browser/ui/cocoa/tabpose_window.mm (right): http://codereview.chromium.org/6621056/diff/4007/chrome/browser/ui/cocoa/tabpose_window.mm#newcode81 chrome/browser/ui/cocoa/tabpose_window.mm:81: CGPoint topLeft = CGPointMake(0.0, self.bounds.size.height); On 2011/03/08 01:33:40, viettrungluu ...
9 years, 9 months ago (2011-03-08 01:44:58 UTC) #6
Nico
9 years, 9 months ago (2011-03-08 01:47:02 UTC) #7
Thanks for the barely comprehensible comments chock full of sightly better than
worthless quality!

Submitting.

http://codereview.chromium.org/6621056/diff/4007/chrome/browser/ui/cocoa/tabp...
File chrome/browser/ui/cocoa/tabpose_window.mm (right):

http://codereview.chromium.org/6621056/diff/4007/chrome/browser/ui/cocoa/tabp...
chrome/browser/ui/cocoa/tabpose_window.mm:40: const int kBottomGradientHeight =
50;
On 2011/03/08 01:33:40, viettrungluu wrote:
> You really should have some comment about what the gradients are and what the
> constants mean. Otherwise, you might as well call these things
> |kMagicConstantNumberN|, N = 1, 2, ....

Done. (I maintain that a variable name is a valid medium for information
transport.)

http://codereview.chromium.org/6621056/diff/4007/chrome/browser/ui/cocoa/tabp...
chrome/browser/ui/cocoa/tabpose_window.mm:57: CGFloat startGray_;
On 2011/03/08 01:33:40, viettrungluu wrote:
> @private?

Done.

http://codereview.chromium.org/6621056/diff/4007/chrome/browser/ui/cocoa/tabp...
chrome/browser/ui/cocoa/tabpose_window.mm:81: CGPoint topLeft = CGPointMake(0.0,
self.bounds.size.height);
On 2011/03/08 01:33:40, viettrungluu wrote:
> NSHeight()?

NSHeight still doesn't work with CGRects

Powered by Google App Engine
This is Rietveld 408576698