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

Issue 2755173002: [CrOS Tether] Add a connection dialog to md-settings. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 month, 1 week ago by Kyle Horimoto
Modified:
1 month, 1 week ago
Reviewers:
stevenjb, Dan Beam
CC:
chromium-reviews, michaelpg+watch-md-settings_chromium.org, dbeam+watch-closure_chromium.org, michaelpg+watch-md-ui_chromium.org, arv+watch_chromium.org, vitalyp+closure_chromium.org, dbeam+watch-settings_chromium.org, srahim+watch_chromium.org, stevenjb+watch-md-settings_chromium.org, jlklein+watch-closure_chromium.org, Ryan Hansberry
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

[CrOS Tether] Add a connection dialog to md-settings. The dialog is currently unused, and it will be hooked up to the rest of the page in a later CL. BUG=672263 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation Review-Url: https://codereview.chromium.org/2755173002 Cr-Commit-Position: refs/heads/master@{#458139} Committed: https://chromium.googlesource.com/chromium/src/+/86c3ba045bd9c7c3dfa92e010b91e2daff8e8ca6

Patch Set 1 #

Patch Set 2 : Add comments. #

Total comments: 10

Patch Set 3 : stevenjb@ comment. #

Total comments: 2

Patch Set 4 : stevenjb@ comment. #

Patch Set 5 : Fix closure compiler error. #

Total comments: 6

Patch Set 6 : dbeam@ comments. #

Total comments: 8

Patch Set 7 : dbeam@ comments. #

Total comments: 2

Patch Set 8 : dbeam@ comments. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+231 lines, -0 lines) Patch
M chrome/app/settings_strings.grdp View 1 chunk +30 lines, -0 lines 0 comments Download
M chrome/browser/resources/settings/icons.html View 1 2 3 4 5 6 1 chunk +5 lines, -0 lines 0 comments Download
M chrome/browser/resources/settings/internet_page/compiled_resources2.gyp View 1 2 3 4 1 chunk +9 lines, -0 lines 0 comments Download
A chrome/browser/resources/settings/internet_page/tether_connection_dialog.html View 1 2 3 4 5 6 7 1 chunk +80 lines, -0 lines 0 comments Download
A chrome/browser/resources/settings/internet_page/tether_connection_dialog.js View 1 2 3 4 5 6 1 chunk +81 lines, -0 lines 0 comments Download
M chrome/browser/resources/settings/settings_resources.grd View 1 chunk +6 lines, -0 lines 0 comments Download
M chrome/browser/ui/webui/settings/md_settings_localized_strings_provider.cc View 1 chunk +20 lines, -0 lines 0 comments Download
Trybot results:  linux_chromium_chromeos_rel_ng   linux_android_rel_ng   cast_shell_android   ios-simulator   linux_chromium_chromeos_rel_ng   closure_compilation   cast_shell_linux   android_compile_dbg   android_clang_dbg_recipe   linux_chromium_chromeos_ozone_rel_ng   mac_chromium_compile_dbg_ng   win_chromium_x64_rel_ng   win_chromium_compile_dbg_ng   linux_chromium_compile_dbg_ng   chromium_presubmit   ios-simulator-xcode-clang   ios-device-xcode-clang   chromeos_amd64-generic_chromium_compile_only_ng   android_n5x_swarming_rel   chromeos_daisy_chromium_compile_only_ng   win_chromium_rel_ng   linux_chromium_asan_rel_ng   mac_chromium_rel_ng   linux_chromium_rel_ng   android_cronet   ios-device   linux_chromium_tsan_rel_ng   win_clang   android_arm64_dbg_recipe   win_chromium_x64_rel_ng   linux_chromium_rel_ng   android_clang_dbg_recipe   win_clang   ios-device-xcode-clang   linux_chromium_asan_rel_ng   linux_android_rel_ng   ios-simulator-xcode-clang   linux_chromium_chromeos_rel_ng   closure_compilation   chromium_presubmit   chromeos_amd64-generic_chromium_compile_only_ng   android_n5x_swarming_rel   win_chromium_compile_dbg_ng   chromeos_daisy_chromium_compile_only_ng   cast_shell_linux   mac_chromium_rel_ng   linux_chromium_compile_dbg_ng   win_chromium_rel_ng   linux_chromium_chromeos_ozone_rel_ng   cast_shell_android   mac_chromium_compile_dbg_ng   android_cronet   android_compile_dbg   linux_chromium_tsan_rel_ng   win_chromium_x64_rel_ng   ios-simulator   ios-device   linux_chromium_rel_ng   android_arm64_dbg_recipe 
Commit queue not available (can’t edit this change).

Messages

Total messages: 34 (15 generated)
Kyle Horimoto
1 month, 1 week ago (2017-03-17 18:51:50 UTC) #3
stevenjb
https://codereview.chromium.org/2755173002/diff/20001/chrome/browser/resources/settings/internet_page/tether_connection_dialog.js File chrome/browser/resources/settings/internet_page/tether_connection_dialog.js (right): https://codereview.chromium.org/2755173002/diff/20001/chrome/browser/resources/settings/internet_page/tether_connection_dialog.js#newcode20 chrome/browser/resources/settings/internet_page/tether_connection_dialog.js:20: CrScrollableBehavior, You shouldn't need this behavior?
1 month, 1 week ago (2017-03-17 20:43:54 UTC) #4
Kyle Horimoto
https://codereview.chromium.org/2755173002/diff/20001/chrome/browser/resources/settings/internet_page/tether_connection_dialog.js File chrome/browser/resources/settings/internet_page/tether_connection_dialog.js (right): https://codereview.chromium.org/2755173002/diff/20001/chrome/browser/resources/settings/internet_page/tether_connection_dialog.js#newcode20 chrome/browser/resources/settings/internet_page/tether_connection_dialog.js:20: CrScrollableBehavior, On 2017/03/17 20:43:54, stevenjb wrote: > You shouldn't ...
1 month, 1 week ago (2017-03-17 20:48:38 UTC) #5
stevenjb
lgtm https://codereview.chromium.org/2755173002/diff/40001/chrome/browser/resources/settings/internet_page/tether_connection_dialog.js File chrome/browser/resources/settings/internet_page/tether_connection_dialog.js (right): https://codereview.chromium.org/2755173002/diff/40001/chrome/browser/resources/settings/internet_page/tether_connection_dialog.js#newcode19 chrome/browser/resources/settings/internet_page/tether_connection_dialog.js:19: I18nBehavior, Also, minor nit: if you remove the ...
1 month, 1 week ago (2017-03-17 20:50:44 UTC) #6
Kyle Horimoto
https://codereview.chromium.org/2755173002/diff/40001/chrome/browser/resources/settings/internet_page/tether_connection_dialog.js File chrome/browser/resources/settings/internet_page/tether_connection_dialog.js (right): https://codereview.chromium.org/2755173002/diff/40001/chrome/browser/resources/settings/internet_page/tether_connection_dialog.js#newcode19 chrome/browser/resources/settings/internet_page/tether_connection_dialog.js:19: I18nBehavior, On 2017/03/17 20:50:44, stevenjb wrote: > Also, minor ...
1 month, 1 week ago (2017-03-17 20:56:19 UTC) #7
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/2755173002/60001
1 month, 1 week ago (2017-03-17 20:57:03 UTC) #10
commit-bot: I haz the power
Try jobs failed on following builders: closure_compilation on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/closure_compilation/builds/7051)
1 month, 1 week ago (2017-03-17 21:10:21 UTC) #12
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/2755173002/80001
1 month, 1 week ago (2017-03-17 21:20:07 UTC) #15
Dan Beam
https://codereview.chromium.org/2755173002/diff/20001/chrome/browser/resources/settings/internet_page/tether_connection_dialog.html File chrome/browser/resources/settings/internet_page/tether_connection_dialog.html (right): https://codereview.chromium.org/2755173002/diff/20001/chrome/browser/resources/settings/internet_page/tether_connection_dialog.html#newcode22 chrome/browser/resources/settings/internet_page/tether_connection_dialog.html:22: --iron-icon-fill-color: var(--google-blue-500); indent https://codereview.chromium.org/2755173002/diff/20001/chrome/browser/resources/settings/internet_page/tether_connection_dialog.html#newcode59 chrome/browser/resources/settings/internet_page/tether_connection_dialog.html:59: tetherData_.tetherNostDeviceName)]] indent https://codereview.chromium.org/2755173002/diff/20001/chrome/browser/resources/settings/internet_page/tether_connection_dialog.html#newcode67 chrome/browser/resources/settings/internet_page/tether_connection_dialog.html:67: ...
1 month, 1 week ago (2017-03-17 21:50:05 UTC) #17
Dan Beam
https://codereview.chromium.org/2755173002/diff/80001/chrome/browser/resources/settings/internet_page/tether_connection_dialog.html File chrome/browser/resources/settings/internet_page/tether_connection_dialog.html (right): https://codereview.chromium.org/2755173002/diff/80001/chrome/browser/resources/settings/internet_page/tether_connection_dialog.html#newcode14 chrome/browser/resources/settings/internet_page/tether_connection_dialog.html:14: @apply(--layout-vertical); why are you using this rather than just ...
1 month, 1 week ago (2017-03-17 21:51:39 UTC) #19
Kyle Horimoto
https://codereview.chromium.org/2755173002/diff/20001/chrome/browser/resources/settings/internet_page/tether_connection_dialog.html File chrome/browser/resources/settings/internet_page/tether_connection_dialog.html (right): https://codereview.chromium.org/2755173002/diff/20001/chrome/browser/resources/settings/internet_page/tether_connection_dialog.html#newcode22 chrome/browser/resources/settings/internet_page/tether_connection_dialog.html:22: --iron-icon-fill-color: var(--google-blue-500); On 2017/03/17 21:50:04, Dan Beam wrote: > ...
1 month, 1 week ago (2017-03-17 22:06:39 UTC) #20
Dan Beam
https://codereview.chromium.org/2755173002/diff/100001/chrome/browser/resources/settings/internet_page/tether_connection_dialog.html File chrome/browser/resources/settings/internet_page/tether_connection_dialog.html (right): https://codereview.chromium.org/2755173002/diff/100001/chrome/browser/resources/settings/internet_page/tether_connection_dialog.html#newcode2 chrome/browser/resources/settings/internet_page/tether_connection_dialog.html:2: <link rel="import" href="chrome://resources/html/cr.html"> not needed https://codereview.chromium.org/2755173002/diff/100001/chrome/browser/resources/settings/internet_page/tether_connection_dialog.html#newcode4 chrome/browser/resources/settings/internet_page/tether_connection_dialog.html:4: <link rel="import" ...
1 month, 1 week ago (2017-03-17 22:21:31 UTC) #21
Kyle Horimoto
https://codereview.chromium.org/2755173002/diff/100001/chrome/browser/resources/settings/internet_page/tether_connection_dialog.html File chrome/browser/resources/settings/internet_page/tether_connection_dialog.html (right): https://codereview.chromium.org/2755173002/diff/100001/chrome/browser/resources/settings/internet_page/tether_connection_dialog.html#newcode2 chrome/browser/resources/settings/internet_page/tether_connection_dialog.html:2: <link rel="import" href="chrome://resources/html/cr.html"> On 2017/03/17 22:21:30, Dan Beam wrote: ...
1 month, 1 week ago (2017-03-17 22:48:39 UTC) #22
Dan Beam
lgtm https://codereview.chromium.org/2755173002/diff/120001/chrome/browser/resources/settings/internet_page/tether_connection_dialog.html File chrome/browser/resources/settings/internet_page/tether_connection_dialog.html (right): https://codereview.chromium.org/2755173002/diff/120001/chrome/browser/resources/settings/internet_page/tether_connection_dialog.html#newcode66 chrome/browser/resources/settings/internet_page/tether_connection_dialog.html:66: </template> </template> -> </li>
1 month, 1 week ago (2017-03-17 22:55:17 UTC) #23
Kyle Horimoto
https://codereview.chromium.org/2755173002/diff/120001/chrome/browser/resources/settings/internet_page/tether_connection_dialog.html File chrome/browser/resources/settings/internet_page/tether_connection_dialog.html (right): https://codereview.chromium.org/2755173002/diff/120001/chrome/browser/resources/settings/internet_page/tether_connection_dialog.html#newcode66 chrome/browser/resources/settings/internet_page/tether_connection_dialog.html:66: </template> On 2017/03/17 22:55:16, Dan Beam wrote: > </template> ...
1 month, 1 week ago (2017-03-17 23:13:48 UTC) #26
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/2755173002/140001
1 month, 1 week ago (2017-03-17 23:14:21 UTC) #27
commit-bot: I haz the power
Try jobs failed on following builders: win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_ng/builds/403367)
1 month, 1 week ago (2017-03-18 01:13:41 UTC) #29
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/2755173002/140001
1 month, 1 week ago (2017-03-20 17:02:20 UTC) #31
commit-bot: I haz the power
1 month, 1 week ago (2017-03-20 18:59:27 UTC) #34
Message was sent while issue was closed.
Committed patchset #8 (id:140001) as
https://chromium.googlesource.com/chromium/src/+/86c3ba045bd9c7c3dfa92e010b91...
Sign in to reply to this message.

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