Chromium Code Reviews
Help | Chromium Project | Sign in
(16)

Issue 6661024: Use a specialized new tab page in TOUCH_UI builds (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
4 years, 4 months ago by Rick Byers (Out until Aug 4th)
Modified:
4 years, 1 month ago
CC:
chromium-reviews, pam+watch_chromium.org, wyck
Visibility:
Public.

Description

Use a specialized new tab page in TOUCH_UI builds This has no change on regular builds (except to rename the NTP resource to be something more generic). On TOUCH_UI builds, an alternate NTP front-end that is optimized for touch is embedded into chrome instead of the standard one. In particular, I use conditional resource processing to choose the source of the IDR_NEW_TAB_HTML resource (new_new_tab_page.html for regular builds, touch_ntp/newtab.html for TOUCH_UI builds). The same WebUI back-end supports the union of functionality needed by all front-ends. Eg. the addition of page-index tracking is used only by this touch-ntp, and there are many features that the touch-ntp doesn't use (yet anyway). The idea with this touch NTP is to focus (for now) on apps, and make it easy to arrange them into pages. You can swipe/drag to switch pages, and press and hold to lift an app and rearrange it. There is lots of further work to improve the touch NTP (including the addition of automated tests and various UI improvements). But it's far enough along now that we'd like to use it as the only NTP in TOUCH_UI builds. Note that only the files in the main 'touch_ntp' directory are embedded as a resource in chrome. The 'tools' sub-directory provides a script for type and syntax checking. The 'standalone' sub-directory is a hack that is used only if newtab.html is loaded directly as a regular web-page. It enables the app to be used (with fake data) directly for rapid prototyping and testing. BUG=None TEST=Existing NTP tests still pass in regular builds. Tests for Touch_UI builds are forthcoming. Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=78596

Patch Set 1 #

Total comments: 82

Patch Set 2 : Tweaks based on CR feedback from arv #

Total comments: 60

Patch Set 3 : Changes for CR feedback from Arv #

Total comments: 4

Patch Set 4 : Ensure strict mode is scoped, and fix the target of touch events #

Total comments: 2

Patch Set 5 : Fix some indentation issues and enable gjslist --strict mode to catch them automatically #

Unified diffs Side-by-side diffs Delta from patch set Stats (+3426 lines, -3 lines) Patch
M chrome/browser/browser_resources.grd View 1 chunk +9 lines, -2 lines 0 comments Download
A chrome/browser/resources/touch_ntp/eventtracker.js View 1 2 3 4 1 chunk +99 lines, -0 lines 0 comments Download
A chrome/browser/resources/touch_ntp/grabber.js View 1 2 3 4 1 chunk +383 lines, -0 lines 0 comments Download
A chrome/browser/resources/touch_ntp/newtab.css View 1 2 1 chunk +217 lines, -0 lines 0 comments Download
A chrome/browser/resources/touch_ntp/newtab.html View 1 2 1 chunk +61 lines, -0 lines 0 comments Download
A chrome/browser/resources/touch_ntp/newtab.js View 1 2 3 4 1 chunk +805 lines, -0 lines 0 comments Download
A chrome/browser/resources/touch_ntp/slider.js View 1 2 3 4 1 chunk +336 lines, -0 lines 0 comments Download
A chrome/browser/resources/touch_ntp/standalone/entaglement-icon.png View Binary file 0 comments Download
A chrome/browser/resources/touch_ntp/standalone/gmail-favicon.png View Binary file 0 comments Download
A chrome/browser/resources/touch_ntp/standalone/gmail-icon.png View Binary file 0 comments Download
A chrome/browser/resources/touch_ntp/standalone/googlebooks-favicon.png View Binary file 0 comments Download
A chrome/browser/resources/touch_ntp/standalone/googlebooks-icon.png View Binary file 0 comments Download
A chrome/browser/resources/touch_ntp/standalone/googlemaps-favicon.png View Binary file 0 comments Download
A chrome/browser/resources/touch_ntp/standalone/googlemaps-icon.png View Binary file 0 comments Download
A chrome/browser/resources/touch_ntp/standalone/googletalk-icon.png View Binary file 0 comments Download
A chrome/browser/resources/touch_ntp/standalone/nytimes-icon.png View Binary file 0 comments Download
A chrome/browser/resources/touch_ntp/standalone/poppit-favicon.png View Binary file 0 comments Download
A chrome/browser/resources/touch_ntp/standalone/poppit-icon.png View Binary file 0 comments Download
A chrome/browser/resources/touch_ntp/standalone/standalone_hack.js View 1 2 3 4 1 chunk +562 lines, -0 lines 0 comments Download
A chrome/browser/resources/touch_ntp/standalone/webstore_icon.png View Binary file 0 comments Download
A chrome/browser/resources/touch_ntp/standalone/youtube-icon.png View Binary file 0 comments Download
A chrome/browser/resources/touch_ntp/tools/check View 1 2 3 4 1 chunk +58 lines, -0 lines 0 comments Download
A chrome/browser/resources/touch_ntp/tools/externs.js View 1 2 1 chunk +40 lines, -0 lines 0 comments Download
A chrome/browser/resources/touch_ntp/touchhandler.js View 1 2 3 4 1 chunk +850 lines, -0 lines 0 comments Download
A chrome/browser/resources/touch_ntp/trash.png View 1 Binary file 0 comments Download
A chrome/browser/resources/touch_ntp/trash-open.png View 1 Binary file 0 comments Download
M chrome/browser/ui/webui/ntp_resource_cache.cc View 1 chunk +6 lines, -1 line 0 comments Download
Trybot results:
Commit: CQ not working?

Messages

Total messages: 14 (0 generated)
Rick Byers (Out until Aug 4th)
Rob/Chad: this is the combined upstreamed version of all the touch-NTP work you've already reviewed ...
4 years, 4 months ago (2011-03-10 15:53:53 UTC) #1
rjkroege
Trusting you that this is exactly like the code that I've already reviewed... LGTM.
4 years, 4 months ago (2011-03-10 18:25:24 UTC) #2
arv (Not doing code reviews)
This is pretty awesome. Clear and well documented code. I wish I got looped in ...
4 years, 4 months ago (2011-03-10 19:25:58 UTC) #3
Rick Byers (Out until Aug 4th)
Thanks for your detailed feedback Erik! Sorry I didn't think to loop you in earlier ...
4 years, 4 months ago (2011-03-11 02:44:33 UTC) #4
jstritar
The changes to the browser resources and ntp_resource_cache LGTM. Also, nice JavaScript code. I was ...
4 years, 4 months ago (2011-03-11 14:47:51 UTC) #5
Rick Byers (Out until Aug 4th)
Thanks Jon! I tried to write some of these classes with the idea of potentially ...
4 years, 4 months ago (2011-03-11 14:57:44 UTC) #6
arv (Not doing code reviews)
Once again. This is pretty solid. I played around with the demo page and I ...
4 years, 4 months ago (2011-03-11 20:08:46 UTC) #7
Rick Byers (Out until Aug 4th)
Thanks again for your feedback Erik. I think I've changed everything to account for it. ...
4 years, 4 months ago (2011-03-15 21:47:54 UTC) #8
arv (Not doing code reviews)
I think using strict mode is good but you need to wrap each file in ...
4 years, 4 months ago (2011-03-15 23:53:43 UTC) #9
Rick Byers (Out until Aug 4th)
Thanks again Erik. I've put everything inside anonymous functions so that 'use strict' will be ...
4 years, 4 months ago (2011-03-16 22:06:02 UTC) #10
arv (Not doing code reviews)
LGTM http://codereview.chromium.org/6661024/diff/20001/chrome/browser/resources/touch_ntp/eventtracker.js File chrome/browser/resources/touch_ntp/eventtracker.js (right): http://codereview.chromium.org/6661024/diff/20001/chrome/browser/resources/touch_ntp/eventtracker.js#newcode14 chrome/browser/resources/touch_ntp/eventtracker.js:14: 'use strict'; On 2011/03/16 22:06:02, rbyers wrote: > ...
4 years, 4 months ago (2011-03-16 22:55:05 UTC) #11
Rick Byers (Out until Aug 4th)
Thanks Erik. I've fixed that one indentation issue and enabled --strict mode for gjslint to ...
4 years, 4 months ago (2011-03-17 16:18:22 UTC) #12
commit-bot: I haz the power
Can't apply patch for file chrome/browser/resources/touch_ntp/standalone/webstore_icon.png. svn: 'chrome/browser/resources/touch_ntp/standalone' is not a working copy
4 years, 4 months ago (2011-03-17 19:03:39 UTC) #13
Rick Byers (Out until Aug 4th)
4 years, 4 months ago (2011-03-17 21:10:12 UTC) #14
FYI, The Commit queue and try server doesn't support adding new binary
files.  Sadrul has submitted this for me (r78596) - thanks Sadrul!

Rick

On Thu, Mar 17, 2011 at 3:03 PM, <commit-bot@chromium.org> wrote:

> Can't apply patch for file
> chrome/browser/resources/touch_ntp/standalone/webstore_icon.png.
> svn: 'chrome/browser/resources/touch_ntp/standalone' is not a working copy
>
>
>
> http://codereview.chromium.org/6661024/
>
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 5fa3ca5