Issue metadata
Sign in to add a comment
|
Harmony - update bluetooth (and WebUSB) device chooser |
||||||||||||||||||||||||||||||||||||||||||||||||||||||
Issue descriptionMocks attached. Source Bluetooth: https://folio.googleplex.com/chrome-ux-specs-and-sources/Chrome%20browser%20(MD)/Secondary%20UI%20Previews%20and%20specs%20(exports)/Preview/11-Permissions/Bluetooth#%3Fz=width USB: https://folio.googleplex.com/chrome-ux-specs-and-sources/Chrome%20browser%20(MD)/Secondary%20UI%20Previews%20and%20specs%20(exports)/Preview/11-Permissions/USB#%3Fz=width (attachments: 20170816)
,
Nov 3 2016
patricialor@ - if it's the same view then these steps are OK. Attaching a screenshot of the current implementation. bettes@ - is it the case that this mock is presenting a list of items? If so, that wasn't at all clear to me. The current implementation includes a table that can list one or more bluetooth devices. If this new design is also intended to present a list of devices I think it needs to be revised, because it includes none of the cues that tell you you're looking at a table of items.
,
Feb 10 2017
I'm not sure this dialog exists? But Issue 610430 is the work item for the dialog in #c2, and it is being actively worked on.
,
Feb 10 2017
,
Feb 10 2017
tapted@ - It does exist, I think it uses the same dialog as the one screenshotted in #c2. So any revamps done there will also affect the bluetooth device chooser one. Looping in ortuno@ in case I am wrong about this.
,
Feb 10 2017
Ah yeah - I'm on top of it now :) There's a "ChooserBubbleUiViewDelegate" and a "ChooserDialogView". "ChooserDialogView" is http://crbug.com/610430 . This one is "ChooserBubbleUiViewDelegate".
,
Feb 10 2017
Awesome, thanks for checking that :O (-ortuno@, sorry for the spam)
,
Feb 12 2017
just adding a note that there is also https://googlechrome.github.io/samples/web-bluetooth/battery-level.html for showing this dialog (and that is for bluetooth, rather than WebUSB, so there may be different strings.
,
Jun 16 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/04d6c626efe0af31662061779ca60cc651062374 commit 04d6c626efe0af31662061779ca60cc651062374 Author: bsep <bsep@chromium.org> Date: Fri Jun 16 22:11:18 2017 Refactor ChooserDialogView to use borders instead of content margins. To simplify DialogDelegate::CreateDialogFrameView, this patch removes the ability of callers to specify content margins. The only callsite that was using special margins, ChooserDialogView, was refactored to use borders like other DialogClientView subclasses. This changes its layout slightly. Also added a BrowserDialogTest for ChooserDialogView. BUG= 654137 Review-Url: https://codereview.chromium.org/2932243002 Cr-Commit-Position: refs/heads/master@{#480194} [modify] https://crrev.com/04d6c626efe0af31662061779ca60cc651062374/chrome/browser/chooser_controller/chooser_controller.h [modify] https://crrev.com/04d6c626efe0af31662061779ca60cc651062374/chrome/browser/chooser_controller/mock_chooser_controller.cc [modify] https://crrev.com/04d6c626efe0af31662061779ca60cc651062374/chrome/browser/ui/views/constrained_web_dialog_delegate_views.cc [modify] https://crrev.com/04d6c626efe0af31662061779ca60cc651062374/chrome/browser/ui/views/extensions/chooser_dialog_view.cc [modify] https://crrev.com/04d6c626efe0af31662061779ca60cc651062374/chrome/browser/ui/views/extensions/chooser_dialog_view.h [add] https://crrev.com/04d6c626efe0af31662061779ca60cc651062374/chrome/browser/ui/views/extensions/chooser_dialog_view_browsertest.cc [modify] https://crrev.com/04d6c626efe0af31662061779ca60cc651062374/chrome/test/BUILD.gn [modify] https://crrev.com/04d6c626efe0af31662061779ca60cc651062374/ui/views/window/dialog_delegate.cc [modify] https://crrev.com/04d6c626efe0af31662061779ca60cc651062374/ui/views/window/dialog_delegate.h
,
Aug 9 2017
,
Aug 9 2017
The mock link at the top is dead, and it sounds like there were some questions about it anyway. This needs an updated mock.
,
Aug 9 2017
,
Aug 9 2017
WebBluetooth and WebUSB are now both straightforward to summon at https://permission.site/ they currently use a similar dialog setup, but there's a separate mock for webUSB which is quite different. (The WebBluetooth mock I could find is in the description now).
,
Aug 14 2017
I'm assuming this bug is tracking Device Chooser UI for both Windows and Mac? Please mark the bug as such or let me know if that's not the case. ------------------------ Feedback for Device Chooser - USB _All - Replace ‘Get help’ footer with (?) icon - Fixed height of 258px - Make table rows 24px in height. Not 22. We're inconsistent. _Win - Selected rows should be in grey (#000 0.08a), not blue - Missing the grey anchored UI ------------------------ Feedback for Device Chooser - Bluetooth _All - Replace ‘Get help’ footer with (?) icon - Fixed height of 258px - Make table rows 24px in height. Not 22. We're inconsistent. - Can we move the scanning affordance to the table? _Win (not able to test) Is there a reason permissions.test doesn't work on my windows machine? ------------------------ All notes can be tracked in the implementation review deck: https://docs.google.com/presentation/d/1efIBdWozcOXv1hEaIxdcdf4A1-8P8HQu7bRpEokLisI/edit?userstoinvite=crivero@google.com&ts=5991f0cf#slide=id.g1d151705ab_0_19
,
Aug 15 2017
USB mocks and specs: https://folio.googleplex.com/chrome-ux-specs-and-sources/Chrome%20browser%20(MD)/Secondary%20UI%20Previews%20and%20specs%20(exports)/Preview/11-Permissions/USB#%3Fz=width Bluetooth mocks and specs: https://folio.googleplex.com/chrome-ux-specs-and-sources/Chrome%20browser%20(MD)/Secondary%20UI%20Previews%20and%20specs%20(exports)/Preview/11-Permissions/Bluetooth#%3Fz=width
,
Aug 16 2017
,
Aug 16 2017
> Win (not able to test) > Is there a reason permissions.test doesn't work on my windows machine? You might need a bluetooth adapter to test in Canary.. chrome://usb-internals/ has a way to add test devices, but I don't see the same for chrome://bluetooth-internals/
,
Aug 16 2017
Besides the adapter, you also need to: 1. turn on chrome://flags/#enable-experimental-web-platform-features on Windows. 2. Pair with some bluetooth devices from the Windows menu before opening the dialog.
,
Aug 16 2017
Note that the (?) icon may not be enough to convey two messages that can be shown: "Get help while scanning for devices..." "Get help or re-scan" (shown after scanning ends)
,
Aug 24 2017
Load balancing to bsep
,
Aug 29 2017
For rescanning, we should surface a third button to the right of the (?) https://folio.googleplex.com/chrome-ux-specs-and-sources/Chrome%20browser%20(MD)/Secondary%20UI%20Previews%20and%20specs%20(exports)/Preview/11-Permissions/Bluetooth#%2FP%20-%20Bluetooth%20-%2003.png%3Fz=width
,
Aug 29 2017
Apologies, "Rescan" should have a hyphen: "Re-scan". Also note the scanning row in the table is removed once scanning is done. And then re-introduced (pushing the rows beneath it down) when re-scanned
,
Sep 5 2017
,
Sep 29 2017
The NextAction date has arrived: 2017-09-29
,
Oct 6 2017
,
Oct 9 2017
Issue 610430 has been merged into this issue.
,
Oct 20 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e6e84f7ee5e1dad96f8698bdcc6409bafd38471c commit e6e84f7ee5e1dad96f8698bdcc6409bafd38471c Author: Bret Sepulveda <bsep@chromium.org> Date: Fri Oct 20 03:37:40 2017 Refactor BrowserDialogTest for device chooser dialogs. The existing BrowserDialogTest only covered ChooserDialogView, not the related ChooserBubbleUi, and used fake data. This patch adds two new fake chooser controllers that look similar to the USB and Bluetooth cases, for a more accurate test, and expands the tests to cover both the modal and bubble versions of the UI. Bug: 654137 Change-Id: I40083466da3b38f525546a8dc695a34603c2f741 Reviewed-on: https://chromium-review.googlesource.com/699679 Reviewed-by: Reilly Grant <reillyg@chromium.org> Reviewed-by: Jun Cai <juncai@chromium.org> Reviewed-by: Trent Apted <tapted@chromium.org> Commit-Queue: Bret Sepulveda <bsep@chromium.org> Cr-Commit-Position: refs/heads/master@{#510320} [modify] https://crrev.com/e6e84f7ee5e1dad96f8698bdcc6409bafd38471c/chrome/browser/BUILD.gn [modify] https://crrev.com/e6e84f7ee5e1dad96f8698bdcc6409bafd38471c/chrome/browser/chooser_controller/chooser_controller.cc [modify] https://crrev.com/e6e84f7ee5e1dad96f8698bdcc6409bafd38471c/chrome/browser/chooser_controller/chooser_controller.h [add] https://crrev.com/e6e84f7ee5e1dad96f8698bdcc6409bafd38471c/chrome/browser/chooser_controller/fake_bluetooth_chooser_controller.cc [add] https://crrev.com/e6e84f7ee5e1dad96f8698bdcc6409bafd38471c/chrome/browser/chooser_controller/fake_bluetooth_chooser_controller.h [add] https://crrev.com/e6e84f7ee5e1dad96f8698bdcc6409bafd38471c/chrome/browser/chooser_controller/fake_usb_chooser_controller.cc [add] https://crrev.com/e6e84f7ee5e1dad96f8698bdcc6409bafd38471c/chrome/browser/chooser_controller/fake_usb_chooser_controller.h [modify] https://crrev.com/e6e84f7ee5e1dad96f8698bdcc6409bafd38471c/chrome/browser/chooser_controller/mock_chooser_controller.h [modify] https://crrev.com/e6e84f7ee5e1dad96f8698bdcc6409bafd38471c/chrome/browser/extensions/device_permissions_dialog_controller.cc [modify] https://crrev.com/e6e84f7ee5e1dad96f8698bdcc6409bafd38471c/chrome/browser/extensions/device_permissions_dialog_controller.h [add] https://crrev.com/e6e84f7ee5e1dad96f8698bdcc6409bafd38471c/chrome/browser/ui/views/device_chooser_browsertest.cc [delete] https://crrev.com/e89888c7a55c44c674fcd90f9db0933712cfa25b/chrome/browser/ui/views/extensions/chooser_dialog_view_browsertest.cc [modify] https://crrev.com/e6e84f7ee5e1dad96f8698bdcc6409bafd38471c/chrome/browser/ui/views/permission_bubble/chooser_bubble_ui.cc [modify] https://crrev.com/e6e84f7ee5e1dad96f8698bdcc6409bafd38471c/chrome/browser/usb/usb_chooser_controller.cc [modify] https://crrev.com/e6e84f7ee5e1dad96f8698bdcc6409bafd38471c/chrome/browser/usb/usb_chooser_controller.h [modify] https://crrev.com/e6e84f7ee5e1dad96f8698bdcc6409bafd38471c/chrome/test/BUILD.gn
,
Nov 10 2017
,
Nov 11 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3d1f0f3a27fe7170f9aa8144c5bd91989f80675a commit 3d1f0f3a27fe7170f9aa8144c5bd91989f80675a Author: Bret Sepulveda <bsep@chromium.org> Date: Sat Nov 11 03:18:03 2017 Refactor DeviceChooserContentViewTest and ChooserDialogViewTest. Reduced redundant test expectations and generally made the tests more terse in preparation for the chooser dialogs' redesign. Bug: 654137 Change-Id: Ifda8645378103d498bb869f8ab5589f6cb66e5db Reviewed-on: https://chromium-review.googlesource.com/731472 Reviewed-by: Jun Cai <juncai@chromium.org> Reviewed-by: Scott Violet <sky@chromium.org> Commit-Queue: Bret Sepulveda <bsep@chromium.org> Cr-Commit-Position: refs/heads/master@{#515814} [modify] https://crrev.com/3d1f0f3a27fe7170f9aa8144c5bd91989f80675a/chrome/browser/BUILD.gn [modify] https://crrev.com/3d1f0f3a27fe7170f9aa8144c5bd91989f80675a/chrome/browser/chooser_controller/fake_bluetooth_chooser_controller.cc [modify] https://crrev.com/3d1f0f3a27fe7170f9aa8144c5bd91989f80675a/chrome/browser/chooser_controller/fake_bluetooth_chooser_controller.h [modify] https://crrev.com/3d1f0f3a27fe7170f9aa8144c5bd91989f80675a/chrome/browser/ui/views/device_chooser_browsertest.cc [modify] https://crrev.com/3d1f0f3a27fe7170f9aa8144c5bd91989f80675a/chrome/browser/ui/views/device_chooser_content_view.cc [modify] https://crrev.com/3d1f0f3a27fe7170f9aa8144c5bd91989f80675a/chrome/browser/ui/views/device_chooser_content_view.h [modify] https://crrev.com/3d1f0f3a27fe7170f9aa8144c5bd91989f80675a/chrome/browser/ui/views/device_chooser_content_view_unittest.cc [modify] https://crrev.com/3d1f0f3a27fe7170f9aa8144c5bd91989f80675a/chrome/browser/ui/views/extensions/chooser_dialog_view_unittest.cc [modify] https://crrev.com/3d1f0f3a27fe7170f9aa8144c5bd91989f80675a/ui/views/controls/table/table_view.cc [modify] https://crrev.com/3d1f0f3a27fe7170f9aa8144c5bd91989f80675a/ui/views/controls/table/table_view_unittest.cc
,
Dec 13 2017
Screenshots for https://chromium-review.googlesource.com/c/chromium/src/+/786781.
,
Dec 18 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a4975e7a5e9e22912c596db2bda6507df78de81f commit a4975e7a5e9e22912c596db2bda6507df78de81f Author: Bret Sepulveda <bsep@chromium.org> Date: Mon Dec 18 23:16:38 2017 Remove device chooser's footnote and add controls in the extra view. As per the Harmony spec, the "Get help" link is changed to a "?" button, and the "re-scan" link is changed to a button. These are moved out of the footnote area and into the bottom-left of the dialog. When scanning, the re-scan button is replaced with a "scanning" label and an adjacent throbber. The table shows nothing if it is scanning and no devices are yet found. Bug: 654137 Change-Id: Ib1f3188efc24873cf08286d442cb156f47af40f8 Reviewed-on: https://chromium-review.googlesource.com/786781 Commit-Queue: Bret Sepulveda <bsep@chromium.org> Reviewed-by: Jun Cai <juncai@chromium.org> Reviewed-by: Trent Apted <tapted@chromium.org> Reviewed-by: Devlin <rdevlin.cronin@chromium.org> Cr-Commit-Position: refs/heads/master@{#524851} [modify] https://crrev.com/a4975e7a5e9e22912c596db2bda6507df78de81f/chrome/app/generated_resources.grd [modify] https://crrev.com/a4975e7a5e9e22912c596db2bda6507df78de81f/chrome/browser/chooser_controller/chooser_controller.cc [modify] https://crrev.com/a4975e7a5e9e22912c596db2bda6507df78de81f/chrome/browser/chooser_controller/chooser_controller.h [modify] https://crrev.com/a4975e7a5e9e22912c596db2bda6507df78de81f/chrome/browser/chooser_controller/fake_bluetooth_chooser_controller.cc [modify] https://crrev.com/a4975e7a5e9e22912c596db2bda6507df78de81f/chrome/browser/chooser_controller/fake_bluetooth_chooser_controller.h [modify] https://crrev.com/a4975e7a5e9e22912c596db2bda6507df78de81f/chrome/browser/extensions/device_permissions_dialog_controller.cc [modify] https://crrev.com/a4975e7a5e9e22912c596db2bda6507df78de81f/chrome/browser/extensions/device_permissions_dialog_controller.h [modify] https://crrev.com/a4975e7a5e9e22912c596db2bda6507df78de81f/chrome/browser/ui/bluetooth/bluetooth_chooser_controller.cc [modify] https://crrev.com/a4975e7a5e9e22912c596db2bda6507df78de81f/chrome/browser/ui/bluetooth/bluetooth_chooser_controller.h [modify] https://crrev.com/a4975e7a5e9e22912c596db2bda6507df78de81f/chrome/browser/ui/cocoa/device_chooser_content_view_cocoa.mm [modify] https://crrev.com/a4975e7a5e9e22912c596db2bda6507df78de81f/chrome/browser/ui/views/device_chooser_content_view.cc [modify] https://crrev.com/a4975e7a5e9e22912c596db2bda6507df78de81f/chrome/browser/ui/views/device_chooser_content_view.h [modify] https://crrev.com/a4975e7a5e9e22912c596db2bda6507df78de81f/chrome/browser/ui/views/device_chooser_content_view_unittest.cc [modify] https://crrev.com/a4975e7a5e9e22912c596db2bda6507df78de81f/chrome/browser/ui/views/extensions/chooser_dialog_view.cc [modify] https://crrev.com/a4975e7a5e9e22912c596db2bda6507df78de81f/chrome/browser/ui/views/extensions/chooser_dialog_view.h [modify] https://crrev.com/a4975e7a5e9e22912c596db2bda6507df78de81f/chrome/browser/ui/views/extensions/chooser_dialog_view_unittest.cc [modify] https://crrev.com/a4975e7a5e9e22912c596db2bda6507df78de81f/chrome/browser/ui/views/permission_bubble/chooser_bubble_ui.cc
,
Dec 19 2017
This is ready for review. Use https://permission.site to test.
,
Jan 19 2018
Screenshots for https://chromium-review.googlesource.com/c/chromium/src/+/875162
,
Jan 22 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9e098bcbd1252eeda8747f02e3a288a979beeff9 commit 9e098bcbd1252eeda8747f02e3a288a979beeff9 Author: Bret Sepulveda <bsep@chromium.org> Date: Mon Jan 22 20:30:25 2018 Bluetooth chooser: error message tweaks. This patch addresses two pieces of feedback on the bluetooth chooser bubble and dialog: * Centers the adapter help message. * Removes the icon spacing for the message shown when there is no devices, since there is no icon in that case. See screenshots in the associated bug. Bug: 654137 Change-Id: I83eb6878e5ea90fd07293a79c65fe9e57a6bf6a0 Reviewed-on: https://chromium-review.googlesource.com/875162 Reviewed-by: Conley Owens <cco3@chromium.org> Reviewed-by: Jun Cai <juncai@chromium.org> Commit-Queue: Bret Sepulveda <bsep@chromium.org> Cr-Commit-Position: refs/heads/master@{#530975} [modify] https://crrev.com/9e098bcbd1252eeda8747f02e3a288a979beeff9/chrome/browser/chooser_controller/fake_bluetooth_chooser_controller.cc [modify] https://crrev.com/9e098bcbd1252eeda8747f02e3a288a979beeff9/chrome/browser/ui/bluetooth/bluetooth_chooser_controller.cc [modify] https://crrev.com/9e098bcbd1252eeda8747f02e3a288a979beeff9/chrome/browser/ui/views/device_chooser_content_view.cc [modify] https://crrev.com/9e098bcbd1252eeda8747f02e3a288a979beeff9/chrome/browser/ui/views/device_chooser_content_view.h [modify] https://crrev.com/9e098bcbd1252eeda8747f02e3a288a979beeff9/chrome/browser/ui/views/device_chooser_content_view_unittest.cc
,
Jan 22 2018
Addressed nits.
,
Feb 18 2018
,
Apr 9 2018
,
Jul 13
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/67277ecc30c2bff61858acf7d584087750ae0cfd commit 67277ecc30c2bff61858acf7d584087750ae0cfd Author: Reilly Grant <reillyg@chromium.org> Date: Fri Jul 13 23:05:13 2018 Center the "no devices found" message in the device chooser This is a follow-up to r564119 where it was suggested that by always returning true from ShouldShowIconBeforeText() the "no devices found" message would be offset to the right. To fix this, when there are no options to display this change hides the table view entirely and displays the message using the same style of overlayed label as the "adapter off" message. Bug: 654137 Change-Id: I7544698425e94fb3d1666ba9c70063091f85f438 Reviewed-on: https://chromium-review.googlesource.com/1117873 Commit-Queue: Reilly Grant <reillyg@chromium.org> Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org> Cr-Commit-Position: refs/heads/master@{#575109} [modify] https://crrev.com/67277ecc30c2bff61858acf7d584087750ae0cfd/chrome/browser/ui/views/device_chooser_content_view.cc [modify] https://crrev.com/67277ecc30c2bff61858acf7d584087750ae0cfd/chrome/browser/ui/views/device_chooser_content_view.h [modify] https://crrev.com/67277ecc30c2bff61858acf7d584087750ae0cfd/chrome/browser/ui/views/device_chooser_content_view_unittest.cc |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||
/Secondary%20UI%20Previews%20and%20specs%20(exports)/Preview#%2FP%20-%20bluetooth_02.png%3Fz=width){kind=link}
/Secondary%20UI%20Previews%20and%20specs%20(exports)/Preview/11-Permissions/Bluetooth#%2FP%20-%20Bluetooth%20-%2003.png%3Fz=width){kind=link}
Comment 1 by patricia...@chromium.org
, Nov 2 2016