Scaffold for MD-Settings.

chrome/browser/resources/settings/settings_main/settings_main.js

Index: chrome/browser/resources/settings/settings_main/settings_main.js
diff --git a/chrome/browser/resources/settings/settings_main/settings_main.js b/chrome/browser/resources/settings/settings_main/settings_main.js
new file mode 100644
index 0000000000000000000000000000000000000000..fe52f13b7284f21602c6c57317193ffecaf47588
--- /dev/null
+++ b/chrome/browser/resources/settings/settings_main/settings_main.js
@@ -0,0 +1,146 @@
+// Copyright 2015 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+/**
+ * @fileoverview
+ * 'cr-settings-main' displays the selected settings page.
+ *
+ * Example:
+ *
+ *     <cr-settings-main pages="{{pages}}" currentPageIndex="{{index}}">
+ *     </cr-settings-main>
+ *
+ * See cr-settings-drawer for example of use in 'core-panel'.
+ *
+ * @group Chrome Settings Elements
+ * @element cr-settings-main
+ */
+Polymer('cr-settings-main', {
+  publish: {
+    /**
+     * Pages that may be shown.
+     *
+     * @attribute pages
+     * @type Array<!Obect>

[Oren Blasberg, 2015/03/09 21:34:00]

!Object

[michaelpg, 2015/03/12 01:41:40]

Done.

+     * @default []
+     */
+    pages: [],
+
+    /**
+     * Index of the currently selected page.
+     *
+     * @attribute selectedIndex
+     * @type number
+     * default 0
+     */
+    currentPageIndex: 0,
+  },
+
+  /** @override */
+  ready: function() {
+    this.processPages_();
+  },
+
+  /**
+   * Polymer pages changed method. Ensures the pages container contains the
+   * pages in the right order.
+   *
+   * @param {*} value The old value if the property value has changed, or the

[Oren Blasberg, 2015/03/09 21:34:00]

In the first case, what type would the old value have?

[michaelpg, 2015/03/12 01:41:40]

Removed.

+   *     array of splices if the existing array has been changed.
+   */
+  pagesChanged: function(value) {
+    // If value is an array of plain objects, it represents splices from an
+    // ArrayObserver.
+    if (value instanceof Array && value.length &&
+        Object.prototype.isPrototypeOf(value[0])) {
+      this.applyPageSplices_(value);
+    } else {
+      this.updatePages_(this.pages);

[Oren Blasberg, 2015/03/09 21:34:00]

updatePages_ doesn't take args -- can you remove the arg from this call?

[michaelpg, 2015/03/12 01:41:40]

Removed.

+    }
+  },
+
+  /**
+   * Adds each page to the page list and prepare the core-animated-pages.
+   *
+   * @private
+   */
+  processPages_: function() {

[Kyle Horimoto, 2015/03/09 20:59:32]

I don't understand why this is necessary. We declare the pages as children of
the core-animated-pages anyway, so why do we need this extra processing? Also,
why would we ever have a <template> here? We won't ever be using a "repeat"
template here...is there another reason why this would be necessary?

[michaelpg, 2015/03/09 22:24:11]

Some annoying rule about containers for pages not participating in the
transition.
https://www.polymer-project.org/docs/elements/core-elements.html#core-animate...
Search for "this does not work".
Oops, in a prior version I had <template if="{{isChromeOs()}}"> surrounding
a11y-page, but there are no <template>s here.

[Kyle Horimoto, 2015/03/09 22:32:03]

Just use the "cross-fade-all" transition instead of "cross-fade", and I think
all this is unnecessary.

[michaelpg, 2015/03/12 01:41:40]

Done-ish (cross-fade-all is too janky)

+    // Generate the list of pages from the DOM.
+    var pages = [];
+    var children = [].slice.call(this.$.pages.children);

[Jeremy Klein, 2015/03/09 21:17:14]

Why not just use this.$.pages.items?

[Oren Blasberg, 2015/03/09 21:34:00]

How come we need to call slice in this unusual way (on an empty array and using
'call')?

[michaelpg, 2015/03/09 22:24:11]

Oops, this was from when I was updating the pages in place (children returns a
live HTMLCollection so this would convert it to a static array).

(Using the empty array is a sort of shorthand for Array.prototype.)

+    for (var i = 0; i < children.length; i++) {
+      // Ignore <template>s.
+      if (children[i].tagName != 'TEMPLATE')

[Jeremy Klein, 2015/03/09 21:17:14]

You can ignore this with an excludedLocalNames attribute on the
core-animated-pages, but can you also explain why you need this?

[michaelpg, 2015/03/09 22:24:11]

See response to khorimito above.

+        pages.push(children[i]);
+    }
+    // Set the list of pages so each page is placed in a <section> within

[Kyle Horimoto, 2015/03/09 20:59:32]

Why a <section>?

[Jeremy Klein, 2015/03/09 21:17:14]

Why does it need to be inside a section?

[michaelpg, 2015/03/09 22:24:11]

Requirement of core-animated-pages, provides a container so the page can be
animated in/out. See response to khorimoto above.

[Jeremy Klein, 2015/03/09 22:35:05]

Section isn't a requirement of core-animated-pages. There's just a restriction
that the container itself can't animate. Did you try out cross-fade-all on the
core-animated-pages for now? The only practical implication of this restriction
is that we won't put transitions on the page element itself, but rather on
individual elements within it. I feel like we should just use cross-fade-all for
now and once we have more specs for specific transitions, we can transition
whatever we want inside the pages.

I just don't really like programmatically wrapping the pages in sections because
then the sections are the core-animation-page's items rather than the pages and
selection binding becomes a bit more convoluted. I think we can avoid this.

[michaelpg, 2015/03/12 01:41:40]

Done. Thanks for the tips Jeremy & Kyle!

+    // core-animated-pages.
+    this.pages = pages;
+  },
+
+  /**
+   * Sets the list of pages shown in the 'core-animated-pages'. Rebuilds the DOM
+   * for 'core-animated-pages', so this should be called sparingly.
+   *
+   * @private
+   */
+  updatePages_: function() {
+    this.$.pages.innerHTML = '';
+    if (!this.pages.length)
+      return;
+
+    if (this.currentPageIndex >= this.pages.length)
+      this.currentPageIndex = 0;
+
+    for (var i = 0; i < this.pages.length; i++)
+      this.$.pages.appendChild(this.createSection_(this.pages[i]));
+  },
+
+  /**
+   * Applies the splices for the list of pages to the sections in the DOM.
+   *
+   * TODO(michaelpg): core-animated-pages throws when all pages are removed.
+   *
+   * @param {!Array<!Object>} splices The changes made to 'pages'.
+   * @private
+   */
+  applyPageSplices_: function(splices) {

[Jeremy Klein, 2015/03/09 21:17:14]

I don't really understand why any of this stuff is necessary. My vision for
keeping the pages attribute updates was basically the following:

Initialize it to this.$.pages.items.
Add a MutationObserver with childList:true on this.$.pages.
In that callback, set this.pages to this.$.pages.items.

I think this will be a much easier was to keep track of this stuff. The logic in
here right now is pretty hard to follow and seems potentially fragile. Am I
missing some reason all of this is needed?

[michaelpg, 2015/03/09 22:24:11]

Hmm, I'll take a look at MutationObserver. I agree this is fragile and I don't
like it.

[michaelpg, 2015/03/12 01:41:40]

Done.

+    var numRemoved = 0;
+    splices.forEach(function(splice) {
+      // Remove the sections to be removed before inserting new children.
+      for (var i = 0; i < splice.removed.length; i++)
+        this.$.pages.removeChild(splice.removed[i].parentNode);
+      numRemoved += splice.removed.length;
+
+      if (!splice.addedCount)
+        return;
+
+      // Find the node to insert before, unless splice.index indicates the end
+      // of the array.
+      var sectionAfter = null;
+      if (splice.index < this.$.pages.children.length)
+        sectionAfter = this.$.pages.children[splice.index];
+
+      for (var i = 0; i < splice.addedCount; i++) {
+        var section = this.createSection_(this.pages[splice.index + i]);
+        this.$.pages.insertBefore(section, sectionAfter);
+      }
+    }.bind(this));
+
+    if (this.currentPageIndex >= this.pages.length)
+      this.currentPageIndex = 0;
+  },
+
+  /** Creates a section for 'core-animated-pages' for the given element.

[Kyle Horimoto, 2015/03/09 20:59:32]

nit: Next line.

+   *
+   * @param {HTMLElement} element Element to add to 'core-animated-pages'.
+   * @private
+   */
+  createSection_: function(element) {
+    element.setAttribute('cross-fade', '');

[Kyle Horimoto, 2015/03/09 20:59:32]

What's this doing?

[Dan Beam, 2015/03/09 21:23:30]

before: <element>
after: <element cross-fade>

[Kyle Horimoto, 2015/03/09 21:28:56]

Well yeah - I know that, but I meant to ask why it was necessary and what it was
accomplishing.

[michaelpg, 2015/03/09 22:24:11]

It tells core-animated-pages that this page should be included in the cross-fade
transition. We can set this from HTML though.

[Jeremy Klein, 2015/03/09 22:35:05]

See note above about cross-fade-all.

[michaelpg, 2015/03/12 01:41:40]

Done.

+    var section = document.createElement('section');
+    section.appendChild(element);
+    return section;
+  },
+});