MD Settings: Minor fixes to manage languages page

MD Settings: Minor fixes to manage languages page

BUG=512479
Committed: https://crrev.com/5407c7f364da72df058a60413c02a418d4709c28
Cr-Commit-Position: refs/heads/master@{#351406}

[michaelpg, 2015-09-28 16:55:43]

michaelpg@chromium.org changed reviewers:
+ stevenjb@chromium.org

[stevenjb, 2015-09-28 23:51:54]

https://codereview.chromium.org/1370143003/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/languages_page/manage_languages_page.html
(right):

https://codereview.chromium.org/1370143003/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/languages_page/manage_languages_page.html:37:
<div class="flex">
I thought we decided to avoid 'class="flex"'?

https://codereview.chromium.org/1370143003/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/languages_page/manage_languages_page.js
(right):

https://codereview.chromium.org/1370143003/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/languages_page/manage_languages_page.js:33: *
@private {Array<{code: string, displayName: string,
!Array<!{

https://codereview.chromium.org/1370143003/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/languages_page/manage_languages_page.js:69:
canRemoveLanguage_: function(languageCode, prospectiveUILanguage) {
enabledLanguages or prospective... ?

https://codereview.chromium.org/1370143003/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/languages_page/manage_languages_page.js:87:
this.$.languages.isEnabled(this.availableLanguages_[i].code));
This is weird... if (this.availableLanguages_) then we effectively ignore the
local availableLanguages. I think the if () should be at the top level, even
though it means two for loops.

[michaelpg, 2015-09-29 00:20:32]

PTAL.

https://codereview.chromium.org/1370143003/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/languages_page/manage_languages_page.html
(right):

https://codereview.chromium.org/1370143003/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/languages_page/manage_languages_page.html:37:
<div class="flex">
On 2015/09/28 23:51:54, stevenjb wrote:
> I thought we decided to avoid 'class="flex"'?

Yes. Guess I may as well handle those while I'm here.

https://codereview.chromium.org/1370143003/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/languages_page/manage_languages_page.js
(right):

https://codereview.chromium.org/1370143003/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/languages_page/manage_languages_page.js:33: *
@private {Array<{code: string, displayName: string,
On 2015/09/28 23:51:54, stevenjb wrote:
> !Array<!{

Done.

https://codereview.chromium.org/1370143003/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/languages_page/manage_languages_page.js:69:
canRemoveLanguage_: function(languageCode, prospectiveUILanguage) {
On 2015/09/28 23:51:54, stevenjb wrote:
> enabledLanguages or prospective... ?

Done. Forgot to Closure-compile this file!

https://codereview.chromium.org/1370143003/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/languages_page/manage_languages_page.js:87:
this.$.languages.isEnabled(this.availableLanguages_[i].code));
On 2015/09/28 23:51:54, stevenjb wrote:
> This is weird... if (this.availableLanguages_) then we effectively ignore the
> local availableLanguages. I think the if () should be at the top level, even
> though it means two for loops.

Done. This should make more sense. Hope you don't mind the inverted if.

[stevenjb, 2015-09-29 16:01:57]

lgtm

[michaelpg, 2015-09-29 17:22:50]

The CQ bit was checked by michaelpg@chromium.org

[commit-bot: I haz the power, 2015-09-29 17:23:46]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1370143003/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1370143003/20001

[commit-bot: I haz the power, 2015-09-29 17:38:21]

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

[commit-bot: I haz the power, 2015-09-29 17:38:22]

Try jobs failed on following builders:
  win_chromium_compile_dbg_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)

[michaelpg, 2015-09-29 20:47:14]

The CQ bit was checked by michaelpg@chromium.org

[commit-bot: I haz the power, 2015-09-29 20:48:20]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1370143003/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1370143003/20001

[commit-bot: I haz the power, 2015-09-29 21:22:23]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-09-29 21:30:12]

Patchset 2 (id:??) landed as
https://crrev.com/5407c7f364da72df058a60413c02a418d4709c28
Cr-Commit-Position: refs/heads/master@{#351406}