MD Settings: Enable to search "Add languages" list.

MD Settings: Enable to search "Add languages" list.

BUG=661342
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

[SangGyeong.Jo, 2016-11-23 02:03:25]

Description was changed from

==========
MD Settings: Enable to search "Add languages" list.

BUG=661342
==========

to

==========
MD Settings: Enable to search "Add languages" list.

BUG=661342
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[SangGyeong.Jo, 2016-11-23 02:04:36]

Description was changed from

==========
MD Settings: Enable to search "Add languages" list.

BUG=661342
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
MD Settings: Enable to search "Add languages" list.

BUG=661342
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[SangGyeong.Jo, 2016-11-23 02:04:36]

sanggyeong.jo@gmail.com changed reviewers:
+ dbeam@chromium.org

[SangGyeong.Jo, 2016-11-23 02:05:07]

Dan Beam@
PTAL

[Dan Beam, 2016-11-23 02:23:04]

dbeam@chromium.org changed reviewers:
+ michaelpg@chromium.org
- dbeam@chromium.org

[SangGyeong.Jo, 2016-11-24 05:00:13]

On 2016/11/23 02:05:07, SangGyeong.Jo wrote:
> Dan Beam@
> PTAL

@michaelpg
PTAL

[michaelpg, 2016-11-29 01:59:38]

On 2016/11/24 05:00:13, SangGyeong.Jo wrote:
> On 2016/11/23 02:05:07, SangGyeong.Jo wrote:
> > Dan Beam@
> > PTAL
> 
> @michaelpg
> PTAL

Please look through the JavaScript section of
http://www.chromium.org/developers/web-development-style-guide and re-submit
your CL. In particular, check out the Google JavaScript Style Guide referenced
there.

[SangGyeong.Jo, 2016-11-30 02:07:48]

On 2016/11/29 01:59:38, michaelpg wrote:
> On 2016/11/24 05:00:13, SangGyeong.Jo wrote:
> > On 2016/11/23 02:05:07, SangGyeong.Jo wrote:
> > > Dan Beam@
> > > PTAL
> > 
> > @michaelpg
> > PTAL
> 
> Please look through the JavaScript section of
> http://www.chromium.org/developers/web-development-style-guide and re-submit
> your CL. In particular, check out the Google JavaScript Style Guide referenced
> there.

I read it, and fix code to follow guide.
Please take a look.
Thank you

[michaelpg, 2016-12-01 20:40:13]

I think we should probably use <settings-subpage-search>, with some tweaks for a
dialog. I'll get back to you later today with more thoughts.

https://codereview.chromium.org/2519853005/diff/20001/chrome/browser/resource...
File chrome/browser/resources/settings/languages_page/add_languages_dialog.html
(right):

https://codereview.chromium.org/2519853005/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/languages_page/add_languages_dialog.html:5:
<link rel="import"
href="chrome://resources/polymer/v1_0/paper-input/paper-input-container.html">
alphabetize (move below paper-checkbox)

https://codereview.chromium.org/2519853005/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/languages_page/add_languages_dialog.html:37:
<iron-list class="ripple-padding" scroll-target="[[$$('.ripple-padding')]]"
keep to 80-column lines (e.g., break before scroll-target)

[michaelpg, 2016-12-02 03:56:41]

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

[michaelpg, 2016-12-02 03:56:41]

dpapad, could you take over for me with respect to making a search box inside a
dialog?

https://codereview.chromium.org/2519853005/diff/20001/chrome/browser/resource...
File chrome/browser/resources/settings/languages_page/add_languages_dialog.html
(right):

https://codereview.chromium.org/2519853005/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/languages_page/add_languages_dialog.html:34:
<paper-input-container>
+dpapad to figure out the best way to add a filter to a dialog, a la
settings-subpage-search.

In particular, how to keep the padding correct: changing the scroll-target (line
37) shrinks the area that scrolls, so the scroller looks like it's in the wrong
position (the current scrollbar position was explicitly requested).

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

https://codereview.chromium.org/2519853005/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/languages_page/add_languages_dialog.js:72:
Boolean(~language.displayName.indexOf(filterValue));
the ~ trick is a bit too clever; much more readable to just do:

  language.displayName.includes(filterValue)

https://codereview.chromium.org/2519853005/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/languages_page/add_languages_dialog.js:72:
Boolean(~language.displayName.indexOf(filterValue));
This comparison should be case-insensitive (e.g. convert everything to
lowercase), so a lowercase search can return capitalized languages and vice
versa.

[SangGyeong.Jo, 2016-12-05 18:29:44]

I fixed the position of scrollbar and it appears at original place.
But I think this is not a good idea, because this makes right-padding to 0.
I'm trying to find better way.

And I changed comparison to case-insensitive.

Please take a look.

[dpapad, 2017-02-28 01:23:22]

@SangGyeong: Are you still interested in landing this CL (sorry for the long
delay in responding). If yes, can you rebase your CL and resolve some minor
conflicts that exist? Thanks

[SangGyeong.Jo, 2017-02-28 02:41:07]

On 2017/02/28 01:23:22, dpapad wrote:
> @SangGyeong: Are you still interested in landing this CL (sorry for the long
> delay in responding). If yes, can you rebase your CL and resolve some minor
> conflicts that exist? Thanks

@dpapad: Ok, I'll work on that.

[SangGyeong.Jo, 2017-03-19 17:41:49]

@dpapad: I rebased by CL and resolved minor conflicts. Please take a look. Thank
you.

[dpapad, 2017-03-20 18:01:31]

On 2017/03/19 at 17:41:49, SangGyeong.Jo wrote:
> @dpapad: I rebased by CL and resolved minor conflicts. Please take a look.
Thank you.

Thanks for updating. In the meantime I had started an alternative
implementation, with also has a slightly different UI, see
https://bugs.chromium.org/p/chromium/issues/detail?id=661342#c14. Depending on
what our UX says, we might follow the alternative approach. You can follow the
bug for updates.
Thanks

[dpapad, 2017-03-24 21:49:16]

dpapad@chromium.org changed reviewers:
- dpapad@chromium.org

[michaelpg, 2017-05-09 20:56:23]

Closing CL as issue is resolved.