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

Issue 3747007: Reserve right-most column of apps grid for web store icon. (Closed)

Created:
10 years, 2 months ago by Aaron Boodman
Modified:
9 years, 6 months ago
CC:
chromium-reviews, arv (Not doing code reviews), ben+cc_chromium.org
Base URL:
git://codf21.jail/chromium.git
Visibility:
Public.

Description

Reserve right-most column of apps grid for web store icon. When there is less than one complete row of apps, position the web store icon naturally in the grid, to the right of the other apps. Once there is at least one complete row, position the web store icon at the top of the right-most column. BUG=58857 TEST=Install a few apps. Web store icon should be in top row directly to the right of the other apps. Install more apps. Once you have more than one row, web store icon should stay in top right corner by itself. No other apps should go below it. Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=62962

Patch Set 1 #

Patch Set 2 : different approach that works for small view too #

Patch Set 3 : different approach #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+59 lines, -7 lines) Patch
M chrome/browser/resources/new_new_tab.html View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/resources/ntp/apps.css View 1 2 2 chunks +42 lines, -2 lines 0 comments Download
M chrome/browser/resources/ntp/apps.js View 1 2 3 chunks +16 lines, -5 lines 2 comments Download

Messages

Total messages: 5 (0 generated)
Aaron Boodman
10 years, 2 months ago (2010-10-15 17:15:25 UTC) #1
Aaron Boodman
Nevermind, on this, I'm going to have to do something more clever because of the ...
10 years, 2 months ago (2010-10-15 19:53:41 UTC) #2
arv1
Just ping me when you want me to look at it. On 12:53 pm, aa ...
10 years, 2 months ago (2010-10-15 20:53:11 UTC) #3
Aaron Boodman
Ready now.
10 years, 2 months ago (2010-10-15 21:47:46 UTC) #4
arv (Not doing code reviews)
10 years, 2 months ago (2010-10-15 22:33:36 UTC) #5
LGTM

http://codereview.chromium.org/3747007/diff/7001/8003
File chrome/browser/resources/ntp/apps.js (right):

http://codereview.chromium.org/3747007/diff/7001/8003#newcode15
chrome/browser/resources/ntp/apps.js:15: var webStoreEntry = null;
do you really need to assign null here?

http://codereview.chromium.org/3747007/diff/7001/8003#newcode18
chrome/browser/resources/ntp/apps.js:18: while (appsSectionContent.firstChild) {
Since you are now removing all of them you can just set textContent to ''

Powered by Google App Engine
This is Rietveld 408576698