Use a real download item....

Use a real download item.

BUG=14659, 17100
TEST=Download multiple things. Items should now be inserted from the left. When a download is in progress, the time to completion should be displayed, when it's done the time should fade out and the filename should move down. Download items should appear in a sweep animation. Clicking the download directly should open it; clicking the arrow on the right of the download item should show the correct context menu. If a download filename is long, it should be elided in the middle.


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

[Avi (use Gerrit), 2009-07-21 14:57:18]

http://codereview.chromium.org/159060/diff/1090/1094
File chrome/browser/cocoa/download_item_cell.h (right):

http://codereview.chromium.org/159060/diff/1090/1094#newcode23
Line 23: DI_MOUSE_OVER_DROPDOWN_PART
Is this right stylewise? Definitely indent two; and I thought the style guide
preferred constant-style names (kWhateverPart) over the old-school macro naming.

http://codereview.chromium.org/159060/diff/1090/1094#newcode44
Line 44: @property (retain) NSString* secondaryTitle;
String properties are copied, not retained.

http://codereview.chromium.org/159060/diff/1090/1095
File chrome/browser/cocoa/download_item_cell.mm (right):

http://codereview.chromium.org/159060/diff/1090/1095#newcode18
Line 18: // Distance fromt op bordert o icon
wei rdspa cing

http://codereview.chromium.org/159060/diff/1090/1095#newcode21
Line 21: // Distance from left bordert o icon
an dagai n

http://codereview.chromium.org/159060/diff/1090/1095#newcode106
Line 106: [self setInitialState];  }
} goes on next line.

http://codereview.chromium.org/159060/diff/1090/1095#newcode148
Line 148: // Use two distinct tracking rects for left and right par
par? parts?

http://codereview.chromium.org/159060/diff/1090/1095#newcode261
Line 261: // Constants from Cole.  Will kConstant   hem oncet he feedback loop
Infection of the weir dspac ingaga in

http://codereview.chromium.org/159060/diff/1090/1095#newcode342
Line 342: [color colorWithAlphaComponent:statusAlpha_],
NSForegroundColorAttributeName,
80col

http://codereview.chromium.org/159060/diff/1090/1095#newcode361
Line 361: // Popup arrow. Put center of mass oft he arrow int he center oft he
Or maybe you need to get a new keyboard; does your space bar work?

http://codereview.chromium.org/159060/diff/1090/1093
File chrome/browser/cocoa/download_shelf_controller.mm (right):

http://codereview.chromium.org/159060/diff/1090/1093#newcode188
Line 188: for (DownloadItemController* itemController in
downloadItemControllers_.get()) {
80col

http://codereview.chromium.org/159060/diff/1090/1097
File chrome/browser/cocoa/gradient_button_cell.mm (right):

http://codereview.chromium.org/159060/diff/1090/1097#newcode111
Line 111: // Drawt he inner glow.
Ahhh.. I get it now. The letter t from the word "the" shifts left.

http://codereview.chromium.org/159060/diff/1090/1097#newcode119
Line 119: // Drawt het op inner highlight.
Or is it the letter t starting any word that starts with t?

[Nico, 2009-07-21 16:32:00]

Thanks for the very quick first round!

http://codereview.chromium.org/159060/diff/1090/1094
File chrome/browser/cocoa/download_item_cell.h (right):

http://codereview.chromium.org/159060/diff/1090/1094#newcode23
Line 23: DI_MOUSE_OVER_DROPDOWN_PART
On 2009/07/21 14:57:18, Avi wrote:
> Is this right stylewise? Definitely indent two; and I thought the style guide
> preferred constant-style names (kWhateverPart) over the old-school macro
naming.

Done.

http://codereview.chromium.org/159060/diff/1090/1094#newcode44
Line 44: @property (retain) NSString* secondaryTitle;
On 2009/07/21 14:57:18, Avi wrote:
> String properties are copied, not retained.

Done.

http://codereview.chromium.org/159060/diff/1090/1095
File chrome/browser/cocoa/download_item_cell.mm (right):

http://codereview.chromium.org/159060/diff/1090/1095#newcode18
Line 18: // Distance fromt op bordert o icon
On 2009/07/21 14:57:18, Avi wrote:
> wei rdspa cing

Done.

http://codereview.chromium.org/159060/diff/1090/1095#newcode21
Line 21: // Distance from left bordert o icon
On 2009/07/21 14:57:18, Avi wrote:
> an dagai n

Done.

http://codereview.chromium.org/159060/diff/1090/1095#newcode106
Line 106: [self setInitialState];  }
On 2009/07/21 14:57:18, Avi wrote:
> } goes on next line.

Whoops.

http://codereview.chromium.org/159060/diff/1090/1095#newcode148
Line 148: // Use two distinct tracking rects for left and right par
On 2009/07/21 14:57:18, Avi wrote:
> par? parts?

Done.

http://codereview.chromium.org/159060/diff/1090/1095#newcode261
Line 261: // Constants from Cole.  Will kConstant   hem oncet he feedback loop
On 2009/07/21 14:57:18, Avi wrote:
> Infection of the weir dspac ingaga in

Yeah, I had an embarrassing incident with `sed` – turns out doing `s/[\t]/  /g`
is pretty stupid. Looks like I didn't clean everything up correctly :-P

Done.

http://codereview.chromium.org/159060/diff/1090/1095#newcode342
Line 342: [color colorWithAlphaComponent:statusAlpha_],
NSForegroundColorAttributeName,
On 2009/07/21 14:57:18, Avi wrote:
> 80col

Done.

http://codereview.chromium.org/159060/diff/1090/1095#newcode361
Line 361: // Popup arrow. Put center of mass oft he arrow int he center oft he
On 2009/07/21 14:57:18, Avi wrote:
> Or maybe you need to get a new keyboard; does your space bar work?

Done.

http://codereview.chromium.org/159060/diff/1090/1097
File chrome/browser/cocoa/gradient_button_cell.mm (right):

http://codereview.chromium.org/159060/diff/1090/1097#newcode111
Line 111: // Drawt he inner glow.
On 2009/07/21 14:57:18, Avi wrote:
> Ahhh.. I get it now. The letter t from the word "the" shifts left.

Done.

http://codereview.chromium.org/159060/diff/1090/1097#newcode119
Line 119: // Drawt het op inner highlight.
On 2009/07/21 14:57:18, Avi wrote:
> Or is it the letter t starting any word that starts with t?

Basically, I accidentally replaced every 't' in download_item_cell with two
spaces (without making a backup), then did `%s/\w\zs  /t/g` in vim to repair
most of the damage, and then fixed the rest manually. It's probably funny if it
doesn't happen to you. Don't tell anyone else on the team ;-)

[Avi (use Gerrit), 2009-07-21 18:34:56]

LGTM