MD Settings: Fix label visibility in subpage search fields

MD Settings: Fix label visibility in subpage search fields

In crrev.com/438430, CrSearchFieldBehavior was simplified, removing
compatibility with iron-input. However, iron-input was still used by
<settings-subpage-search> to control the visibility of the input field
label.

This CL adds a public 'hasSearchText' property to the common search
field behavior which does not require iron-input to compute. This allows
<settings-subpage-search> to remove its dependency on iron-input, while
also fixing two minor issues:
- The input field label on <settings-subpage-search> now reappears when
  the 'clear search' button is clicked.
- The search icon in <cr-toolbar-search-field> now appears unfocused
  when the search field is cleared while it is unfocused.

BUG=665700, 665307, 634665
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2586803002
Cr-Commit-Position: refs/heads/master@{#444578}
Committed: https://chromium.googlesource.com/chromium/src/+/d69fa965ea7ddc2e61c7701a6b209c1d5f8bccda

[tsergeant, 2016-12-18 23:11:45]

Description was changed from

==========
MD Settings: Fix label visibility in subpage search fields

In crrev.com/438430, CrSearchFieldBehavior was simplified to remove
compatibility with iron-input's bindValue property. However, this
property was still required by <settings-subpage-search> to control
visibility of the input field label.

This restores the deleted code, so that the label will correctly
reappear when the 'clear search' button is clicked.

BUG=665700
==========

to

==========
MD Settings: Fix label visibility in subpage search fields

In crrev.com/438430, CrSearchFieldBehavior was simplified to remove
compatibility with iron-input's bindValue property. However, this
property was still required by <settings-subpage-search> to control
visibility of the input field label.

This restores the deleted code, so that the label will correctly
reappear when the 'clear search' button is clicked.

BUG=665700
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[tsergeant, 2016-12-18 23:11:57]

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

[commit-bot: I haz the power, 2016-12-18 23:12:01]

Dry run: 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-12-19 01:38:57]

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

[commit-bot: I haz the power, 2016-12-19 01:38:57]

Dry run: This issue passed the CQ dry run.

[tsergeant, 2016-12-19 04:36:59]

tsergeant@chromium.org changed reviewers:
+ dpapad@chromium.org

[tsergeant, 2016-12-19 04:37:00]

PTAL, just a small fix to the patch from last week.

[dpapad, 2016-12-19 19:14:14]

On 2016/12/19 at 04:37:00, tsergeant wrote:
> PTAL, just a small fix to the patch from last week.

LGTM. But also, does settings_subpage_search.html need to use iron-input instead
of plain input? The only MD styling I see is the expanding underline on focus,
which we can easily replacate (already done for md-select).

[tsergeant, 2017-01-17 02:58:04]

Description was changed from

==========
MD Settings: Fix label visibility in subpage search fields

In crrev.com/438430, CrSearchFieldBehavior was simplified to remove
compatibility with iron-input's bindValue property. However, this
property was still required by <settings-subpage-search> to control
visibility of the input field label.

This restores the deleted code, so that the label will correctly
reappear when the 'clear search' button is clicked.

BUG=665700
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
MD Settings: Fix label visibility in subpage search fields

In crrev.com/438430, CrSearchFieldBehavior was simplified, removing
compatibility with iron-input. However, iron-input was still used by
<settings-subpage-search> to control the visibility of the input field
label.

This CL adds a public 'hasSearchText' property to the common search
field behavior which does not require iron-input to compute. This allows
<settings-subpage-search> to remove its dependency on iron-input, while
also fixing the input field label to reappear when the 'clear search'
button is clicked.

BUG=665700
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[tsergeant, 2017-01-17 03:05:32]

Patchset #2 (id:20001) has been deleted

[tsergeant, 2017-01-17 03:05:44]

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

[commit-bot: I haz the power, 2017-01-17 03:06:02]

Dry run: 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, 2017-01-17 03:14:52]

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

[commit-bot: I haz the power, 2017-01-17 03:14:53]

Dry run: 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_compila...)

[tsergeant, 2017-01-17 03:41:05]

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

[commit-bot: I haz the power, 2017-01-17 03:41:10]

Dry run: 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, 2017-01-17 04:34:11]

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

[commit-bot: I haz the power, 2017-01-17 04:34:12]

Dry run: This issue passed the CQ dry run.

[tsergeant, 2017-01-17 06:51:53]

Patchset #2 (id:40001) has been deleted

[tsergeant, 2017-01-17 07:04:26]

On 2016/12/19 19:14:14, dpapad wrote:
> On 2016/12/19 at 04:37:00, tsergeant wrote:
> > PTAL, just a small fix to the patch from last week.
> 
> LGTM. But also, does settings_subpage_search.html need to use iron-input
instead
> of plain input? The only MD styling I see is the expanding underline on focus,
> which we can easily replacate (already done for md-select).

I finally remembered to come back to this (better late than never, I guess).

iron-input was only used for showing the prompt, so there isn't really much
value in keeping it around.

I've uploaded a new patchset which removes the iron-input and implements the
bugfix in the process, by moving some functionality down from
<cr-toolbar-search-field> into the CrSearchFieldBehavior. I think this approach
is clearer, and it kind of cements the CrSearchFieldBehavior as having useful
functionality.

Anyway, PTAL!

[dpapad, 2017-01-18 02:52:26]

Does this by any chance also fix
https://bugs.chromium.org/p/chromium/issues/detail?id=665307?

https://codereview.chromium.org/2586803002/diff/60001/ui/webui/resources/cr_e...
File ui/webui/resources/cr_elements/cr_toolbar/cr_toolbar_search_field.js
(right):

https://codereview.chromium.org/2586803002/diff/60001/ui/webui/resources/cr_e...
ui/webui/resources/cr_elements/cr_toolbar/cr_toolbar_search_field.js:66: if
(this.hasSearchText)
Shouldn't we update the boolean either way?

this.showingSearch = this.hasSearchText;

[tsergeant, 2017-01-18 04:13:18]

Patchset #3 (id:80001) has been deleted

[tsergeant, 2017-01-18 04:13:56]

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

[tsergeant, 2017-01-18 04:14:05]

It does now!

https://codereview.chromium.org/2586803002/diff/60001/ui/webui/resources/cr_e...
File ui/webui/resources/cr_elements/cr_toolbar/cr_toolbar_search_field.js
(right):

https://codereview.chromium.org/2586803002/diff/60001/ui/webui/resources/cr_e...
ui/webui/resources/cr_elements/cr_toolbar/cr_toolbar_search_field.js:66: if
(this.hasSearchText)
On 2017/01/18 02:52:26, dpapad wrote:
> Shouldn't we update the boolean either way?
> 
> this.showingSearch = this.hasSearchText;

Hmm, almost: we don't want to close the search field as soon as there's no text
in it -- otherwise, backspacing all the text away would be enough to close the
field.

However if we take focus into account, we can get the correct behavior while
fixing the bug you mention above:
this.showingSearch = this.hasSearchText || this.isSearchFocused();

[commit-bot: I haz the power, 2017-01-18 04:14:08]

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

[tsergeant, 2017-01-18 04:17:28]

Description was changed from

==========
MD Settings: Fix label visibility in subpage search fields

In crrev.com/438430, CrSearchFieldBehavior was simplified, removing
compatibility with iron-input. However, iron-input was still used by
<settings-subpage-search> to control the visibility of the input field
label.

This CL adds a public 'hasSearchText' property to the common search
field behavior which does not require iron-input to compute. This allows
<settings-subpage-search> to remove its dependency on iron-input, while
also fixing the input field label to reappear when the 'clear search'
button is clicked.

BUG=665700
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
MD Settings: Fix label visibility in subpage search fields

In crrev.com/438430, CrSearchFieldBehavior was simplified, removing
compatibility with iron-input. However, iron-input was still used by
<settings-subpage-search> to control the visibility of the input field
label.

This CL adds a public 'hasSearchText' property to the common search
field behavior which does not require iron-input to compute. This allows
<settings-subpage-search> to remove its dependency on iron-input, while
also fixing two minor issues:
- The input field label on <settings-subpage-search> now reappears when
  the 'clear search' button is clicked.
- The search icon in <cr-toolbar-search-field> now appears unfocused
  when the search field is cleared while it is unfocused.

BUG=665700,665307
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[commit-bot: I haz the power, 2017-01-18 05:09:39]

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

[commit-bot: I haz the power, 2017-01-18 05:09:40]

Dry run: This issue passed the CQ dry run.

[tsergeant, 2017-01-18 06:43:27]

Description was changed from

==========
MD Settings: Fix label visibility in subpage search fields

In crrev.com/438430, CrSearchFieldBehavior was simplified, removing
compatibility with iron-input. However, iron-input was still used by
<settings-subpage-search> to control the visibility of the input field
label.

This CL adds a public 'hasSearchText' property to the common search
field behavior which does not require iron-input to compute. This allows
<settings-subpage-search> to remove its dependency on iron-input, while
also fixing two minor issues:
- The input field label on <settings-subpage-search> now reappears when
  the 'clear search' button is clicked.
- The search icon in <cr-toolbar-search-field> now appears unfocused
  when the search field is cleared while it is unfocused.

BUG=665700,665307
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
MD Settings: Fix label visibility in subpage search fields

In crrev.com/438430, CrSearchFieldBehavior was simplified, removing
compatibility with iron-input. However, iron-input was still used by
<settings-subpage-search> to control the visibility of the input field
label.

This CL adds a public 'hasSearchText' property to the common search
field behavior which does not require iron-input to compute. This allows
<settings-subpage-search> to remove its dependency on iron-input, while
also fixing two minor issues:
- The input field label on <settings-subpage-search> now reappears when
  the 'clear search' button is clicked.
- The search icon in <cr-toolbar-search-field> now appears unfocused
  when the search field is cleared while it is unfocused.

BUG=665700,665307,634665
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[dpapad, 2017-01-18 17:56:04]

lgtm

[tsergeant, 2017-01-18 23:14:36]

The CQ bit was checked by tsergeant@chromium.org

[commit-bot: I haz the power, 2017-01-18 23:15:12]

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, 2017-01-18 23:25:45]

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

[commit-bot: I haz the power, 2017-01-18 23:25:49]

Failed to apply patch for chrome/browser/resources/md_downloads/vulcanized.html:
While running git apply --index -p1;
  error: patch failed:
chrome/browser/resources/md_downloads/vulcanized.html:2903
  error: chrome/browser/resources/md_downloads/vulcanized.html: patch does not
apply

Patch:       chrome/browser/resources/md_downloads/vulcanized.html
Index: chrome/browser/resources/md_downloads/vulcanized.html
diff --git a/chrome/browser/resources/md_downloads/vulcanized.html
b/chrome/browser/resources/md_downloads/vulcanized.html
index
252b5b2610b672fad3bd1e857e09ceeee689beba..151dfdce7c82cb312c7d246964c28035e477d758
100644
--- a/chrome/browser/resources/md_downloads/vulcanized.html
+++ b/chrome/browser/resources/md_downloads/vulcanized.html
@@ -2903,6 +2903,7 @@ to {
     <defs>
       
       <g id="add"><path d="M19 13h-6v6h-2v-6H5v-2h6V5h2v6h6v2z"></path></g>
+      <g id="arrow-drop-up"><path d="M7 14l5-5 5 5z"></path></g>
       <g id="arrow-drop-down"><path d="M7 10l5 5 5-5z"></path></g>
       <g id="cancel"><path d="M12 2C6.47 2 2 6.47 2 12s4.47 10 10 10 10-4.47
10-10S17.53 2 12 2zm5 13.59L15.59 17 12 13.41 8.41 17 7 15.59 10.59 12 7 8.41
8.41 7 12 10.59 15.59 7 17 8.41 13.41 12 17 15.59z"></path></g>
 <if expr="chromeos">
@@ -2917,6 +2918,7 @@ to {
       <g id="expand-more"><path d="M16.59 8.59L12 13.17 7.41 8.59 6 10l6 6
6-6z"></path></g>
       <g id="extension"><path d="M20.5 11H19V7c0-1.1-.9-2-2-2h-4V3.5C13 2.12
11.88 1 10.5 1S8 2.12 8 3.5V5H4c-1.1 0-1.99.9-1.99 2v3.8H3.5c1.49 0 2.7 1.21 2.7
2.7s-1.21 2.7-2.7 2.7H2V20c0 1.1.9 2 2 2h3.8v-1.5c0-1.49 1.21-2.7 2.7-2.7 1.49 0
2.7 1.21 2.7 2.7V22H17c1.1 0 2-.9 2-2v-4h1.5c1.38 0 2.5-1.12 2.5-2.5S21.88 11
20.5 11z"></path></g>
       <g id="file-download"><path d="M19 9h-4V3H9v6H5l7 7 7-7zM5
18v2h14v-2H5z"></path></g>
+      <g id="folder"><path d="M10 4H4c-1.1 0-1.99.9-1.99 2L2 18c0 1.1.9 2 2
2h16c1.1 0 2-.9 2-2V8c0-1.1-.9-2-2-2h-8l-2-2z"></path></g>
       <g id="fullscreen"><path d="M7 14H5v5h5v-2H7v-3zm-2-4h2V7h3V5H5v5zm12
7h-3v2h5v-5h-2v3zM14 5v2h3v3h2V5h-5z"></path></g>
       <g id="group"><path d="M16 11c1.66 0 2.99-1.34 2.99-3S17.66 5 16 5c-1.66
0-3 1.34-3 3s1.34 3 3 3zm-8 0c1.66 0 2.99-1.34 2.99-3S9.66 5 8 5C6.34 5 5 6.34 5
8s1.34 3 3 3zm0 2c-2.33 0-7 1.17-7 3.5V19h14v-2.5c0-2.33-4.67-3.5-7-3.5zm8
0c-.29 0-.62.02-.97.05 1.16.84 1.97 1.97 1.97
3.45V19h6v-2.5c0-2.33-4.67-3.5-7-3.5z"></path></g>
       <g id="menu"><path d="M3
18h18v-2H3v2zm0-5h18v-2H3v2zm0-7v2h18V6H3z"></path></g>
@@ -2999,7 +3001,7 @@ label {
         white-space: nowrap;
 }
 
-:host([has-search-text_]) label {
+:host([has-search-text]) label {
   visibility: hidden;
 }
 
@@ -3056,10 +3058,10 @@ input[type='search']::-webkit-search-decoration,
input[type='search']::-webkit-s
     </paper-icon-button>
     <div id="searchTerm">
       <label id="prompt" for="searchInput">[[label]]</label>
-      <input id="searchInput" type="search" on-input="onSearchInput_"
on-search="onSearchTermSearch" on-keydown="onSearchTermKeydown_"
on-focus="onInputFocus_" on-blur="onInputBlur_" incremental="" autofocus="">
+      <input id="searchInput" type="search" on-input="onSearchTermInput"
on-search="onSearchTermSearch" on-keydown="onSearchTermKeydown_"
on-focus="onInputFocus_" on-blur="onInputBlur_" incremental="" autofocus="">
       
     </div>
-    <template is="dom-if" if="[[hasSearchText_]]">
+    <template is="dom-if" if="[[hasSearchText]]">
       <paper-icon-button icon="cr:cancel" id="clearSearch"
title="[[clearLabel]]" on-tap="clearSearch_">
       </paper-icon-button>
     </template>

[tsergeant, 2017-01-18 23:44:24]

The CQ bit was checked by tsergeant@chromium.org

[tsergeant, 2017-01-18 23:44:25]

The patchset sent to the CQ was uploaded after l-g-t-m from dpapad@chromium.org
Link to the patchset: https://codereview.chromium.org/2586803002/#ps140001
(title: "Rebase")

[commit-bot: I haz the power, 2017-01-18 23:45:08]

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, 2017-01-19 01:00:48]

CQ is committing da patch.

Bot data: {"patchset_id": 140001, "attempt_start_ts": 1484783064701000,
"parent_rev": "846a1b9762d89d23f293b0492df433e2239f71f5", "commit_rev":
"d69fa965ea7ddc2e61c7701a6b209c1d5f8bccda"}

[commit-bot: I haz the power, 2017-01-19 01:01:22]

Description was changed from

==========
MD Settings: Fix label visibility in subpage search fields

In crrev.com/438430, CrSearchFieldBehavior was simplified, removing
compatibility with iron-input. However, iron-input was still used by
<settings-subpage-search> to control the visibility of the input field
label.

This CL adds a public 'hasSearchText' property to the common search
field behavior which does not require iron-input to compute. This allows
<settings-subpage-search> to remove its dependency on iron-input, while
also fixing two minor issues:
- The input field label on <settings-subpage-search> now reappears when
  the 'clear search' button is clicked.
- The search icon in <cr-toolbar-search-field> now appears unfocused
  when the search field is cleared while it is unfocused.

BUG=665700,665307,634665
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
MD Settings: Fix label visibility in subpage search fields

In crrev.com/438430, CrSearchFieldBehavior was simplified, removing
compatibility with iron-input. However, iron-input was still used by
<settings-subpage-search> to control the visibility of the input field
label.

This CL adds a public 'hasSearchText' property to the common search
field behavior which does not require iron-input to compute. This allows
<settings-subpage-search> to remove its dependency on iron-input, while
also fixing two minor issues:
- The input field label on <settings-subpage-search> now reappears when
  the 'clear search' button is clicked.
- The search icon in <cr-toolbar-search-field> now appears unfocused
  when the search field is cleared while it is unfocused.

BUG=665700,665307,634665
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2586803002
Cr-Commit-Position: refs/heads/master@{#444578}
Committed:
https://chromium.googlesource.com/chromium/src/+/d69fa965ea7ddc2e61c7701a6b20...
==========

[commit-bot: I haz the power, 2017-01-19 01:01:24]

Committed patchset #5 (id:140001) as
https://chromium.googlesource.com/chromium/src/+/d69fa965ea7ddc2e61c7701a6b20...