MD WebUI: Center search field correctly inside cr-toolbar

MD WebUI: Center search field correctly inside cr-toolbar

This changes the behavior of the search field so that it no longer
expands on open in wide windows. This makes it possible to center the
search field inside the toolbar. Also tweaks the open/close animation
for the toolbar in narrow mode to be smoother.

BUG=610609
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation;master.tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/1ce950b1dfba435c8671b75c82bd2256a0718533
Cr-Commit-Position: refs/heads/master@{#405681}

[tsergeant, 2016-07-12 02:54:31]

Description was changed from

==========
MD WebUI: Center search field correctly inside cr-toolbar

This changes the behavior of the search field so that it no longer
expands on open in wide windows. This makes it possible to center the
search field inside the toolbar.

BUG=610609
==========

to

==========
MD WebUI: Center search field correctly inside cr-toolbar

This changes the behavior of the search field so that it no longer
expands on open in wide windows. This makes it possible to center the
search field inside the toolbar.

BUG=610609
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

[tsergeant, 2016-07-12 03:57:26]

Description was changed from

==========
MD WebUI: Center search field correctly inside cr-toolbar

This changes the behavior of the search field so that it no longer
expands on open in wide windows. This makes it possible to center the
search field inside the toolbar.

BUG=610609
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

to

==========
MD WebUI: Center search field correctly inside cr-toolbar

This changes the behavior of the search field so that it no longer
expands on open in wide windows. This makes it possible to center the
search field inside the toolbar. Also tweaks the open/close animation
for the toolbar in narrow mode to be smoother, and changes the font size
of search text.

BUG=610609
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

[tsergeant, 2016-07-12 04:00:04]

Description was changed from

==========
MD WebUI: Center search field correctly inside cr-toolbar

This changes the behavior of the search field so that it no longer
expands on open in wide windows. This makes it possible to center the
search field inside the toolbar. Also tweaks the open/close animation
for the toolbar in narrow mode to be smoother, and changes the font size
of search text.

BUG=610609
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

to

==========
MD WebUI: Center search field correctly inside cr-toolbar

This changes the behavior of the search field so that it no longer
expands on open in wide windows. This makes it possible to center the
search field inside the toolbar. Also tweaks the open/close animation
for the toolbar in narrow mode to be smoother.

BUG=610609
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

[tsergeant, 2016-07-12 04:02:20]

Patchset #1 (id:1) has been deleted

[tsergeant, 2016-07-12 04:02:27]

Patchset #1 (id:20001) has been deleted

[tsergeant, 2016-07-12 04:02:35]

Patchset #1 (id:40001) has been deleted

[tsergeant, 2016-07-12 04:04:02]

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

[commit-bot: I haz the power, 2016-07-12 04:04:36]

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

[tsergeant, 2016-07-12 06:05:38]

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

[tsergeant, 2016-07-12 06:05:39]

PTAL!

[commit-bot: I haz the power, 2016-07-12 06:47:37]

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

[commit-bot: I haz the power, 2016-07-12 06:47:38]

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

[dpapad, 2016-07-12 20:25:10]

+dbeam

Now that the leftContent is absolutely positioned, it seems that it is not
handling the case of a super long "Setings" string (in a language other than
English), see http://imgur.com/1axE7Rq. Not sure how we want to handle this
(eliding the "Settings" string maybe)? Not sure how would we detect that eliding
is necessary though.

[Dan Beam, 2016-07-12 20:37:36]

On 2016/07/12 20:25:10, dpapad wrote:
> +dbeam
> 
> Now that the leftContent is absolutely positioned, it seems that it is not
> handling the case of a super long "Setings" string (in a language other than
> English), see http://imgur.com/1axE7Rq. Not sure how we want to handle this
> (eliding the "Settings" string maybe)? Not sure how would we detect that
eliding
> is necessary though.

Does "Einstellungen" fit?

[tsergeant, 2016-07-12 23:46:33]

On 2016/07/12 20:37:36, Dan Beam wrote:
> On 2016/07/12 20:25:10, dpapad wrote:
> > +dbeam
> > 
> > Now that the leftContent is absolutely positioned, it seems that it is not
> > handling the case of a super long "Setings" string (in a language other than
> > English), see http://imgur.com/1axE7Rq. Not sure how we want to handle this
> > (eliding the "Settings" string maybe)? Not sure how would we detect that
> eliding
> > is necessary though.
> 
> Does "Einstellungen" fit?

I looked into this a little bit. I couldn't find any translations which don't
fit, but that doesn't mean there aren't any.

One way to deal with it is to manually calculate the max-width of leftContent as
(width of the toolbar at breakpoint - width of search field - padding) / 2,
which works out to around  140px. I don't think there's any way to elide the
text automagically.

[tsergeant, 2016-07-13 06:45:39]

On 2016/07/12 23:46:33, tsergeant wrote:
> On 2016/07/12 20:37:36, Dan Beam wrote:
> > On 2016/07/12 20:25:10, dpapad wrote:
> > > +dbeam
> > > 
> > > Now that the leftContent is absolutely positioned, it seems that it is not
> > > handling the case of a super long "Setings" string (in a language other
than
> > > English), see http://imgur.com/1axE7Rq. Not sure how we want to handle
this
> > > (eliding the "Settings" string maybe)? Not sure how would we detect that
> > eliding
> > > is necessary though.
> > 
> > Does "Einstellungen" fit?
> 
> I looked into this a little bit. I couldn't find any translations which don't
> fit, but that doesn't mean there aren't any.
> 
> One way to deal with it is to manually calculate the max-width of leftContent
as
> (width of the toolbar at breakpoint - width of search field - padding) / 2,
> which works out to around  140px. I don't think there's any way to elide the
> text automagically.

Looking into this more, the Spanish string for settings ("Configuración") is
long enough to align exactly with the start of the field.

I see a couple of main options:
* Change the break point from 900px to something like 950px.
* Force the search field not to occupy the space taken up by #leftContent. I've
uploaded a new patchset which does this in a simple way, but hardcodes in a
width.

I think any 'perfect' solution is going to require dynamically calculating
widths, which I'd rather not get into for something affecting a small number of
languages at very specific window widths.

WDYT?

[dpapad, 2016-07-13 20:50:04]

On 2016/07/13 at 06:45:39, tsergeant wrote:
> On 2016/07/12 23:46:33, tsergeant wrote:
> > On 2016/07/12 20:37:36, Dan Beam wrote:
> > > On 2016/07/12 20:25:10, dpapad wrote:
> > > > +dbeam
> > > > 
> > > > Now that the leftContent is absolutely positioned, it seems that it is
not
> > > > handling the case of a super long "Setings" string (in a language other
than
> > > > English), see http://imgur.com/1axE7Rq. Not sure how we want to handle
this
> > > > (eliding the "Settings" string maybe)? Not sure how would we detect that
> > > eliding
> > > > is necessary though.
> > > 
> > > Does "Einstellungen" fit?
> > 
> > I looked into this a little bit. I couldn't find any translations which
don't
> > fit, but that doesn't mean there aren't any.
> > 
> > One way to deal with it is to manually calculate the max-width of
leftContent as
> > (width of the toolbar at breakpoint - width of search field - padding) / 2,
> > which works out to around  140px. I don't think there's any way to elide the
> > text automagically.
> 
> Looking into this more, the Spanish string for settings ("Configuración") is
long enough to align exactly with the start of the field.
> 
> I see a couple of main options:
> * Change the break point from 900px to something like 950px.
> * Force the search field not to occupy the space taken up by #leftContent.
I've uploaded a new patchset which does this in a simple way, but hardcodes in a
width.
> 
> I think any 'perfect' solution is going to require dynamically calculating
widths, which I'd rather not get into for something affecting a small number of
languages at very specific window widths.
> 
> WDYT?

I increased the font size to "Very large", and was still able to get the page
title to overlap with the text
field (after substituting "Settings" with "Configuración"), see
http://imgur.com/f1KPdSM. I agree about your point
for a "perfect" solution, I am just not super confident that the problem
currently exists for a small number of
languages + widths configurations.

Are we 100% sure that there is no pure CSS solution that can give us what we
need? Can calc() somehow help us here, so that we
do use a dynamic calculations, but still have it in CSS only?

[dpapad, 2016-07-13 20:50:11]

https://codereview.chromium.org/2140943002/diff/80001/ui/webui/resources/cr_e...
File ui/webui/resources/cr_elements/cr_toolbar/cr_toolbar_search_field.html
(right):

https://codereview.chromium.org/2140943002/diff/80001/ui/webui/resources/cr_e...
ui/webui/resources/cr_elements/cr_toolbar/cr_toolbar_search_field.html:116:
margin-left: 18px;
Can we not merge this CSS rule and the next one by using
-webkit-margin-start: 18px;

[tsergeant, 2016-07-14 03:48:30]

Description was changed from

==========
MD WebUI: Center search field correctly inside cr-toolbar

This changes the behavior of the search field so that it no longer
expands on open in wide windows. This makes it possible to center the
search field inside the toolbar. Also tweaks the open/close animation
for the toolbar in narrow mode to be smoother.

BUG=610609
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

to

==========
MD WebUI: Center search field correctly inside cr-toolbar

This changes the behavior of the search field so that it no longer
expands on open in wide windows. This makes it possible to center the
search field inside the toolbar. Also tweaks the open/close animation
for the toolbar in narrow mode to be smoother.

BUG=610609
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation;master.tryserver.chromium.linux:closure_compilation
==========

[tsergeant, 2016-07-14 03:50:51]

You're right, there is a good solution with calc. In fact, I basically said
exactly what needs to be done in an email yesterday, I just didn't realise it.

PTAL!

https://codereview.chromium.org/2140943002/diff/80001/ui/webui/resources/cr_e...
File ui/webui/resources/cr_elements/cr_toolbar/cr_toolbar_search_field.html
(right):

https://codereview.chromium.org/2140943002/diff/80001/ui/webui/resources/cr_e...
ui/webui/resources/cr_elements/cr_toolbar/cr_toolbar_search_field.html:116:
margin-left: 18px;
On 2016/07/13 20:50:11, dpapad wrote:
> Can we not merge this CSS rule and the next one by using
> -webkit-margin-start: 18px;

-webkit-margin-start doesn't animate, and having the margin animate smooths out
the open/close animation (otherwise it will jump around by 18px at the start of
the animation).

[dpapad, 2016-07-14 16:47:28]

LGTM

https://codereview.chromium.org/2140943002/diff/100001/ui/webui/resources/cr_...
File ui/webui/resources/cr_elements/cr_toolbar/cr_toolbar.html (right):

https://codereview.chromium.org/2140943002/diff/100001/ui/webui/resources/cr_...
ui/webui/resources/cr_elements/cr_toolbar/cr_toolbar.html:20: text-overflow:
ellipsis;
Could those be moved under #leftContent instead of host?

https://codereview.chromium.org/2140943002/diff/100001/ui/webui/resources/cr_...
ui/webui/resources/cr_elements/cr_toolbar/cr_toolbar.html:52: max-width:
calc((100% - var(--cr-toolbar-field-width) - 18px) / 2);
Nice! I am glad calc() can help us here.

[tsergeant, 2016-07-15 01:35:46]

https://codereview.chromium.org/2140943002/diff/100001/ui/webui/resources/cr_...
File ui/webui/resources/cr_elements/cr_toolbar/cr_toolbar.html (right):

https://codereview.chromium.org/2140943002/diff/100001/ui/webui/resources/cr_...
ui/webui/resources/cr_elements/cr_toolbar/cr_toolbar.html:20: text-overflow:
ellipsis;
On 2016/07/14 16:47:28, dpapad wrote:
> Could those be moved under #leftContent instead of host?

Do you mean "instead of h1"?

Playing around with it, these are necessary here, but we can get rid of the
overflow:hidden on #leftContent.

[tsergeant, 2016-07-15 01:35:51]

The CQ bit was checked by tsergeant@chromium.org

[tsergeant, 2016-07-15 01:35:52]

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/2140943002/#ps120001
(title: "Remove extra overflow")

[commit-bot: I haz the power, 2016-07-15 01:36:27]

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-07-15 01:36:29]

Your CL relies on deprecated CQ feature(s):
 * Specifying master names in CQ_INCLUDE_TRYBOTS part of description without
"master." prefix is deprecated:
     tryserver.chromium.linux
   For more details, see http://crbug.com/617627.

[commit-bot: I haz the power, 2016-07-15 02:28:14]

Committed patchset #4 (id:120001)

[commit-bot: I haz the power, 2016-07-15 02:28:21]

CQ bit was unchecked.

[commit-bot: I haz the power, 2016-07-15 02:29:50]

Description was changed from

==========
MD WebUI: Center search field correctly inside cr-toolbar

This changes the behavior of the search field so that it no longer
expands on open in wide windows. This makes it possible to center the
search field inside the toolbar. Also tweaks the open/close animation
for the toolbar in narrow mode to be smoother.

BUG=610609
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation;master.tryserver.chromium.linux:closure_compilation
==========

to

==========
MD WebUI: Center search field correctly inside cr-toolbar

This changes the behavior of the search field so that it no longer
expands on open in wide windows. This makes it possible to center the
search field inside the toolbar. Also tweaks the open/close animation
for the toolbar in narrow mode to be smoother.

BUG=610609
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation;master.tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/1ce950b1dfba435c8671b75c82bd2256a0718533
Cr-Commit-Position: refs/heads/master@{#405681}
==========

[commit-bot: I haz the power, 2016-07-15 02:29:51]

Patchset 4 (id:??) landed as
https://crrev.com/1ce950b1dfba435c8671b75c82bd2256a0718533
Cr-Commit-Position: refs/heads/master@{#405681}