MD Settings: Polish for Languages main page

MD Settings: Polish for Languages main page

Updates to the mocks:
* Fix cr-shared-menu width to 320px
* "Move to the top" shortcut for moving languages all the way up
* Expanded style for cr-shared-menu with separator
* Change selected input method visual state to say "Enabled"
* Change spellcheck toggle buttons to checkboxes

Also:
* Close cr-shared-menu after changing a checkbox item
* Render Add Languages dialog's iron-list properly

screenshots: https://crbug.com/630982#c13

BUG=630982
R=dbeam@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/e11da429912fdbd07dfda4ebafec78fcc2163c58
Cr-Commit-Position: refs/heads/master@{#421695}

[michaelpg, 2016-09-22 22:02:39]

Description was changed from

==========
MD Settings: Polish for Languages main page

BUG=630982
R=dbeam@chromium.org
==========

to

==========
MD Settings: Polish for Languages main page

BUG=630982
R=dbeam@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[michaelpg, 2016-09-22 22:15:31]

Description was changed from

==========
MD Settings: Polish for Languages main page

BUG=630982
R=dbeam@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
MD Settings: Polish for Languages main page

Updates to the mocks:
* "Move to the top" shortcut for moving languages all the way up
* Change selected input method visual state to say "Enabled"
* Fix cr-shared-menu width to 320px
* Change spellcheck toggle buttons to checkboxes

Also:
* Close cr-shared-menu after changing a checkbox item
* Render Add Languages dialog's iron-list properly



BUG=630982
R=dbeam@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[michaelpg, 2016-09-22 22:18:29]

PTAL.

[michaelpg, 2016-09-22 22:19:51]

Description was changed from

==========
MD Settings: Polish for Languages main page

Updates to the mocks:
* "Move to the top" shortcut for moving languages all the way up
* Change selected input method visual state to say "Enabled"
* Fix cr-shared-menu width to 320px
* Change spellcheck toggle buttons to checkboxes

Also:
* Close cr-shared-menu after changing a checkbox item
* Render Add Languages dialog's iron-list properly



BUG=630982
R=dbeam@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
MD Settings: Polish for Languages main page

Updates to the mocks:
* "Move to the top" shortcut for moving languages all the way up
* Change selected input method visual state to say "Enabled"
* Fix cr-shared-menu width to 320px
* Change spellcheck toggle buttons to checkboxes

Also:
* Close cr-shared-menu after changing a checkbox item
* Render Add Languages dialog's iron-list properly

screenshots: https://crbug.com/630982#c13



BUG=630982
R=dbeam@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[Dan Beam, 2016-09-22 22:27:09]

the line heights of all menu items (whether they have a checkbox or not) look
the same in the mocks

also, there's a separator

when are those changes coming?

afaict from your screenshots that's not done yet

[michaelpg, 2016-09-23 01:32:07]

Description was changed from

==========
MD Settings: Polish for Languages main page

Updates to the mocks:
* "Move to the top" shortcut for moving languages all the way up
* Change selected input method visual state to say "Enabled"
* Fix cr-shared-menu width to 320px
* Change spellcheck toggle buttons to checkboxes

Also:
* Close cr-shared-menu after changing a checkbox item
* Render Add Languages dialog's iron-list properly

screenshots: https://crbug.com/630982#c13



BUG=630982
R=dbeam@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
MD Settings: Polish for Languages main page

Updates to the mocks:
* Fix cr-shared-menu width to 320px
* "Move to the top" shortcut for moving languages all the way up
* Expanded style for cr-shared-menu with separator
* Change selected input method visual state to say "Enabled"
* Change spellcheck toggle buttons to checkboxes

Also:
* Close cr-shared-menu after changing a checkbox item
* Render Add Languages dialog's iron-list properly

screenshots: https://crbug.com/630982#c13



BUG=630982
R=dbeam@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[michaelpg, 2016-09-23 01:34:18]

On 2016/09/22 22:27:09, Dan Beam wrote:
> the line heights of all menu items (whether they have a checkbox or not) look
> the same in the mocks
> 
> also, there's a separator
> 
> when are those changes coming?
> 
> afaict from your screenshots that's not done yet

Thanks for pointing those out. I've added those changes to the CL.

[Dan Beam, 2016-09-23 23:25:42]

https://codereview.chromium.org/2359353002/diff/80001/chrome/browser/resource...
File chrome/browser/resources/settings/languages_page/add_languages_dialog.js
(right):

https://codereview.chromium.org/2359353002/diff/80001/chrome/browser/resource...
chrome/browser/resources/settings/languages_page/add_languages_dialog.js:39:
setTimeout(list.fire.bind(list, 'iron-resize'));
setTimeout(function() {
  this.$$('iron-list').fire('iron-resize');
}.bind(this));

is 1 line longer but likely much easier to read

https://codereview.chromium.org/2359353002/diff/80001/chrome/browser/resource...
File chrome/browser/resources/settings/languages_page/languages_page.html
(right):

https://codereview.chromium.org/2359353002/diff/80001/chrome/browser/resource...
chrome/browser/resources/settings/languages_page/languages_page.html:228:
hidden="[[isFirstOrSecondLanguage_(
if we're going to show only one of "Move up" vs "Move to top" for the second
item, bettes@ and I vote for "Move to top"

https://codereview.chromium.org/2359353002/diff/80001/chrome/test/data/webui/...
File chrome/test/data/webui/settings/languages_page_browsertest.js (right):

https://codereview.chromium.org/2359353002/diff/80001/chrome/test/data/webui/...
chrome/test/data/webui/settings/languages_page_browsertest.js:279: });
do any of these test 'move to top'?

https://codereview.chromium.org/2359353002/diff/80001/ui/webui/resources/cr_e...
File ui/webui/resources/cr_elements/cr_shared_menu/cr_shared_menu.html (right):

https://codereview.chromium.org/2359353002/diff/80001/ui/webui/resources/cr_e...
ui/webui/resources/cr_elements/cr_shared_menu/cr_shared_menu.html:22:
@apply(--cr-shared-menu);
can you just use a var() instead of an @apply() if you're only changing 1 value?

[Dan Beam, 2016-09-23 23:28:22]

lgtm w/nits (sorry, read your other test harder, see you're testing
moveToFront() there)

so just the UI tweak (which we could do optionally later if you prefer, and the
nit about var() vs mixin for only 1 prop being affected)

thanks for tweaking the line-heights, it looks much better in your followup
screenshots

[michaelpg, 2016-09-28 23:24:30]

https://codereview.chromium.org/2359353002/diff/80001/chrome/browser/resource...
File chrome/browser/resources/settings/languages_page/add_languages_dialog.js
(right):

https://codereview.chromium.org/2359353002/diff/80001/chrome/browser/resource...
chrome/browser/resources/settings/languages_page/add_languages_dialog.js:39:
setTimeout(list.fire.bind(list, 'iron-resize'));
On 2016/09/23 23:25:42, Dan Beam wrote:
> setTimeout(function() {
>   this.$$('iron-list').fire('iron-resize');
> }.bind(this));
> 
> is 1 line longer but likely much easier to read

Done.

https://codereview.chromium.org/2359353002/diff/80001/chrome/browser/resource...
File chrome/browser/resources/settings/languages_page/languages_page.html
(right):

https://codereview.chromium.org/2359353002/diff/80001/chrome/browser/resource...
chrome/browser/resources/settings/languages_page/languages_page.html:228:
hidden="[[isFirstOrSecondLanguage_(
On 2016/09/23 23:25:42, Dan Beam wrote:
> if we're going to show only one of "Move up" vs "Move to top" for the second
> item, bettes@ and I vote for "Move to top"

Done.

https://codereview.chromium.org/2359353002/diff/80001/ui/webui/resources/cr_e...
File ui/webui/resources/cr_elements/cr_shared_menu/cr_shared_menu.html (right):

https://codereview.chromium.org/2359353002/diff/80001/ui/webui/resources/cr_e...
ui/webui/resources/cr_elements/cr_shared_menu/cr_shared_menu.html:22:
@apply(--cr-shared-menu);
On 2016/09/23 23:25:42, Dan Beam wrote:
> can you just use a var() instead of an @apply() if you're only changing 1
value?

Done.

[michaelpg, 2016-09-28 23:26:19]

The CQ bit was checked by michaelpg@chromium.org

[michaelpg, 2016-09-28 23:26:20]

The patchset sent to the CQ was uploaded after l-g-t-m from dbeam@chromium.org
Link to the patchset: https://codereview.chromium.org/2359353002/#ps100001
(title: "dbeam feedback")

[commit-bot: I haz the power, 2016-09-28 23:26: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-09-29 00:37:26]

Description was changed from

==========
MD Settings: Polish for Languages main page

Updates to the mocks:
* Fix cr-shared-menu width to 320px
* "Move to the top" shortcut for moving languages all the way up
* Expanded style for cr-shared-menu with separator
* Change selected input method visual state to say "Enabled"
* Change spellcheck toggle buttons to checkboxes

Also:
* Close cr-shared-menu after changing a checkbox item
* Render Add Languages dialog's iron-list properly

screenshots: https://crbug.com/630982#c13



BUG=630982
R=dbeam@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
MD Settings: Polish for Languages main page

Updates to the mocks:
* Fix cr-shared-menu width to 320px
* "Move to the top" shortcut for moving languages all the way up
* Expanded style for cr-shared-menu with separator
* Change selected input method visual state to say "Enabled"
* Change spellcheck toggle buttons to checkboxes

Also:
* Close cr-shared-menu after changing a checkbox item
* Render Add Languages dialog's iron-list properly

screenshots: https://crbug.com/630982#c13



BUG=630982
R=dbeam@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[commit-bot: I haz the power, 2016-09-29 00:37:29]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-09-29 00:48:51]

Description was changed from

==========
MD Settings: Polish for Languages main page

Updates to the mocks:
* Fix cr-shared-menu width to 320px
* "Move to the top" shortcut for moving languages all the way up
* Expanded style for cr-shared-menu with separator
* Change selected input method visual state to say "Enabled"
* Change spellcheck toggle buttons to checkboxes

Also:
* Close cr-shared-menu after changing a checkbox item
* Render Add Languages dialog's iron-list properly

screenshots: https://crbug.com/630982#c13



BUG=630982
R=dbeam@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
MD Settings: Polish for Languages main page

Updates to the mocks:
* Fix cr-shared-menu width to 320px
* "Move to the top" shortcut for moving languages all the way up
* Expanded style for cr-shared-menu with separator
* Change selected input method visual state to say "Enabled"
* Change spellcheck toggle buttons to checkboxes

Also:
* Close cr-shared-menu after changing a checkbox item
* Render Add Languages dialog's iron-list properly

screenshots: https://crbug.com/630982#c13

BUG=630982
R=dbeam@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/e11da429912fdbd07dfda4ebafec78fcc2163c58
Cr-Commit-Position: refs/heads/master@{#421695}
==========

[commit-bot: I haz the power, 2016-09-29 00:48:53]

Patchset 6 (id:??) landed as
https://crrev.com/e11da429912fdbd07dfda4ebafec78fcc2163c58
Cr-Commit-Position: refs/heads/master@{#421695}