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

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
3 years, 1 month ago by Rick Byers
Modified:
2 years, 10 months ago
CC:
chromium-reviews_chromium.org, 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) Lint Patch
M chrome/browser/browser_resources.grd View 1 chunk +9 lines, -2 lines 0 comments ? errors Download
A chrome/browser/resources/touch_ntp/eventtracker.js View 1 2 3 4 1 chunk +99 lines, -0 lines 0 comments ? errors Download
A chrome/browser/resources/touch_ntp/grabber.js View 1 2 3 4 1 chunk +383 lines, -0 lines 0 comments ? errors Download
A chrome/browser/resources/touch_ntp/newtab.css View 1 2 1 chunk +217 lines, -0 lines 0 comments ? errors Download
A chrome/browser/resources/touch_ntp/newtab.html View 1 2 1 chunk +61 lines, -0 lines 0 comments ? errors Download
A chrome/browser/resources/touch_ntp/newtab.js View 1 2 3 4 1 chunk +805 lines, -0 lines 0 comments ? errors Download
A chrome/browser/resources/touch_ntp/slider.js View 1 2 3 4 1 chunk +336 lines, -0 lines 0 comments ? errors Download
A chrome/browser/resources/touch_ntp/standalone/entaglement-icon.png View Binary file 0 comments ? errors Download
A chrome/browser/resources/touch_ntp/standalone/gmail-favicon.png View Binary file 0 comments ? errors Download
A chrome/browser/resources/touch_ntp/standalone/gmail-icon.png View Binary file 0 comments ? errors Download
A chrome/browser/resources/touch_ntp/standalone/googlebooks-favicon.png View Binary file 0 comments ? errors Download
A chrome/browser/resources/touch_ntp/standalone/googlebooks-icon.png View Binary file 0 comments ? errors Download
A chrome/browser/resources/touch_ntp/standalone/googlemaps-favicon.png View Binary file 0 comments ? errors Download
A chrome/browser/resources/touch_ntp/standalone/googlemaps-icon.png View Binary file 0 comments ? errors Download
A chrome/browser/resources/touch_ntp/standalone/googletalk-icon.png View Binary file 0 comments ? errors Download
A chrome/browser/resources/touch_ntp/standalone/nytimes-icon.png View Binary file 0 comments ? errors Download
A chrome/browser/resources/touch_ntp/standalone/poppit-favicon.png View Binary file 0 comments ? errors Download
A chrome/browser/resources/touch_ntp/standalone/poppit-icon.png View Binary file 0 comments ? errors Download
A chrome/browser/resources/touch_ntp/standalone/standalone_hack.js View 1 2 3 4 1 chunk +562 lines, -0 lines 0 comments ? errors Download
A chrome/browser/resources/touch_ntp/standalone/webstore_icon.png View Binary file 0 comments ? errors Download
A chrome/browser/resources/touch_ntp/standalone/youtube-icon.png View Binary file 0 comments ? errors Download
A chrome/browser/resources/touch_ntp/tools/check View 1 2 3 4 1 chunk +58 lines, -0 lines 0 comments ? errors Download
A chrome/browser/resources/touch_ntp/tools/externs.js View 1 2 1 chunk +40 lines, -0 lines 0 comments ? errors Download
A chrome/browser/resources/touch_ntp/touchhandler.js View 1 2 3 4 1 chunk +850 lines, -0 lines 0 comments ? errors Download
A chrome/browser/resources/touch_ntp/trash.png View 1 Binary file 0 comments ? errors Download
A chrome/browser/resources/touch_ntp/trash-open.png View 1 Binary file 0 comments ? errors Download
M chrome/browser/ui/webui/ntp_resource_cache.cc View 1 chunk +6 lines, -1 line 0 comments 0 errors Download
Trybot results:
Commit:

Messages

Total messages: 14
Rick Byers
Rob/Chad: this is the combined upstreamed version of all the touch-NTP work you've already reviewed ...
3 years, 1 month ago #1
rjkroege
Trusting you that this is exactly like the code that I've already reviewed... LGTM.
3 years, 1 month ago #2
arv
This is pretty awesome. Clear and well documented code. I wish I got looped in ...
3 years, 1 month ago #3
Rick Byers
Thanks for your detailed feedback Erik! Sorry I didn't think to loop you in earlier ...
3 years, 1 month ago #4
jstritar
The changes to the browser resources and ntp_resource_cache LGTM. Also, nice JavaScript code. I was ...
3 years, 1 month ago #5
Rick Byers
Thanks Jon! I tried to write some of these classes with the idea of potentially ...
3 years, 1 month ago #6
arv
Once again. This is pretty solid. I played around with the demo page and I ...
3 years, 1 month ago #7
Rick Byers
Thanks again for your feedback Erik. I think I've changed everything to account for it. ...
3 years, 1 month ago #8
arv
I think using strict mode is good but you need to wrap each file in ...
3 years, 1 month ago #9
Rick Byers
Thanks again Erik. I've put everything inside anonymous functions so that 'use strict' will be ...
3 years, 1 month ago #10
arv
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: > ...
3 years, 1 month ago #11
Rick Byers
Thanks Erik. I've fixed that one indentation issue and enabled --strict mode for gjslint to ...
3 years, 1 month ago #12
I haz the power (commit-bot)
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
3 years, 1 month ago #13
Rick Byers
3 years, 1 month ago #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 1280:2d3e6564b7b6