Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1625633004:
bluetooth: Tell chooser discovery ended (Closed)
Descriptionbluetooth: Tell chooser the discovery ended
Two cases:
1. Device(s) discovered already. No user visible changes.
2. No device(s) discoverd. Show "No bluetooth devices found. Search again"
text. https://folio.googleplex.com/chrome-ux/mocks/236-fizz/choosers/032515_ChooserPhone#%2F03_NoneAfterTimeOut.jpg
BUG=539607
Committed: https://crrev.com/a1c019470be566be49967683947198fc14fed84c
Cr-Commit-Position: refs/heads/master@{#371877}
Patch Set 1 #
Total comments: 3
Patch Set 2 : Improve comment #Messages
Total messages: 18 (6 generated)
ortuno@chromium.org changed reviewers: + jyasskin@chromium.org
PTAL. Not sure if I should wait for your testing patch or not, since it's such a small change.
ping?
With the comment change, LGTM. Sorry for the delay. https://codereview.chromium.org/1625633004/diff/1/chrome/android/java/src/org... File chrome/android/java/src/org/chromium/chrome/browser/BluetoothChooserDialog.java (right): https://codereview.chromium.org/1625633004/diff/1/chrome/android/java/src/org... chrome/android/java/src/org/chromium/chrome/browser/BluetoothChooserDialog.java:322: // Tell the chooser discovery ended. Could you mention that sending an empty list only does anything if nothing was found? We'll need to add another signal to ItemChooserDialog to show that discovery finished after some devices were found.
https://codereview.chromium.org/1625633004/diff/1/chrome/android/java/src/org... File chrome/android/java/src/org/chromium/chrome/browser/BluetoothChooserDialog.java (right): https://codereview.chromium.org/1625633004/diff/1/chrome/android/java/src/org... chrome/android/java/src/org/chromium/chrome/browser/BluetoothChooserDialog.java:322: // Tell the chooser discovery ended. On 2016/01/27 at 18:22:37, Jeffrey Yasskin wrote: > Could you mention that sending an empty list only does anything if nothing was found? Done. WDYT? > We'll need to add another signal to ItemChooserDialog to show that discovery finished after some devices were found. Yeah, sadly the only way to currently communicate with the ItemChooserDialog is through showList(), so maybe we can use the empty list as an indication that the scan has stopped?
https://codereview.chromium.org/1625633004/diff/1/chrome/android/java/src/org... File chrome/android/java/src/org/chromium/chrome/browser/BluetoothChooserDialog.java (right): https://codereview.chromium.org/1625633004/diff/1/chrome/android/java/src/org... chrome/android/java/src/org/chromium/chrome/browser/BluetoothChooserDialog.java:322: // Tell the chooser discovery ended. On 2016/01/27 18:36:06, ortuno wrote: > On 2016/01/27 at 18:22:37, Jeffrey Yasskin wrote: > > Could you mention that sending an empty list only does anything if nothing was > found? > > Done. WDYT? LGTM > > We'll need to add another signal to ItemChooserDialog to show that discovery > finished after some devices were found. > > Yeah, sadly the only way to currently communicate with the ItemChooserDialog is > through showList(), so maybe we can use the empty list as an indication that the > scan has stopped? We own the ItemChooserDialog too, so feel free to change that. We should probably cc Reilly/Jun to make sure our changes there will keep working for them, but we shouldn't shy away from the changes. :)
The CQ bit was checked by ortuno@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1625633004/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1625633004/20001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: chromium_presubmit on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
ortuno@chromium.org changed reviewers: + newt@chromium.org
newt: PTAL
lgtm
The CQ bit was checked by ortuno@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1625633004/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1625633004/20001
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== bluetooth: Tell chooser the discovery ended Two cases: 1. Device(s) discovered already. No user visible changes. 2. No device(s) discoverd. Show "No bluetooth devices found. Search again" text. https://folio.googleplex.com/chrome-ux/mocks/236-fizz/choosers/032515_Chooser... BUG=539607 ========== to ========== bluetooth: Tell chooser the discovery ended Two cases: 1. Device(s) discovered already. No user visible changes. 2. No device(s) discoverd. Show "No bluetooth devices found. Search again" text. https://folio.googleplex.com/chrome-ux/mocks/236-fizz/choosers/032515_Chooser... BUG=539607 Committed: https://crrev.com/a1c019470be566be49967683947198fc14fed84c Cr-Commit-Position: refs/heads/master@{#371877} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/a1c019470be566be49967683947198fc14fed84c Cr-Commit-Position: refs/heads/master@{#371877} |
