Files.app: Fit the size of search box as its contents size.

Files.app: Fit the size of search box as its contents size.

This CL updated CSS and javascript to make the size of search box fit to its
contents size.  In the JavaScript funciton 'updateSearchBoxStyles_', we measure
the widht of search box text by using TextMeasure class and set the search box
size depending on the width.

TextMeasure class has a dummy span element. When measuring text size, it sets
the text to the dummy span element and obtains its size by using
getBoundingClientRect method.

This CL also added #search-icon and #search-clear-button elements. Originally
these UI parts are implemented as the background image and
-webkit-search-cancel-button of the serach box respectively. This way had a
trouble about rtl mode thus we need the specific selectors for the rtl mode. On
the other hands, current implementation can handle rtl mode without other
selectors.

BUG=241300, 239292
TEST=manually. rtl mode and legacy UI are also checked.

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=202375

[hirono, 2013-05-24 09:23:20]

Yoshiki-san,

Could you check the CL of growing search box?
Thank you very much.

[yoshiki, 2013-05-24 09:56:15]

https://codereview.chromium.org/15946005/diff/2001/chrome/browser/resources/f...
File chrome/browser/resources/file_manager/js/file_manager.js (right):

https://codereview.chromium.org/15946005/diff/2001/chrome/browser/resources/f...
chrome/browser/resources/file_manager/js/file_manager.js:3465: var
TEXT_BOX_PADDING = 16;
Add the comment '// in px.'

https://codereview.chromium.org/15946005/diff/2001/chrome/browser/resources/f...
chrome/browser/resources/file_manager/js/file_manager.js:3861: * TextMeasure is
a measure for text that returns the width of text.
I think this approach (creating dummySpan and measure it) is somewhat hacky.
Please write the comment about the implementation.

https://codereview.chromium.org/15946005/diff/2001/chrome/browser/resources/f...
chrome/browser/resources/file_manager/js/file_manager.js:3867: this.dummySpan =
document.createElement('span');
This is a private variable, so please add an underscode at the end of the name.

https://codereview.chromium.org/15946005/diff/2001/chrome/browser/resources/f...
chrome/browser/resources/file_manager/js/file_manager.js:3868: this.dummySpan =
document.getElementsByTagName('body')[0]
Pass FileManager.dialogDom_ or (dialogDom_.ownerDocument) from the caller and
use it instead of direct use of 'document'.

https://codereview.chromium.org/15946005/diff/2001/chrome/browser/resources/f...
chrome/browser/resources/file_manager/js/file_manager.js:3873: var copiedStyles
= ['fontSize',
"attributesToBeCopied" (or something like it) is better than "copiesStyles"?

https://codereview.chromium.org/15946005/diff/2001/chrome/browser/resources/f...
chrome/browser/resources/file_manager/js/file_manager.js:3879:
console.log(copiedStyles[i] + ': ' + styles[copiedStyles[i]]);
remove debug log

https://codereview.chromium.org/15946005/diff/2001/chrome/browser/resources/f...
chrome/browser/resources/file_manager/js/file_manager.js:3886: return
this.dummySpan.getBoundingClientRect().width;
getBoundingClientRect() returns null if the element is hidden. Please add
null-check.

[yoshiki, 2013-05-24 09:58:33]

https://codereview.chromium.org/15946005/diff/2001/chrome/browser/resources/f...
File chrome/browser/resources/file_manager/js/file_manager.js (right):

https://codereview.chromium.org/15946005/diff/2001/chrome/browser/resources/f...
chrome/browser/resources/file_manager/js/file_manager.js:3872: var styles =
getComputedStyle(element, '');
window.getComputedStyle()

[hirono, 2013-05-24 10:36:43]

Fixed.
Thank you very much!

https://codereview.chromium.org/15946005/diff/2001/chrome/browser/resources/f...
File chrome/browser/resources/file_manager/js/file_manager.js (right):

https://codereview.chromium.org/15946005/diff/2001/chrome/browser/resources/f...
chrome/browser/resources/file_manager/js/file_manager.js:3465: var
TEXT_BOX_PADDING = 16;
On 2013/05/24 09:56:16, yoshiki wrote:
> Add the comment '// in px.'

Done.

https://codereview.chromium.org/15946005/diff/2001/chrome/browser/resources/f...
chrome/browser/resources/file_manager/js/file_manager.js:3861: * TextMeasure is
a measure for text that returns the width of text.
On 2013/05/24 09:56:16, yoshiki wrote:
> I think this approach (creating dummySpan and measure it) is somewhat hacky.
> Please write the comment about the implementation.

Done.

https://codereview.chromium.org/15946005/diff/2001/chrome/browser/resources/f...
chrome/browser/resources/file_manager/js/file_manager.js:3867: this.dummySpan =
document.createElement('span');
On 2013/05/24 09:56:16, yoshiki wrote:
> This is a private variable, so please add an underscode at the end of the
name.

Done.

https://codereview.chromium.org/15946005/diff/2001/chrome/browser/resources/f...
chrome/browser/resources/file_manager/js/file_manager.js:3868: this.dummySpan =
document.getElementsByTagName('body')[0]
On 2013/05/24 09:56:16, yoshiki wrote:
> Pass FileManager.dialogDom_ or (dialogDom_.ownerDocument) from the caller and
> use it instead of direct use of 'document'.

How about using element.document?
|element| is already passed as an argument.

https://codereview.chromium.org/15946005/diff/2001/chrome/browser/resources/f...
chrome/browser/resources/file_manager/js/file_manager.js:3872: var styles =
getComputedStyle(element, '');
On 2013/05/24 09:58:33, yoshiki wrote:
> window.getComputedStyle()

Done.

https://codereview.chromium.org/15946005/diff/2001/chrome/browser/resources/f...
chrome/browser/resources/file_manager/js/file_manager.js:3873: var copiedStyles
= ['fontSize',
On 2013/05/24 09:56:16, yoshiki wrote:
> "attributesToBeCopied" (or something like it) is better than "copiesStyles"?

attributes looks like HTML attributes. So 'stylesToBeCopied' is OK?

https://codereview.chromium.org/15946005/diff/2001/chrome/browser/resources/f...
chrome/browser/resources/file_manager/js/file_manager.js:3879:
console.log(copiedStyles[i] + ': ' + styles[copiedStyles[i]]);
On 2013/05/24 09:56:16, yoshiki wrote:
> remove debug log

Done.

https://codereview.chromium.org/15946005/diff/2001/chrome/browser/resources/f...
chrome/browser/resources/file_manager/js/file_manager.js:3886: return
this.dummySpan.getBoundingClientRect().width;
On 2013/05/24 09:56:16, yoshiki wrote:
> getBoundingClientRect() returns null if the element is hidden. Please add
> null-check.

Done.

[yoshiki, 2013-05-24 14:49:54]

lgtm with nits

https://codereview.chromium.org/15946005/diff/16001/chrome/browser/resources/...
File chrome/browser/resources/file_manager/js/file_manager.js (right):

https://codereview.chromium.org/15946005/diff/16001/chrome/browser/resources/...
chrome/browser/resources/file_manager/js/file_manager.js:3858: /**
nit: Please move this class to the first part of this file. (maybe around L17 or
L78 is good place).  Utility class/function should be placed at the first of the
file.

https://codereview.chromium.org/15946005/diff/16001/chrome/browser/resources/...
chrome/browser/resources/file_manager/js/file_manager.js:3867: *     width of
text is mesures like as it is rendered in this element.
nit: add '@constructor'

https://codereview.chromium.org/15946005/diff/16001/chrome/browser/resources/...
chrome/browser/resources/file_manager/js/file_manager.js:3873:
.appendChild(this.dummySpan_);
nit: a dot should be at the last of the previous line.

[hirono, 2013-05-27 01:58:28]

Fixed.
Thank you!

https://codereview.chromium.org/15946005/diff/16001/chrome/browser/resources/...
File chrome/browser/resources/file_manager/js/file_manager.js (right):

https://codereview.chromium.org/15946005/diff/16001/chrome/browser/resources/...
chrome/browser/resources/file_manager/js/file_manager.js:3858: /**
On 2013/05/24 14:49:54, yoshiki wrote:
> nit: Please move this class to the first part of this file. (maybe around L17
or
> L78 is good place).  Utility class/function should be placed at the first of
the
> file.

Done.

https://codereview.chromium.org/15946005/diff/16001/chrome/browser/resources/...
chrome/browser/resources/file_manager/js/file_manager.js:3867: *     width of
text is mesures like as it is rendered in this element.
On 2013/05/24 14:49:54, yoshiki wrote:
> nit: add '@constructor'

Done.

https://codereview.chromium.org/15946005/diff/16001/chrome/browser/resources/...
chrome/browser/resources/file_manager/js/file_manager.js:3873:
.appendChild(this.dummySpan_);
On 2013/05/24 14:49:54, yoshiki wrote:
> nit: a dot should be at the last of the previous line.

Done.

[commit-bot: I haz the power, 2013-05-27 01:58:40]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/hirono@chromium.org/15946005/22001

[commit-bot: I haz the power, 2013-05-27 06:08:20]

Commit queue rejected this change because the description was changed
between the time the change entered the commit queue and the time it
was ready to commit. You can safely check the commit box again.

[commit-bot: I haz the power, 2013-05-27 06:13:32]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/hirono@chromium.org/15946005/22001

[commit-bot: I haz the power, 2013-05-27 06:13:52]

Change committed as 202375