Make WebBluetooth chooser consistent on Android and desktops (desktop part)

Make WebBluetooth chooser consistent on Android and desktops (desktop part)

This CL makes the following changes:
1. Remove "Not seeing your device?" string.
2. Remove the period at "No Bluetooth devices found.", "No devices found."
3. Show get help link and scanning status text below the chooser:
(a) When the Bluetooth is turned off, show "Get help" link text.
(b) When scanning, show "Get help while scanning for devices...",
the "Get help" is link text, "while scanning for devices..." is plain text.
(c) When scanning and found some device, or scanning is complete, show
"Get help or re-scan", both "Get help" and "re-scan" are link text.

I uploaded some screenshots from ChromeOS and Mac on the issue page.

BUG=659271
Committed: https://crrev.com/2e52a5a282f509f48e397626f5fcd30f5ab8b1fe
Cr-Commit-Position: refs/heads/master@{#429914}

[juncai, 2016-11-03 01:43:47]

The CQ bit was checked by juncai@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-03 01:44:06]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[juncai, 2016-11-03 03:01:57]

juncai@chromium.org changed reviewers:
+ rsesek@chromium.org, sky@chromium.org

[juncai, 2016-11-03 03:01:57]

rsesek@chromium.org: Please review changes in 
//chrome/browser/ui/cocoa

sky@chromium.org: Please review changes in 
//chrome/browser/ui/views
//ui/views/controls/styled_label.h

[juncai, 2016-11-03 03:02:21]

rsesek@chromium.org: Please review changes in 
//chrome/browser/ui/cocoa

sky@chromium.org: Please review changes in 
//chrome/browser/ui/views
//ui/views/controls/styled_label.h

[commit-bot: I haz the power, 2016-11-03 03:45:03]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-03 03:45:05]

Dry run: Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT,
no build URL)

[juncai, 2016-11-03 05:28:37]

Description was changed from

==========
Make WebBluetooth chooser consistent on Android and desktops (desktop part)

This CL makes the following change:
1. Remove "Not seeing your device?" string.
2. Remove the period at "No Bluetooth devices found.", "No devices found."
3. Show get help link and scanning status text below the chooser:
(a) When the Bluetooth is turned off, show "Get help" link text.
(b) When scanning, show "Get help while scanning for devices...",
the "Get help" is link text, "while scanning for devices..." is plain text.
(c) When scanning and found some device, or scanning is complete, show
"Get help or re-scan", both "Get help" and "re-scan" are link text.

I uploaded some screenshots from ChromeOS and Mac on the issue page.

BUG=659271
==========

to

==========
Make WebBluetooth chooser consistent on Android and desktops (desktop part)

This CL makes the following changes:
1. Remove "Not seeing your device?" string.
2. Remove the period at "No Bluetooth devices found.", "No devices found."
3. Show get help link and scanning status text below the chooser:
(a) When the Bluetooth is turned off, show "Get help" link text.
(b) When scanning, show "Get help while scanning for devices...",
the "Get help" is link text, "while scanning for devices..." is plain text.
(c) When scanning and found some device, or scanning is complete, show
"Get help or re-scan", both "Get help" and "re-scan" are link text.

I uploaded some screenshots from ChromeOS and Mac on the issue page.

BUG=659271
==========

[sky, 2016-11-03 13:24:23]

https://codereview.chromium.org/2468403002/diff/1/chrome/browser/ui/views/cho...
File chrome/browser/ui/views/chooser_content_view.cc (right):

https://codereview.chromium.org/2468403002/diff/1/chrome/browser/ui/views/cho...
chrome/browser/ui/views/chooser_content_view.cc:211: footnote_link_->Layout();
This only works if the sized needed for the new text is exactly same as the
previous size. Are you sure that's the case? I would expect a layout to occur on
the parent, e.g. footnote_link_->parent()->Layout(), and possibly
footnote_link->parent()->parent()-Layout() if the number of lines might change
(even that later might not be enough in that case). How do you know the text
only fits one line?

https://codereview.chromium.org/2468403002/diff/1/chrome/browser/ui/views/cho...
File chrome/browser/ui/views/chooser_content_view.h (right):

https://codereview.chromium.org/2468403002/diff/1/chrome/browser/ui/views/cho...
chrome/browser/ui/views/chooser_content_view.h:79: base::string16
help_text_for_test() const { return help_text_; }
How about friending the test and removing the for_test functions that clutter
the api?

https://codereview.chromium.org/2468403002/diff/1/ui/views/controls/styled_la...
File ui/views/controls/styled_label.h (right):

https://codereview.chromium.org/2468403002/diff/1/ui/views/controls/styled_la...
ui/views/controls/styled_label.h:71: base::string16 text() const { return text_;
}
const base::string16&?

[Robert Sesek, 2016-11-03 15:22:30]

Based on the screenshots in the bug, I think there's a little too much space
after the "Get help" link. Can the spacing be made even on the top and bottom?

[juncai, 2016-11-03 20:23:49]

The CQ bit was checked by juncai@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-03 20:24:31]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[juncai, 2016-11-03 20:26:24]

https://codereview.chromium.org/2468403002/diff/1/chrome/browser/ui/views/cho...
File chrome/browser/ui/views/chooser_content_view.cc (right):

https://codereview.chromium.org/2468403002/diff/1/chrome/browser/ui/views/cho...
chrome/browser/ui/views/chooser_content_view.cc:211: footnote_link_->Layout();
On 2016/11/03 13:24:23, sky wrote:
> This only works if the sized needed for the new text is exactly same as the
> previous size. Are you sure that's the case? I would expect a layout to occur
on
> the parent, e.g. footnote_link_->parent()->Layout(), and possibly
> footnote_link->parent()->parent()-Layout() if the number of lines might change
> (even that later might not be enough in that case). How do you know the text
> only fits one line?

Right, I changed the code to use the root view to call the Layout().

Done.

https://codereview.chromium.org/2468403002/diff/1/chrome/browser/ui/views/cho...
File chrome/browser/ui/views/chooser_content_view.h (right):

https://codereview.chromium.org/2468403002/diff/1/chrome/browser/ui/views/cho...
chrome/browser/ui/views/chooser_content_view.h:79: base::string16
help_text_for_test() const { return help_text_; }
On 2016/11/03 13:24:23, sky wrote:
> How about friending the test and removing the for_test functions that clutter
> the api?

Done.

https://codereview.chromium.org/2468403002/diff/1/ui/views/controls/styled_la...
File ui/views/controls/styled_label.h (right):

https://codereview.chromium.org/2468403002/diff/1/ui/views/controls/styled_la...
ui/views/controls/styled_label.h:71: base::string16 text() const { return text_;
}
On 2016/11/03 13:24:23, sky wrote:
> const base::string16&?

Done.

[juncai, 2016-11-03 20:26:58]

On 2016/11/03 15:22:30, Robert Sesek wrote:
> Based on the screenshots in the bug, I think there's a little too much space
> after the "Get help" link. Can the spacing be made even on the top and bottom?

Adjust the spacing to be even on the top and bottom of the "Get help" button on
Mac. Uploaded some screenshots on the issue page.

Done.

[commit-bot: I haz the power, 2016-11-03 21:53:07]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-03 21:53:08]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[sky, 2016-11-03 22:33:52]

LGTM

https://codereview.chromium.org/2468403002/diff/20001/chrome/browser/ui/views...
File chrome/browser/ui/views/chooser_content_view.cc (right):

https://codereview.chromium.org/2468403002/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/chooser_content_view.cc:213:
footnote_link_->GetWidget()->GetRootView()->Layout();
You really don't need footnote_link_ here (and below),
GetWidget()->GetRootView()->Layout() should be enough. Also, I'm assuming the
widget has enough height, as otherwise it's possible you need to increase the
overall widget height.

[juncai, 2016-11-04 02:24:20]

The CQ bit was checked by juncai@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-04 02:25:07]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[juncai, 2016-11-04 02:27:47]

https://codereview.chromium.org/2468403002/diff/20001/chrome/browser/ui/views...
File chrome/browser/ui/views/chooser_content_view.cc (right):

https://codereview.chromium.org/2468403002/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/chooser_content_view.cc:213:
footnote_link_->GetWidget()->GetRootView()->Layout();
On 2016/11/03 22:33:52, sky wrote:
> You really don't need footnote_link_ here (and below),
> GetWidget()->GetRootView()->Layout() should be enough. Also, I'm assuming the
> widget has enough height, as otherwise it's possible you need to increase the
> overall widget height.

I tested it with a long string, and the widget has enough height to display it.

Done.

[commit-bot: I haz the power, 2016-11-04 05:20:20]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-04 05:20:21]

Dry run: This issue passed the CQ dry run.

[Robert Sesek, 2016-11-04 16:32:58]

LGTM

[juncai, 2016-11-04 16:36:52]

The CQ bit was checked by juncai@chromium.org

[juncai, 2016-11-04 16:36:52]

The patchset sent to the CQ was uploaded after l-g-t-m from sky@chromium.org
Link to the patchset: https://codereview.chromium.org/2468403002/#ps40001
(title: "address comments")

[commit-bot: I haz the power, 2016-11-04 16:37:36]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-04 16:44:35]

Description was changed from

==========
Make WebBluetooth chooser consistent on Android and desktops (desktop part)

This CL makes the following changes:
1. Remove "Not seeing your device?" string.
2. Remove the period at "No Bluetooth devices found.", "No devices found."
3. Show get help link and scanning status text below the chooser:
(a) When the Bluetooth is turned off, show "Get help" link text.
(b) When scanning, show "Get help while scanning for devices...",
the "Get help" is link text, "while scanning for devices..." is plain text.
(c) When scanning and found some device, or scanning is complete, show
"Get help or re-scan", both "Get help" and "re-scan" are link text.

I uploaded some screenshots from ChromeOS and Mac on the issue page.

BUG=659271
==========

to

==========
Make WebBluetooth chooser consistent on Android and desktops (desktop part)

This CL makes the following changes:
1. Remove "Not seeing your device?" string.
2. Remove the period at "No Bluetooth devices found.", "No devices found."
3. Show get help link and scanning status text below the chooser:
(a) When the Bluetooth is turned off, show "Get help" link text.
(b) When scanning, show "Get help while scanning for devices...",
the "Get help" is link text, "while scanning for devices..." is plain text.
(c) When scanning and found some device, or scanning is complete, show
"Get help or re-scan", both "Get help" and "re-scan" are link text.

I uploaded some screenshots from ChromeOS and Mac on the issue page.

BUG=659271
==========

[commit-bot: I haz the power, 2016-11-04 16:44:36]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-11-04 17:15:56]

Description was changed from

==========
Make WebBluetooth chooser consistent on Android and desktops (desktop part)

This CL makes the following changes:
1. Remove "Not seeing your device?" string.
2. Remove the period at "No Bluetooth devices found.", "No devices found."
3. Show get help link and scanning status text below the chooser:
(a) When the Bluetooth is turned off, show "Get help" link text.
(b) When scanning, show "Get help while scanning for devices...",
the "Get help" is link text, "while scanning for devices..." is plain text.
(c) When scanning and found some device, or scanning is complete, show
"Get help or re-scan", both "Get help" and "re-scan" are link text.

I uploaded some screenshots from ChromeOS and Mac on the issue page.

BUG=659271
==========

to

==========
Make WebBluetooth chooser consistent on Android and desktops (desktop part)

This CL makes the following changes:
1. Remove "Not seeing your device?" string.
2. Remove the period at "No Bluetooth devices found.", "No devices found."
3. Show get help link and scanning status text below the chooser:
(a) When the Bluetooth is turned off, show "Get help" link text.
(b) When scanning, show "Get help while scanning for devices...",
the "Get help" is link text, "while scanning for devices..." is plain text.
(c) When scanning and found some device, or scanning is complete, show
"Get help or re-scan", both "Get help" and "re-scan" are link text.

I uploaded some screenshots from ChromeOS and Mac on the issue page.

BUG=659271

Committed: https://crrev.com/2e52a5a282f509f48e397626f5fcd30f5ab8b1fe
Cr-Commit-Position: refs/heads/master@{#429914}
==========

[commit-bot: I haz the power, 2016-11-04 17:15:58]

Patchset 3 (id:??) landed as
https://crrev.com/2e52a5a282f509f48e397626f5fcd30f5ab8b1fe
Cr-Commit-Position: refs/heads/master@{#429914}