MD Settings: add empty list message for spell-check edit page.

chrome/browser/resources/settings/languages_page/edit_dictionary_page.js

Index: chrome/browser/resources/settings/languages_page/edit_dictionary_page.js
diff --git a/chrome/browser/resources/settings/languages_page/edit_dictionary_page.js b/chrome/browser/resources/settings/languages_page/edit_dictionary_page.js
index 421e78056ef2aadfda373c505ada3e9ee8c781a6..35813cb04e1a5f90dc4acfa30ca5ec6864dd7b03 100644
--- a/chrome/browser/resources/settings/languages_page/edit_dictionary_page.js
+++ b/chrome/browser/resources/settings/languages_page/edit_dictionary_page.js
@@ -20,14 +20,21 @@ Polymer({
     },
   },
 
+  /** @type {LanguageSettingsPrivate} */
+  languageSettingsPrivate: null,
+
   ready: function() {
-    chrome.languageSettingsPrivate.getSpellcheckWords(function(words) {

[scottchen, 2017/02/15 19:34:45]

This change is needed so that the languageSettingsPrivate can be spoofed during
test.

+    this.languageSettingsPrivate =
+        settings.languageSettingsPrivateApiForTest ||
+        /** @type {!LanguageSettingsPrivate} */(chrome.languageSettingsPrivate);
+
+    this.languageSettingsPrivate.getSpellcheckWords(function(words) {
       this.words_ = words;
     }.bind(this));
 
     // Updates are applied locally so they appear immediately, but we should
     // listen for changes in case they come from elsewhere.
-    chrome.languageSettingsPrivate.onCustomDictionaryChanged.addListener(
+    this.languageSettingsPrivate.onCustomDictionaryChanged.addListener(
         this.onCustomDictionaryChanged_.bind(this));
 
     // Add a key handler for the paper-input.
@@ -85,7 +92,7 @@ Polymer({
    * @param {!{model: !{item: string}}} e
    */
   onRemoveWordTap_: function(e) {
-    chrome.languageSettingsPrivate.removeSpellcheckWord(e.model.item);
+    this.languageSettingsPrivate.removeSpellcheckWord(e.model.item);
     this.arrayDelete('words_', e.model.item);
   },
 
@@ -102,12 +109,24 @@ Polymer({
 
     var index = this.words_.indexOf(word);

[dpapad, 2017/02/15 20:36:37]

Unrelated to your fix: I hope real users don't have too many words. Otherwise
linear search is not the best choice here.

[scottchen, 2017/02/22 21:42:25]

Acknowledged.

     if (index == -1) {
-      chrome.languageSettingsPrivate.addSpellcheckWord(word);
+      this.languageSettingsPrivate.addSpellcheckWord(word);
       this.push('words_', word);
+      index = this.words_.length - 1;

[scottchen, 2017/02/15 19:34:46]

This was not scrolling to the bottom correctly

     }
 
     // Scroll to the word (usually the bottom, or to the index if the word
     // is already present).
-    this.$.list.scrollToIndex(index);
+    this.async(function(){

[scottchen, 2017/02/15 19:34:46]

With the list being in a dom-if now, this would need to be async to give it time
to show up in the dom.

+      this.root.querySelector('#list').scrollToIndex(index);
+    });
   },
+
+  /**
+   * Checks if any words exists in the dictionary.
+   * @private
+   * @return {!boolea}

[dpapad, 2017/02/15 20:36:37]

Typo (should cause compilation error).

[scottchen, 2017/02/22 21:42:25]

Done.

+   */
+  hasWords_: function() {
+    return !!(this.words_ && this.words_.length);

[dpapad, 2017/02/15 20:36:37]

words_ is initialized to the empty array. Why do we need to check if it exists?

[scottchen, 2017/02/22 21:42:25]

I was thinking just in case languageSettingsPrivate.getSpellcheckWords(cb) calls
cb with null or undefined for whatever reason (or in the future). Or do you
think its not necessary to pre-error-prevent?

[dpapad, 2017/02/22 21:58:52]

I don't think this is necessary to be defensive in that way. The JSDoc at [1],
should be reflecting the implementation, and therefore no null/undefined should
be returned. We are defensive by trusting the JSDoc and the JSCompiler to catch
errors.

[1]
https://cs.chromium.org/chromium/src/third_party/closure_compiler/externs/lan...

[dpapad, 2017/02/23 00:32:03]

Ping ^. Can we remove the check for this.words_? Then you can probably get rid
of hasWords_() helper completely and directly use words_.length in the binding.

[scottchen, 2017/02/23 01:00:17]

It appears that I cannot to [[!!boolean]] in template so I think I still need a
middleman function. Removed the null check.

+  }
 });