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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 2760453002: Creating options page and storing preferences. (Closed)

Created:
3 years, 9 months ago by elichtenberg
Modified:
3 years, 9 months ago
Reviewers:
dmazzoni, David Tseng
CC:
chromium-reviews, alemate+watch_chromium.org, arv+watch_chromium.org, oshima+watch_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Creating options page and storing preferences. BUG=593885 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation Review-Url: https://codereview.chromium.org/2760453002 Cr-Commit-Position: refs/heads/master@{#459617} Committed: https://chromium.googlesource.com/chromium/src/+/50999421600c93f6d673885430423166e307f17a

Patch Set 1 : Not for review #

Patch Set 2 : Created options page, storing preferences to chrome.storage.sync. #

Patch Set 3 : Fixed closure compiler errors. #

Patch Set 4 : Fixing change made to compiled_resources2.gyp #

Total comments: 4

Patch Set 5 : Responded to comments #

Patch Set 6 : Refactored switch_access.js to pass unit tests. #

Patch Set 7 : Fixed closure compiler error #

Total comments: 12

Patch Set 8 : Responded to comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+429 lines, -131 lines) Patch
M chrome/browser/resources/chromeos/switch_access/BUILD.gn View 1 2 3 4 5 1 chunk +5 lines, -0 lines 0 comments Download
A chrome/browser/resources/chromeos/switch_access/background.js View 1 2 3 4 5 6 7 1 chunk +10 lines, -0 lines 0 comments Download
M chrome/browser/resources/chromeos/switch_access/compiled_resources2.gyp View 1 2 3 4 5 6 1 chunk +29 lines, -2 lines 0 comments Download
M chrome/browser/resources/chromeos/switch_access/manifest.json.jinja2 View 1 2 3 4 5 1 chunk +6 lines, -2 lines 0 comments Download
A chrome/browser/resources/chromeos/switch_access/options.css View 1 1 chunk +8 lines, -0 lines 0 comments Download
A chrome/browser/resources/chromeos/switch_access/options.html View 1 2 3 4 5 6 7 1 chunk +29 lines, -0 lines 0 comments Download
A chrome/browser/resources/chromeos/switch_access/options.js View 1 2 3 4 5 6 7 1 chunk +89 lines, -0 lines 0 comments Download
A chrome/browser/resources/chromeos/switch_access/prefs.js View 1 1 chunk +91 lines, -0 lines 0 comments Download
M chrome/browser/resources/chromeos/switch_access/switch_access.js View 1 2 3 4 5 6 7 6 chunks +158 lines, -123 lines 0 comments Download
M chrome/browser/resources/chromeos/switch_access/switch_access_unittest.gtestjs View 1 2 3 4 5 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/resources/chromeos/switch_access/test_support.js View 1 2 3 4 5 1 chunk +0 lines, -4 lines 0 comments Download
M chrome/browser/resources/chromeos/switch_access/testable_switch_access.js View 1 2 3 4 5 1 chunk +2 lines, -0 lines 0 comments Download
M chrome/test/BUILD.gn View 1 2 3 4 5 1 chunk +1 line, -0 lines 0 comments Download

Dependent Patchsets:

Issue 2777203006 Patch 1
Issue 2777203006 Patch 20001
Issue 2777203006 Patch 40001

Messages

Total messages: 37 (28 generated)
elichtenberg
3 years, 9 months ago (2017-03-22 20:09:21 UTC) #3
dmazzoni
lgtm, but let David review too https://codereview.chromium.org/2760453002/diff/60001/chrome/browser/resources/chromeos/switch_access/options.js File chrome/browser/resources/chromeos/switch_access/options.js (right): https://codereview.chromium.org/2760453002/diff/60001/chrome/browser/resources/chromeos/switch_access/options.js#newcode31 chrome/browser/resources/chromeos/switch_access/options.js:31: document.addEventListener('DOMContentLoaded', this.init_.bind(this)); Does ...
3 years, 9 months ago (2017-03-22 21:53:54 UTC) #10
elichtenberg
https://codereview.chromium.org/2760453002/diff/60001/chrome/browser/resources/chromeos/switch_access/options.js File chrome/browser/resources/chromeos/switch_access/options.js (right): https://codereview.chromium.org/2760453002/diff/60001/chrome/browser/resources/chromeos/switch_access/options.js#newcode31 chrome/browser/resources/chromeos/switch_access/options.js:31: document.addEventListener('DOMContentLoaded', this.init_.bind(this)); On 2017/03/22 21:53:53, dmazzoni wrote: > Does ...
3 years, 9 months ago (2017-03-22 23:02:29 UTC) #12
elichtenberg
3 years, 9 months ago (2017-03-23 22:04:59 UTC) #22
elichtenberg
3 years, 9 months ago (2017-03-24 16:44:33 UTC) #27
David Tseng
lgtm https://codereview.chromium.org/2760453002/diff/120001/chrome/browser/resources/chromeos/switch_access/background.js File chrome/browser/resources/chromeos/switch_access/background.js (right): https://codereview.chromium.org/2760453002/diff/120001/chrome/browser/resources/chromeos/switch_access/background.js#newcode5 chrome/browser/resources/chromeos/switch_access/background.js:5: window.switchAccess = new SwitchAccess(); nit: js doc? https://codereview.chromium.org/2760453002/diff/120001/chrome/browser/resources/chromeos/switch_access/options.html ...
3 years, 9 months ago (2017-03-24 18:53:11 UTC) #28
elichtenberg
https://codereview.chromium.org/2760453002/diff/120001/chrome/browser/resources/chromeos/switch_access/background.js File chrome/browser/resources/chromeos/switch_access/background.js (right): https://codereview.chromium.org/2760453002/diff/120001/chrome/browser/resources/chromeos/switch_access/background.js#newcode5 chrome/browser/resources/chromeos/switch_access/background.js:5: window.switchAccess = new SwitchAccess(); On 2017/03/24 18:53:11, David Tseng ...
3 years, 9 months ago (2017-03-25 00:02:20 UTC) #31
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2760453002/140001
3 years, 9 months ago (2017-03-25 00:03:39 UTC) #34
commit-bot: I haz the power
3 years, 9 months ago (2017-03-25 01:49:42 UTC) #37
Message was sent while issue was closed. 
Committed patchset #8 (id:140001) as
https://chromium.googlesource.com/chromium/src/+/50999421600c93f6d67388543042...

Powered by Google App Engine
This is Rietveld 408576698