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

Issue 2344133003: Add help link when Bluetooth adapter is off for WebBluetooth chooser (Closed)

Created:
4 years, 3 months ago by juncai
Modified:
4 years, 2 months ago
CC:
chromium-reviews, tfarina
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Add help link when Bluetooth adapter is off for WebBluetooth chooser This CL adds a help link on the WebBluetooth chooser when the Bluetooth adapter is off. This will help users get more information of how to turn on Bluetooth. I uploaded some screenshots on the issue page. BUG=644168 Committed: https://crrev.com/c1bb01aa41e009f6075c5a87e945496a16976194 Cr-Commit-Position: refs/heads/master@{#421952}

Patch Set 1 : added help link when Bluetooth adapter is off for WebBluetooth chooser #

Patch Set 2 : rebase #

Total comments: 6

Patch Set 3 : address comments #

Total comments: 14

Patch Set 4 : address more comments #

Patch Set 5 : updated the kBluetoothAdapterOffHelpURL #

Total comments: 13

Patch Set 6 : address more comments #

Total comments: 7

Patch Set 7 : address more comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+107 lines, -50 lines) Patch
M chrome/app/generated_resources.grd View 1 2 3 4 5 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/chooser_controller/chooser_controller.h View 1 2 3 4 5 1 chunk +3 lines, -0 lines 0 comments Download
M chrome/browser/chooser_controller/chooser_controller.cc View 1 2 3 4 5 6 2 chunks +5 lines, -0 lines 0 comments Download
M chrome/browser/chooser_controller/mock_chooser_controller.h View 1 2 3 4 5 6 2 chunks +1 line, -1 line 0 comments Download
M chrome/browser/chooser_controller/mock_chooser_controller.cc View 1 2 6 3 chunks +2 lines, -8 lines 0 comments Download
M chrome/browser/ui/bluetooth/bluetooth_chooser_controller.h View 2 chunks +1 line, -1 line 0 comments Download
M chrome/browser/ui/bluetooth/bluetooth_chooser_controller.cc View 5 chunks +10 lines, -8 lines 0 comments Download
M chrome/browser/ui/bluetooth/bluetooth_chooser_controller_unittest.cc View 1 2 3 4 5 1 chunk +0 lines, -2 lines 0 comments Download
M chrome/browser/ui/cocoa/extensions/chooser_dialog_cocoa_controller_unittest.mm View 1 2 6 1 chunk +3 lines, -2 lines 0 comments Download
M chrome/browser/ui/views/chooser_content_view.h View 1 2 3 4 5 6 3 chunks +13 lines, -4 lines 0 comments Download
M chrome/browser/ui/views/chooser_content_view.cc View 1 2 3 4 5 6 7 chunks +39 lines, -10 lines 0 comments Download
M chrome/browser/ui/views/chooser_content_view_unittest.cc View 1 2 7 chunks +18 lines, -12 lines 0 comments Download
M chrome/common/url_constants.h View 1 2 3 4 5 1 chunk +3 lines, -0 lines 0 comments Download
M chrome/common/url_constants.cc View 1 2 3 4 5 1 chunk +7 lines, -0 lines 0 comments Download

Dependent Patchsets:

Messages

Total messages: 48 (31 generated)
juncai
sky@chromium.org: Please review changes in //chrome/browser/ui/views/ jyasskin@chromium.org: Please review changes in //chrome/browser/chooser_controller/ //chrome/browser/ui/bluetooth/
4 years, 3 months ago (2016-09-17 00:05:19 UTC) #8
sky
https://codereview.chromium.org/2344133003/diff/20001/chrome/browser/ui/views/chooser_content_view.cc File chrome/browser/ui/views/chooser_content_view.cc (right): https://codereview.chromium.org/2344133003/diff/20001/chrome/browser/ui/views/chooser_content_view.cc#newcode69 chrome/browser/ui/views/chooser_content_view.cc:69: l10n_util::GetStringUTF16(IDS_BLUETOOTH_DEVICE_CHOOSER_ADAPTER_OFF)); TURN_ADAPTER_OFF/ https://codereview.chromium.org/2344133003/diff/20001/chrome/browser/ui/views/chooser_content_view.cc#newcode76 chrome/browser/ui/views/chooser_content_view.cc:76: adapter_off_help_->SetPosition( Why are you setting ...
4 years, 3 months ago (2016-09-18 16:06:27 UTC) #11
juncai
rsesek@chromium.org: Please review changes in //chrome/browser/ui/cocoa/extensions/chooser_dialog_cocoa_controller_unittest.mm https://codereview.chromium.org/2344133003/diff/20001/chrome/browser/ui/views/chooser_content_view.cc File chrome/browser/ui/views/chooser_content_view.cc (right): https://codereview.chromium.org/2344133003/diff/20001/chrome/browser/ui/views/chooser_content_view.cc#newcode69 chrome/browser/ui/views/chooser_content_view.cc:69: l10n_util::GetStringUTF16(IDS_BLUETOOTH_DEVICE_CHOOSER_ADAPTER_OFF)); On 2016/09/18 ...
4 years, 3 months ago (2016-09-19 19:55:24 UTC) #15
Robert Sesek
chooser_dialog_cocoa_controller_unittest.mm LGTM
4 years, 3 months ago (2016-09-19 20:09:24 UTC) #16
sky
https://codereview.chromium.org/2344133003/diff/40001/chrome/browser/ui/views/chooser_content_view.cc File chrome/browser/ui/views/chooser_content_view.cc (right): https://codereview.chromium.org/2344133003/diff/40001/chrome/browser/ui/views/chooser_content_view.cc#newcode38 chrome/browser/ui/views/chooser_content_view.cc:38: class ChooserContentViewLayout : public views::LayoutManager { ChooserContentViewLayoutManager. https://codereview.chromium.org/2344133003/diff/40001/chrome/browser/ui/views/chooser_content_view.cc#newcode55 chrome/browser/ui/views/chooser_content_view.cc:55: ...
4 years, 3 months ago (2016-09-19 20:12:51 UTC) #17
juncai
https://codereview.chromium.org/2344133003/diff/40001/chrome/browser/ui/views/chooser_content_view.cc File chrome/browser/ui/views/chooser_content_view.cc (right): https://codereview.chromium.org/2344133003/diff/40001/chrome/browser/ui/views/chooser_content_view.cc#newcode38 chrome/browser/ui/views/chooser_content_view.cc:38: class ChooserContentViewLayout : public views::LayoutManager { On 2016/09/19 20:12:51, ...
4 years, 3 months ago (2016-09-19 23:35:29 UTC) #24
sky
LGTM
4 years, 3 months ago (2016-09-19 23:50:02 UTC) #25
juncai
ping jyasskin@.
4 years, 2 months ago (2016-09-26 16:38:56 UTC) #28
Jeffrey Yasskin
https://codereview.chromium.org/2344133003/diff/80001/chrome/browser/chooser_controller/chooser_controller.h File chrome/browser/chooser_controller/chooser_controller.h (right): https://codereview.chromium.org/2344133003/diff/80001/chrome/browser/chooser_controller/chooser_controller.h#newcode100 chrome/browser/chooser_controller/chooser_controller.h:100: virtual void OpenAdapterOffHelpUrl() const; Put this next to the ...
4 years, 2 months ago (2016-09-26 23:25:08 UTC) #29
juncai
https://codereview.chromium.org/2344133003/diff/80001/chrome/browser/chooser_controller/chooser_controller.h File chrome/browser/chooser_controller/chooser_controller.h (right): https://codereview.chromium.org/2344133003/diff/80001/chrome/browser/chooser_controller/chooser_controller.h#newcode100 chrome/browser/chooser_controller/chooser_controller.h:100: virtual void OpenAdapterOffHelpUrl() const; On 2016/09/26 23:25:07, Jeffrey Yasskin ...
4 years, 2 months ago (2016-09-28 22:45:43 UTC) #32
Jeffrey Yasskin
sky, could you double-check ChooserContentView::Layout() to make sure it's right to avoid the LayoutManager? LGTM ...
4 years, 2 months ago (2016-09-28 23:12:24 UTC) #33
sky
https://codereview.chromium.org/2344133003/diff/100001/chrome/browser/ui/views/chooser_content_view.cc File chrome/browser/ui/views/chooser_content_view.cc (right): https://codereview.chromium.org/2344133003/diff/100001/chrome/browser/ui/views/chooser_content_view.cc#newcode96 chrome/browser/ui/views/chooser_content_view.cc:96: gfx::Point(kAdapterOffHelpLinkPadding, kAdapterOffHelpLinkPadding)); It's certainly ok to roll your own ...
4 years, 2 months ago (2016-09-29 03:32:32 UTC) #36
juncai
https://codereview.chromium.org/2344133003/diff/100001/chrome/browser/chooser_controller/chooser_controller.cc File chrome/browser/chooser_controller/chooser_controller.cc (right): https://codereview.chromium.org/2344133003/diff/100001/chrome/browser/chooser_controller/chooser_controller.cc#newcode70 chrome/browser/chooser_controller/chooser_controller.cc:70: void ChooserController::OpenAdapterOffHelpUrl() const {} On 2016/09/28 23:12:23, Jeffrey Yasskin ...
4 years, 2 months ago (2016-09-29 21:14:58 UTC) #39
sky
LGTM
4 years, 2 months ago (2016-09-29 22:12:05 UTC) #42
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/2344133003/120001
4 years, 2 months ago (2016-09-29 22:14:08 UTC) #45
commit-bot: I haz the power
Committed patchset #7 (id:120001)
4 years, 2 months ago (2016-09-29 22:20:39 UTC) #46
commit-bot: I haz the power
4 years, 2 months ago (2016-09-29 22:24:54 UTC) #48
Message was sent while issue was closed.
Patchset 7 (id:??) landed as
https://crrev.com/c1bb01aa41e009f6075c5a87e945496a16976194
Cr-Commit-Position: refs/heads/master@{#421952}

Powered by Google App Engine
This is Rietveld 408576698