Issue 9903008: Added first-run infographic screens for IT2Me and Me2Me.

Issue 9903008: Added first-run infographic screens for IT2Me and Me2Me. (Closed)

Created:
8 years, 9 months ago by Jamie

Modified:
8 years, 8 months ago

Reviewers:
simonmorris

CC:
chromium-reviews, jamiewalch+watch_chromium.org, dcaiafa+watch_chromium.org, simonmorris+watch_chromium.org, hclam+watch_chromium.org, wez+watch_chromium.org, amit, sanjeevr, garykac+watch_chromium.org, lambroslambrou+watch_chromium.org, alexeypa+watch_chromium.org, sergeyu+watch_chromium.org

Base URL:
svn://svn.chromium.org/chrome/trunk/src

Visibility:
Public.

More Reviews

Description

Added first-run infographic screens for IT2Me and Me2Me. BUG=120184 TEST=Manual Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=129643

Patch Set 1 #

Total comments: 5

Patch Set 2 : Renamed variable #

Patch Set 3 : Rebase #

Created: 8 years, 8 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+147 lines, -80 lines)			Patch
M	remoting/remoting.gyp	View	1 2	1 chunk	+2 lines, -0 lines	0 comments	Download
A	remoting/resources/infographic_my_computers.png	View		0 chunks	+-1 lines, --1 lines	0 comments	Download
A	remoting/resources/infographic_remote_assistance.png	View		0 chunks	+-1 lines, --1 lines	0 comments	Download
M	remoting/webapp/_locales/en/messages.json	View	1 2	2 chunks	+12 lines, -0 lines	0 comments	Download
M	remoting/webapp/event_handlers.js	View	1 2	1 chunk	+5 lines, -1 line	0 comments	Download
M	remoting/webapp/host_list.js	View		2 chunks	+0 lines, -16 lines	0 comments	Download
M	remoting/webapp/main.css	View	1 2	3 chunks	+14 lines, -6 lines	0 comments	Download
M	remoting/webapp/main.html	View	1 2	1 chunk	+84 lines, -59 lines	0 comments	Download
M	remoting/webapp/remoting.js	View	1 2	1 chunk	+3 lines, -0 lines	0 comments	Download
M	remoting/webapp/ui_mode.js	View	1	1 chunk	+29 lines, -0 lines	0 comments	Download

Messages

Total messages: 3 (0 generated)

Expand Messages | Collapse Messages

Jamie

ptal https://chromiumcodereview.appspot.com/9903008/diff/1/remoting/webapp/host_list.js File remoting/webapp/host_list.js (left): https://chromiumcodereview.appspot.com/9903008/diff/1/remoting/webapp/host_list.js#oldcode208 remoting/webapp/host_list.js:208: * Show or hide the host-list UI. This ...

8 years, 9 months ago (2012-03-29 01:38:12 UTC) #1

simonmorris

lgtm http://codereview.chromium.org/9903008/diff/1/remoting/webapp/ui_mode.js File remoting/webapp/ui_mode.js (right): http://codereview.chromium.org/9903008/diff/1/remoting/webapp/ui_mode.js#newcode124 remoting/webapp/ui_mode.js:124: var show = !!window.localStorage.getItem('it2me-visited'); It would be better ...

8 years, 8 months ago (2012-03-29 16:34:38 UTC) #2

Jamie

8 years, 8 months ago (2012-03-29 18:01:46 UTC) #3

FYI

https://chromiumcodereview.appspot.com/9903008/diff/1/remoting/webapp/ui_mode.js
File remoting/webapp/ui_mode.js (right):

https://chromiumcodereview.appspot.com/9903008/diff/1/remoting/webapp/ui_mode...
remoting/webapp/ui_mode.js:124: var show =
!!window.localStorage.getItem('it2me-visited');
On 2012/03/29 16:34:38, simonmorris wrote:
> I think 'visited' is a clearer name for this variable than 'show'.

Done.

https://chromiumcodereview.appspot.com/9903008/diff/1/remoting/webapp/ui_mode...
remoting/webapp/ui_mode.js:124: var show =
!!window.localStorage.getItem('it2me-visited');
On 2012/03/29 16:34:38, simonmorris wrote:
> It would be better to have constants for the names of localStorage items, to
> avoid problems with typos.
> 
> But it's probably better to have a separate CL that converts all uses of item
> names, and puts them in one place.

Yes, I didn't bother because all the uses are so close together, but a repo of
all local storage keys might be useful.

Expand Messages | Collapse Messages