Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(85)

Issue 14261017: Local NTP: Render two rows of Most Visited tiles. (Closed)

Created:
7 years, 5 months ago by jeremycho
Modified:
7 years, 5 months ago
Reviewers:
Dan Beam
CC:
chromium-reviews, arv+watch_chromium.org, melevin, dhollowa+watch_chromium.org, dougw+watch_chromium.org, gideonwald, sreeram, dominich, samarth+watch_chromium.org, kmadhusu+watch_chromium.org, David Black, Jered
Base URL:
https://git.chromium.org/chromium/src.git@master
Visibility:
Public.

Description

Local NTP: Render two rows of Most Visited tiles. BUG=196498 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=196429

Patch Set 1 #

Total comments: 4

Patch Set 2 : Respond to comments. #

Patch Set 3 : Small comment update. #

Patch Set 4 : Pixel tweak. #

Patch Set 5 : Rebase past 196184. #

Patch Set 6 : ...and merge! #

Unified diffs Side-by-side diffs Delta from patch set Stats (+56 lines, -117 lines) Patch
M chrome/browser/resources/local_ntp/local_ntp.css View 1 2 3 4 5 chunks +9 lines, -7 lines 0 comments Download
M chrome/browser/resources/local_ntp/local_ntp.js View 1 2 3 4 5 14 chunks +47 lines, -110 lines 0 comments Download

Messages

Total messages: 10 (0 generated)
jeremycho
Hi Dan - this is ready for review. We'd like to get this in for ...
7 years, 5 months ago (2013-04-25 00:03:16 UTC) #1
Dan Beam
generally lg, lots of red code == good https://codereview.chromium.org/14261017/diff/1/chrome/browser/resources/local_ntp/local_ntp.js File chrome/browser/resources/local_ntp/local_ntp.js (left): https://codereview.chromium.org/14261017/diff/1/chrome/browser/resources/local_ntp/local_ntp.js#oldcode280 chrome/browser/resources/local_ntp/local_ntp.js:280: } ...
7 years, 5 months ago (2013-04-25 00:09:49 UTC) #2
jeremycho
Thanks for the quick reply! https://codereview.chromium.org/14261017/diff/1/chrome/browser/resources/local_ntp/local_ntp.js File chrome/browser/resources/local_ntp/local_ntp.js (left): https://codereview.chromium.org/14261017/diff/1/chrome/browser/resources/local_ntp/local_ntp.js#oldcode280 chrome/browser/resources/local_ntp/local_ntp.js:280: } else if (isUndoing) ...
7 years, 5 months ago (2013-04-25 00:15:03 UTC) #3
Dan Beam
lgtm
7 years, 5 months ago (2013-04-25 00:33:18 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jeremycho@chromium.org/14261017/11001
7 years, 5 months ago (2013-04-25 01:02:58 UTC) #5
commit-bot: I haz the power
Failed to apply patch for chrome/browser/resources/local_ntp/local_ntp.js: While running patch -p1 --forward --force --no-backup-if-mismatch; patching file ...
7 years, 5 months ago (2013-04-25 01:03:00 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jeremycho@chromium.org/14261017/18001
7 years, 5 months ago (2013-04-25 01:35:23 UTC) #7
commit-bot: I haz the power
Retried try job too often on win7_aura for step(s) interactive_ui_tests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win7_aura&number=33136
7 years, 5 months ago (2013-04-25 04:13:21 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jeremycho@chromium.org/14261017/18001
7 years, 5 months ago (2013-04-25 16:16:47 UTC) #9
commit-bot: I haz the power
7 years, 5 months ago (2013-04-25 17:48:38 UTC) #10
Message was sent while issue was closed.
Change committed as 196429

Powered by Google App Engine
This is Rietveld 408576698