Show the message 'No files match "abc"' if the search results are empty.

Show the message 'No files match "abc"' if the search results are empty.

Make search case-insensitive.

Match files by extensions and other substrings.

BUG=126633, 126704, 126993
TEST="The message is shown in place of file list if the search results are empty."
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=137377

[dgozman, 2012-05-15 13:12:38]

https://chromiumcodereview.appspot.com/10388142/diff/1/chrome/browser/resourc...
File chrome/browser/resources/file_manager/css/file_manager.css (right):

https://chromiumcodereview.appspot.com/10388142/diff/1/chrome/browser/resourc...
chrome/browser/resources/file_manager/css/file_manager.css:1290:
#list-container:not([no-search-results='true']) #no-search-results {
Drop the |='true'|. It's cleaner.

https://chromiumcodereview.appspot.com/10388142/diff/1/chrome/browser/resourc...
File chrome/browser/resources/file_manager/js/file_manager.js (right):

https://chromiumcodereview.appspot.com/10388142/diff/1/chrome/browser/resourc...
chrome/browser/resources/file_manager/js/file_manager.js:270:
FileManager.prototype.$ = function(id) {
Don't like this :(

https://chromiumcodereview.appspot.com/10388142/diff/1/chrome/browser/resourc...
chrome/browser/resources/file_manager/js/file_manager.js:4357: var index =
template.indexOf('$1');
You can just use <b>$1</b> inside the localized string and set innerHTML. See
NO_ACTION_FOR_FILE for example.

[SeRya, 2012-05-15 13:19:01]

https://chromiumcodereview.appspot.com/10388142/diff/1/chrome/browser/resourc...
File chrome/browser/resources/file_manager/css/file_manager.css (right):

https://chromiumcodereview.appspot.com/10388142/diff/1/chrome/browser/resourc...
chrome/browser/resources/file_manager/css/file_manager.css:1290:
#list-container:not([no-search-results='true']) #no-search-results {
Use 'hidden' instead of custom attribute.

[Vladislav Kaznacheev, 2012-05-15 13:38:12]

https://chromiumcodereview.appspot.com/10388142/diff/1/chrome/browser/resourc...
File chrome/browser/resources/file_manager/css/file_manager.css (right):

https://chromiumcodereview.appspot.com/10388142/diff/1/chrome/browser/resourc...
chrome/browser/resources/file_manager/css/file_manager.css:1287: top: 28px;  /*
Leave room for the file list header. */
28px is not needed when we are in the grid view

https://chromiumcodereview.appspot.com/10388142/diff/1/chrome/browser/resourc...
File chrome/browser/resources/file_manager/js/file_manager.js (right):

https://chromiumcodereview.appspot.com/10388142/diff/1/chrome/browser/resourc...
chrome/browser/resources/file_manager/js/file_manager.js:270:
FileManager.prototype.$ = function(id) {
Oh no.

https://chromiumcodereview.appspot.com/10388142/diff/1/chrome/browser/resourc...
chrome/browser/resources/file_manager/js/file_manager.js:4367: }
add .bind(this) and you do not need var self

[Oleg Eterevsky, 2012-05-15 13:55:27]

Please have another look.

https://chromiumcodereview.appspot.com/10388142/diff/1/chrome/browser/resourc...
File chrome/browser/resources/file_manager/css/file_manager.css (right):

https://chromiumcodereview.appspot.com/10388142/diff/1/chrome/browser/resourc...
chrome/browser/resources/file_manager/css/file_manager.css:1287: top: 28px;  /*
Leave room for the file list header. */
On 2012/05/15 13:38:13, Vladislav Kaznacheev wrote:
> 28px is not needed when we are in the grid view

It looks ok in grid view.

https://chromiumcodereview.appspot.com/10388142/diff/1/chrome/browser/resourc...
chrome/browser/resources/file_manager/css/file_manager.css:1290:
#list-container:not([no-search-results='true']) #no-search-results {
On 2012/05/15 13:12:39, dgozman wrote:
> Drop the |='true'|. It's cleaner.

Done.

https://chromiumcodereview.appspot.com/10388142/diff/1/chrome/browser/resourc...
chrome/browser/resources/file_manager/css/file_manager.css:1290:
#list-container:not([no-search-results='true']) #no-search-results {
On 2012/05/15 13:19:02, SeRya wrote:
> Use 'hidden' instead of custom attribute.

Done.

https://chromiumcodereview.appspot.com/10388142/diff/1/chrome/browser/resourc...
File chrome/browser/resources/file_manager/js/file_manager.js (right):

https://chromiumcodereview.appspot.com/10388142/diff/1/chrome/browser/resourc...
chrome/browser/resources/file_manager/js/file_manager.js:270:
FileManager.prototype.$ = function(id) {
On 2012/05/15 13:12:39, dgozman wrote:
> Don't like this :(

Done.

https://chromiumcodereview.appspot.com/10388142/diff/1/chrome/browser/resourc...
chrome/browser/resources/file_manager/js/file_manager.js:270:
FileManager.prototype.$ = function(id) {
On 2012/05/15 13:38:13, Vladislav Kaznacheev wrote:
> Oh no.

Done.

https://chromiumcodereview.appspot.com/10388142/diff/1/chrome/browser/resourc...
chrome/browser/resources/file_manager/js/file_manager.js:4357: var index =
template.indexOf('$1');
On 2012/05/15 13:12:39, dgozman wrote:
> You can just use <b>$1</b> inside the localized string and set innerHTML. See
> NO_ACTION_FOR_FILE for example.

Done.

https://chromiumcodereview.appspot.com/10388142/diff/1/chrome/browser/resourc...
chrome/browser/resources/file_manager/js/file_manager.js:4367: }
On 2012/05/15 13:38:13, Vladislav Kaznacheev wrote:
> add .bind(this) and you do not need var self

Done.

https://chromiumcodereview.appspot.com/10388142/diff/1/chrome/browser/resourc...
chrome/browser/resources/file_manager/js/file_manager.js:4367: }
On 2012/05/15 13:38:13, Vladislav Kaznacheev wrote:
> add .bind(this) and you do not need var self

Done.

[SeRya, 2012-05-15 14:08:03]

https://chromiumcodereview.appspot.com/10388142/diff/7/chrome/browser/resourc...
File chrome/browser/resources/file_manager/css/file_manager.css (right):

https://chromiumcodereview.appspot.com/10388142/diff/7/chrome/browser/resourc...
chrome/browser/resources/file_manager/css/file_manager.css:1291:
#no-search-results[hidden] {
There is global [hidden] rule, you only need to duplicate this because you set
dicplay:block above. Remove this reule and display:block.

https://chromiumcodereview.appspot.com/10388142/diff/7/chrome/browser/resourc...
File chrome/browser/resources/file_manager/js/file_manager.js (right):

https://chromiumcodereview.appspot.com/10388142/diff/7/chrome/browser/resourc...
chrome/browser/resources/file_manager/js/file_manager.js:4340: var searchString
= this.document_.getElementById('search-box').value;
var searchBox = this.document_.getElementById('search-box');

https://chromiumcodereview.appspot.com/10388142/diff/7/chrome/browser/resourc...
chrome/browser/resources/file_manager/js/file_manager.js:4352:
this.document_.getElementById('no-search-results').removeAttribute(
.hidden = false;

https://chromiumcodereview.appspot.com/10388142/diff/7/chrome/browser/resourc...
chrome/browser/resources/file_manager/js/file_manager.js:4358:
'rescan-completed', this.reportEmptySearchResults_);
The 'splice' event of fileList is better because it may be changed before rescan
happen (it is not completely wrong because rescan happens anyway but probably
with delay).

For instance we have code that instantly insert just copied file into the file
list not waiting rescan.

https://chromiumcodereview.appspot.com/10388142/diff/7/chrome/browser/resourc...
chrome/browser/resources/file_manager/js/file_manager.js:4361:
this.document_.getElementById('no-search-results').setAttribute('hidden',
There is a setter for 'hidden'. Use ".hidden = true".

[Oleg Eterevsky, 2012-05-15 14:16:50]

https://chromiumcodereview.appspot.com/10388142/diff/7/chrome/browser/resourc...
File chrome/browser/resources/file_manager/css/file_manager.css (right):

https://chromiumcodereview.appspot.com/10388142/diff/7/chrome/browser/resourc...
chrome/browser/resources/file_manager/css/file_manager.css:1291:
#no-search-results[hidden] {
On 2012/05/15 14:08:03, SeRya wrote:
> There is global [hidden] rule, you only need to duplicate this because you set
> dicplay:block above. Remove this reule and display:block.

Done.

https://chromiumcodereview.appspot.com/10388142/diff/7/chrome/browser/resourc...
File chrome/browser/resources/file_manager/js/file_manager.js (right):

https://chromiumcodereview.appspot.com/10388142/diff/7/chrome/browser/resourc...
chrome/browser/resources/file_manager/js/file_manager.js:4340: var searchString
= this.document_.getElementById('search-box').value;
On 2012/05/15 14:08:03, SeRya wrote:
> var searchBox = this.document_.getElementById('search-box');

Why? I don't need it.

https://chromiumcodereview.appspot.com/10388142/diff/7/chrome/browser/resourc...
chrome/browser/resources/file_manager/js/file_manager.js:4352:
this.document_.getElementById('no-search-results').removeAttribute(
On 2012/05/15 14:08:03, SeRya wrote:
> .hidden = false;

Done.

https://chromiumcodereview.appspot.com/10388142/diff/7/chrome/browser/resourc...
chrome/browser/resources/file_manager/js/file_manager.js:4358:
'rescan-completed', this.reportEmptySearchResults_);
On 2012/05/15 14:08:03, SeRya wrote:
> The 'splice' event of fileList is better because it may be changed before
rescan
> happen (it is not completely wrong because rescan happens anyway but probably
> with delay).
> 
> For instance we have code that instantly insert just copied file into the file
> list not waiting rescan.

I don't believe there is any difference.

https://chromiumcodereview.appspot.com/10388142/diff/7/chrome/browser/resourc...
chrome/browser/resources/file_manager/js/file_manager.js:4361:
this.document_.getElementById('no-search-results').setAttribute('hidden',
On 2012/05/15 14:08:03, SeRya wrote:
> There is a setter for 'hidden'. Use ".hidden = true".

Done.

[SeRya, 2012-05-15 14:21:28]

lgtm

https://chromiumcodereview.appspot.com/10388142/diff/11001/chrome/browser/res...
File chrome/browser/resources/file_manager/main.html (right):

https://chromiumcodereview.appspot.com/10388142/diff/11001/chrome/browser/res...
chrome/browser/resources/file_manager/main.html:168: <div id="no-search-results"
hidden="true"></div>
Just hidden. Without ="true"

[Oleg Eterevsky, 2012-05-15 14:27:08]

Added a few lines in FileManager.prototype.onSearchBoxUpdate_ to make search
case-insensitive and find substrings. Could you give another look?

https://chromiumcodereview.appspot.com/10388142/diff/11001/chrome/browser/res...
File chrome/browser/resources/file_manager/main.html (right):

https://chromiumcodereview.appspot.com/10388142/diff/11001/chrome/browser/res...
chrome/browser/resources/file_manager/main.html:168: <div id="no-search-results"
hidden="true"></div>
On 2012/05/15 14:21:29, SeRya wrote:
> Just hidden. Without ="true"

Done.

[dgozman, 2012-05-15 14:32:53]

I don't see the change about case sensitiveness and substrings.

https://chromiumcodereview.appspot.com/10388142/diff/11001/chrome/app/generat...
File chrome/app/generated_resources.grd (right):

https://chromiumcodereview.appspot.com/10388142/diff/11001/chrome/app/generat...
chrome/app/generated_resources.grd:11048: +        No files match &lt;b&gt;"<ph
name="SEARCH_STRING">$1<ex>abc</ex></ph>"&lt;/b&gt;
FYI. This cannot be merged.

[Oleg Eterevsky, 2012-05-16 06:51:28]

Sorry, last time I forgot to upload the change regarding case sensitivity and
substrings. I've done it now.

https://chromiumcodereview.appspot.com/10388142/diff/11001/chrome/app/generat...
File chrome/app/generated_resources.grd (right):

https://chromiumcodereview.appspot.com/10388142/diff/11001/chrome/app/generat...
chrome/app/generated_resources.grd:11048: +        No files match &lt;b&gt;"<ph
name="SEARCH_STRING">$1<ex>abc</ex></ph>"&lt;/b&gt;
On 2012/05/15 14:32:54, dgozman wrote:
> FYI. This cannot be merged.

It's no big deal. Shall I move it to a separate CL, or can I just leave it here
so that Satoru merge the CL without this change?

[SeRya, 2012-05-16 07:27:34]

lgtm