Scaffold for MD-Settings, inside chrome://md-settings.

## Not ready for review -- dependent on https://codereview.chromium.org/981203007

Scaffold for MD-Settings, inside chrome://md-settings.

This (Patch 6) is the add-on to issue 981203007 (Patch 5). It updates
a11y_page, adds dummy_page, adds everything to Chrome and exposes
cr-settings from chrome://md-settings.

BUG=464979

[Jeremy Klein, 2015-03-07 23:55:12]

jlklein@chromium.org changed reviewers:
+ jlklein@chromium.org

[Jeremy Klein, 2015-03-07 23:55:14]

Just a first pass. I'm sure I missed a bunch here. Big +1 to splitting this up
into much smaller pieces. Personally, this is way too much for me to grok at
once.

https://codereview.chromium.org/986843002/diff/20001/chrome/browser/resources...
File chrome/browser/resources/settings/a11y_page/a11y_page.html (right):

https://codereview.chromium.org/986843002/diff/20001/chrome/browser/resources...
chrome/browser/resources/settings/a11y_page/a11y_page.html:11:
extends="cr-settings-page" constructor="CrSettingsA11yPage">
I'm concerned about using inheritance for this since 0.8 doesn't have a clear
story for inheritance yet:

https://github.com/Polymer/polymer/blob/0.8-preview/PRIMER.md#mixins--inherit...

Is there an easy was we can switch to composition here?

https://codereview.chromium.org/986843002/diff/20001/chrome/browser/resources...
File chrome/browser/resources/settings/compiled_resources.gyp (right):

https://codereview.chromium.org/986843002/diff/20001/chrome/browser/resources...
chrome/browser/resources/settings/compiled_resources.gyp:7: 'target_name':
'cr_settings/cr_settings',
Our stuff isn't compilable at the moment so I don't really think we should add
this file yet. Did you try compiling?

https://codereview.chromium.org/986843002/diff/20001/chrome/browser/resources...
File chrome/browser/resources/settings/settings.js (right):

https://codereview.chromium.org/986843002/diff/20001/chrome/browser/resources...
chrome/browser/resources/settings/settings.js:30: this.pages_.push(new
CrSettingsA11yPage());
I really still feel like I'd rather do this declaratively. I like being able to
see the dom structure in the HTML with the pages. That would also allow you to
tie up the prefs in the html.

https://codereview.chromium.org/986843002/diff/20001/chrome/browser/resources...
File chrome/browser/resources/settings/settings_drawer/settings_drawer.js
(right):

https://codereview.chromium.org/986843002/diff/20001/chrome/browser/resources...
chrome/browser/resources/settings/settings_drawer/settings_drawer.js:31: pages:
[],
This should be initialized in the created callback. See
https://www.polymer-project.org/docs/polymer/polymer.html. Search for "For
property values that are objects or arrays".

https://codereview.chromium.org/986843002/diff/20001/chrome/browser/resources...
File chrome/browser/resources/settings/settings_main/settings_main.css (right):

https://codereview.chromium.org/986843002/diff/20001/chrome/browser/resources...
chrome/browser/resources/settings/settings_main/settings_main.css:6:
background-color: rgb(219, 222, 224);
Did these colors come from the material palette?

http://www.google.com/design/spec/style/color.html

https://codereview.chromium.org/986843002/diff/20001/chrome/browser/resources...
File chrome/browser/resources/settings/settings_main/settings_main.js (right):

https://codereview.chromium.org/986843002/diff/20001/chrome/browser/resources...
chrome/browser/resources/settings/settings_main/settings_main.js:28: pages: [],
Same comment about initialization.

https://codereview.chromium.org/986843002/diff/20001/chrome/browser/resources...
chrome/browser/resources/settings/settings_main/settings_main.js:44: * @param
{*} value The old value if the property value has changed, or the
Please use a more specific type.

https://codereview.chromium.org/986843002/diff/20001/chrome/browser/resources...
chrome/browser/resources/settings/settings_main/settings_main.js:64:
updatePages_: function() {
A lot of the logic in this file could be simplified/removed if we insert the
pages declaratively.

https://codereview.chromium.org/986843002/diff/20001/chrome/browser/resources...
File chrome/browser/resources/settings/settings_page/settings_page.js (right):

https://codereview.chromium.org/986843002/diff/20001/chrome/browser/resources...
chrome/browser/resources/settings/settings_page/settings_page.js:27: prefs:
null,
I don't think that all pages will need a prefs in the long run. This is one of
the downfalls of inheritance IMO. I think I'd prefer adding a
cr-settings-page-metadata element for the titles, icon, and help. Each of the
pages could have a metadata element, thus exposing a shared interface to the
page container. I think this would make things less tightly coupled while also
being more flexible in the transition to 0.8. WDYT?

https://codereview.chromium.org/986843002/diff/20001/chrome/browser/resources...
File chrome/browser/resources/settings/utils/utils.js (right):

https://codereview.chromium.org/986843002/diff/20001/chrome/browser/resources...
chrome/browser/resources/settings/utils/utils.js:5: var utils = {
nit: The name utils tends to lead to dumping grounds of miscellaneous functions.
Can we call this something more specific? Maybe platformInfo?

https://codereview.chromium.org/986843002/diff/20001/chrome/browser/resources...
chrome/browser/resources/settings/utils/utils.js:5: var utils = {
We should probably use a module pattern here.

[michaelpg, 2015-03-08 01:13:01]

Sorry Jeremy, this is just an FYI patch! The one that up for review is issue
981203007.

I didn't mean to upload the compiled_resources file, sry.

[Jeremy Klein, 2015-03-08 01:20:08]

On 2015/03/08 01:13:01, michaelpg wrote:
> Sorry Jeremy, this is just an FYI patch! The one that up for review is issue
> 981203007.
> 
> I didn't mean to upload the compiled_resources file, sry.

Ah, ok my bad! That makes more sense :-). Thanks!

[michaelpg, 2015-03-08 04:22:47]

https://codereview.chromium.org/986843002/diff/20001/chrome/browser/resources...
File chrome/browser/resources/settings/settings_page/settings_page.js (right):

https://codereview.chromium.org/986843002/diff/20001/chrome/browser/resources...
chrome/browser/resources/settings/settings_page/settings_page.js:27: prefs:
null,
Good stuff! This isn't out for review yet, but I wanted to respond.

On 2015/03/07 23:55:14, Jeremy Klein wrote:
> I don't think that all pages will need a prefs in the long run. This is one of
> the downfalls of inheritance IMO.

Agreed. The models will need to be included on a page-by-page basis. We could
have the individual pages include them, like how components can include
<app-globals> in Polymer examples.

> I think I'd prefer adding a
> cr-settings-page-metadata element for the titles, icon, and help. Each of the
> pages could have a metadata element, thus exposing a shared interface to the
> page container.

Putting shared properties in a metadata object provides an interface for the
container like you say, so we can use type annotations for closure, but then it
feels like we're letting closure dictate our design. Like, "pageTitle" is a core
property of what a page is, so giving a page a title via composition doesn't
seem natural. Can we look for a better compromise?

"shared interface" sounds good -- is there a way we could just do that, without
doing class-like inheritance or metadata elements? Would "@implements" help
here?

> I think this would make things less tightly coupled while also
> being more flexible in the transition to 0.8. WDYT?

I've been assuming 0.8 doesn't intend to *ship* without inheritance just because
the preview doesn't have it yet. Because it's a core part of the web component
spec and it's used throughout core-elements and paper-elements. IOW, we should
take advantage of inheritance where it makes sense -- not saying it does, here,
or necessarily will anywhere, but IMHO parity with a work-in-progress isn't
worth handicapping ourselves.

I'll write up these concerns in a design doc so we can talk about this in a more
permanent place.

[michaelpg, 2015-03-09 09:03:48]

Patchset #3 (id:40001) has been deleted

[Jeremy Klein, 2015-03-10 20:03:33]

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

[michaelpg, 2015-03-12 22:30:09]

Patchset #7 (id:140001) has been deleted