[MD Extensions] Add a details subpage, move to one list

[MD Extensions] Add a details subpage, move to one list

Kick the dust off the MD extensions implementation now that we have
updated mocks.
* Add a "details" subpage that shows the details for a given extension.
  Note that this isn't complete - there are quite a few TODOs left.
* Move to a single list view instead of three lists for extensions,
  apps, and websites.
* Combine the apps and websites sections.

BUG=529395
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/da2ca5ac93171a4ab1352063f5103eae84e5f8ce
Cr-Commit-Position: refs/heads/master@{#391408}

[Devlin, 2016-04-23 01:42:27]

Description was changed from

==========
[MD Extensions]
BUG=
==========

to

==========
[MD Extensions] Add a details subpage, move to one list

Kick the dust off the MD extensions implementation now that we have
updated mocks.
* Add a "details" subpage that shows the details for a given extension.
  Note that this isn't complete - there are quite a few TODOs left.
* Move to a single list view instead of three lists for extensions,
  apps, and websites.
* Combine the apps and websites sections.

BUG=529395
==========

[Devlin, 2016-04-23 01:49:10]

Patchset #1 (id:1) has been deleted

[Devlin, 2016-04-23 01:49:16]

Patchset #1 (id:20001) has been deleted

[Devlin, 2016-04-23 01:50:09]

rdevlin.cronin@chromium.org changed reviewers:
+ dbeam@chromium.org

[Devlin, 2016-04-23 01:50:11]

Hey, we have new mocks!  Let's get crackin'.

+michaelpg FYI.

https://codereview.chromium.org/1913353002/diff/40001/chrome/browser/resource...
File chrome/browser/resources/md_extensions/compiled_resources.gyp (right):

https://codereview.chromium.org/1913353002/diff/40001/chrome/browser/resource...
chrome/browser/resources/md_extensions/compiled_resources.gyp:1: # Copyright
2015 The Chromium Authors. All rights reserved.
I heard some ruckus around moving to compiled_resources2.gyp - should this be
migrated as well?

https://codereview.chromium.org/1913353002/diff/40001/chrome/browser/resource...
File chrome/browser/resources/md_extensions/detail_view.html (right):

https://codereview.chromium.org/1913353002/diff/40001/chrome/browser/resource...
chrome/browser/resources/md_extensions/detail_view.html:13: #main {
Back when I was working on this before (~3 months ago), the norm was to have css
in separate files.  I've since seen some combined html/css like this.  Is there
a preference?

https://codereview.chromium.org/1913353002/diff/40001/chrome/browser/resource...
chrome/browser/resources/md_extensions/detail_view.html:81: </paper-checkbox>
Note: more options to come, and I still need to clean up the extensions-item
template.  I didn't want to make this CL larger than it needed to be.

https://codereview.chromium.org/1913353002/diff/40001/chrome/browser/resource...
File chrome/browser/resources/md_extensions/detail_view.js (right):

https://codereview.chromium.org/1913353002/diff/40001/chrome/browser/resource...
chrome/browser/resources/md_extensions/detail_view.js:49:
console.warn('Closing');
Whoops - will remove.

https://codereview.chromium.org/1913353002/diff/40001/chrome/browser/resource...
File chrome/browser/resources/md_extensions/item_list.js (right):

https://codereview.chromium.org/1913353002/diff/40001/chrome/browser/resource...
chrome/browser/resources/md_extensions/item_list.js:57: this.sharedElements = {
hero: e.detail.element };
This could be more easily avoided if there was an iron-list "getElementForItem"
function, but I didn't see one when I was poking around...thoughts?

https://codereview.chromium.org/1913353002/diff/40001/chrome/browser/resource...
File chrome/browser/resources/md_extensions/manager.html (left):

https://codereview.chromium.org/1913353002/diff/40001/chrome/browser/resource...
chrome/browser/resources/md_extensions/manager.html:16:
hide-extensions-button="[[computeListHidden_(extensions, extensions.splices)]]"
With separate pages for these, I don't think it makes sense to hide the buttons
anymore.  I've asked bettes for confirmation, but this is my hunch.

https://codereview.chromium.org/1913353002/diff/40001/chrome/browser/resource...
File chrome/browser/resources/md_extensions/manager.html (right):

https://codereview.chromium.org/1913353002/diff/40001/chrome/browser/resource...
chrome/browser/resources/md_extensions/manager.html:20: <neon-animated-pages
id="pages" selected="0">
I talked with michaelpg@ about whether or not to use settings-animated-pages
here, but we figured it was best not to (given that settings-animated-pages is
very heavily linked to settings-router, and, in turn, chrome://settings).  His
recommendation was to go with this.

https://codereview.chromium.org/1913353002/diff/40001/chrome/test/data/webui/...
File chrome/test/data/webui/extensions/extension_manager_test.js (left):

https://codereview.chromium.org/1913353002/diff/40001/chrome/test/data/webui/...
chrome/test/data/webui/extensions/extension_manager_test.js:30:
test(assert(TestNames.SplitSections), function() {
Relevant pieces of this test have moved to the ShowItems test.

https://codereview.chromium.org/1913353002/diff/40001/chrome/test/data/webui/...
chrome/test/data/webui/extensions/extension_manager_test.js:128:
test(assert(TestNames.AppSectionVisibility), function() {
We can now combine these into one test, since there's only one list for all
items.

[Dan Beam, 2016-04-26 01:32:13]

https://codereview.chromium.org/1913353002/diff/60001/chrome/browser/resource...
File chrome/browser/resources/md_extensions/compiled_resources.gyp (right):

https://codereview.chromium.org/1913353002/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_extensions/compiled_resources.gyp:22:
'detail_view.js',
dare you to switch to v2 GYP ;)

https://codereview.chromium.org/1913353002/diff/60001/chrome/browser/resource...
File chrome/browser/resources/md_extensions/detail_view.html (right):

https://codereview.chromium.org/1913353002/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_extensions/detail_view.html:20: color: #757575;
where are you getting these color values from?  are there --paper or --google
equivalents?

https://codereview.chromium.org/1913353002/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_extensions/detail_view.html:24: padding: 8px 12px 0
12px;
nit: padding: 8px 12px 0;

https://codereview.chromium.org/1913353002/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_extensions/detail_view.html:31: padding: 12px 20px
12px 20px;
nit: padding: 12px 20px;

https://codereview.chromium.org/1913353002/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_extensions/detail_view.html:58: <div
class="section-title">[[i18n('itemDescriptionLabel')]]</div>
or you can use $i18n{} ;)

https://codereview.chromium.org/1913353002/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_extensions/detail_view.html:76: <template
is="dom-if" if="[[data.incognitoAccess.isEnabled]]">
don't we want to pu this dom-if around the whole section?

https://codereview.chromium.org/1913353002/diff/60001/chrome/browser/resource...
File chrome/browser/resources/md_extensions/detail_view.js (right):

https://codereview.chromium.org/1913353002/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_extensions/detail_view.js:16: value: function() {
return { hero: this.$.main }; }
{hero: this.$.main}

no spaces around braces

https://codereview.chromium.org/1913353002/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_extensions/detail_view.js:16: value: function() {
return { hero: this.$.main }; }
maybe just set this inside of ready?  i.e.

ready: function() {
  this.heroElement = this.$.main;
},

https://codereview.chromium.org/1913353002/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_extensions/detail_view.js:16: value: function() {
return { hero: this.$.main }; }
why the `sharedElements` object when there's only 1 element you're exposing?

https://codereview.chromium.org/1913353002/diff/60001/chrome/browser/resource...
File chrome/browser/resources/md_extensions/item_list.js (right):

https://codereview.chromium.org/1913353002/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_extensions/item_list.js:58: 
doc

https://codereview.chromium.org/1913353002/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_extensions/item_list.js:60: this.sharedElements = {
hero: e.detail.element };
no spaces around braces

https://codereview.chromium.org/1913353002/diff/60001/chrome/browser/resource...
File chrome/browser/resources/md_extensions/manager.js (right):

https://codereview.chromium.org/1913353002/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_extensions/manager.js:122: computeListHidden_:
function(a, b, c, d) {
why a, b, c, d with no doc?

https://codereview.chromium.org/1913353002/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_extensions/manager.js:167: 
doc

https://codereview.chromium.org/1913353002/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_extensions/manager.js:172: 
/** @private */

https://codereview.chromium.org/1913353002/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_extensions/manager.js:179: * @param
{extensions.Manager} manager
a helper that takes a manager and implements a delegate?

this code is starting to get a little confusing

https://codereview.chromium.org/1913353002/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_extensions/manager.js:190:
this.manager.$['items-list'].set('items', this.manager.extensions);
why not just ask for for an items list element if it's the only property that's
used from |this.manager|?

https://codereview.chromium.org/1913353002/diff/60001/chrome/browser/resource...
File chrome/browser/resources/md_extensions/service.js (right):

https://codereview.chromium.org/1913353002/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_extensions/service.js:117: assert(data);
nit: i find this more readable (also, assert() returns what's passed to it)

function matchesId(extension) { return extension.id == id; }
var data = assert(this.extensions_.filter(matchesId)[0]);
this.manager_.showDetailsFor(data);

https://codereview.chromium.org/1913353002/diff/60001/chrome/test/data/webui/...
File chrome/test/data/webui/extensions/extension_manager_test.js (right):

https://codereview.chromium.org/1913353002/diff/60001/chrome/test/data/webui/...
chrome/test/data/webui/extensions/extension_manager_test.js:118:
manager.listHelper_.showApps();
might be worthwhile to use MockInteractions.tap() instead

https://codereview.chromium.org/1913353002/diff/60001/chrome/test/data/webui/...
File chrome/test/data/webui/extensions/extension_sidebar_test.js (right):

https://codereview.chromium.org/1913353002/diff/60001/chrome/test/data/webui/...
chrome/test/data/webui/extensions/extension_sidebar_test.js:32: showExtensions:
function() {},
it's not currently clear from this API that apps and extensions are mutually
exclusive

maybe make an enum mode for the page to be so it's clearer that things are
mutually exclusive, i.e.

/** @enum {number} */
var ShowingType = {
  EXTENSIONS: 1,
  APPS: 2,
};

[Dan Beam, 2016-04-26 01:32:46]

also, could you post screenshots?

[Devlin, 2016-04-26 17:24:19]

Screenshots:
https://drive.google.com/corp/drive/u/0/folders/0BxjK6_MzZ8UPSjg3cXhfbkNXYlE
(sorry, internal only)

https://codereview.chromium.org/1913353002/diff/60001/chrome/browser/resource...
File chrome/browser/resources/md_extensions/compiled_resources.gyp (right):

https://codereview.chromium.org/1913353002/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_extensions/compiled_resources.gyp:22:
'detail_view.js',
On 2016/04/26 01:32:12, Dan Beam wrote:
> dare you to switch to v2 GYP ;)

I'll do this in a separate patch set.

https://codereview.chromium.org/1913353002/diff/60001/chrome/browser/resource...
File chrome/browser/resources/md_extensions/detail_view.html (right):

https://codereview.chromium.org/1913353002/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_extensions/detail_view.html:20: color: #757575;
On 2016/04/26 01:32:12, Dan Beam wrote:
> where are you getting these color values from?  are there --paper or --google
> equivalents?

Done.

https://codereview.chromium.org/1913353002/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_extensions/detail_view.html:24: padding: 8px 12px 0
12px;
On 2016/04/26 01:32:12, Dan Beam wrote:
> nit: padding: 8px 12px 0;

One of these days I'll memorize what 3 values means in margin/padding.  Done.

https://codereview.chromium.org/1913353002/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_extensions/detail_view.html:31: padding: 12px 20px
12px 20px;
On 2016/04/26 01:32:12, Dan Beam wrote:
> nit: padding: 12px 20px;

Done.

https://codereview.chromium.org/1913353002/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_extensions/detail_view.html:58: <div
class="section-title">[[i18n('itemDescriptionLabel')]]</div>
On 2016/04/26 01:32:12, Dan Beam wrote:
> or you can use $i18n{} ;)

Done.  Is that done pre-processing (or what's the benefit over this)?

https://codereview.chromium.org/1913353002/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_extensions/detail_view.html:76: <template
is="dom-if" if="[[data.incognitoAccess.isEnabled]]">
On 2016/04/26 01:32:12, Dan Beam wrote:
> don't we want to pu this dom-if around the whole section?

Hmm... kind of (a lot more controls are gonna go in here).  Added a different
dom-if that can be extended for those controls (I'll have a patch up that uses
it later today).

https://codereview.chromium.org/1913353002/diff/60001/chrome/browser/resource...
File chrome/browser/resources/md_extensions/detail_view.js (right):

https://codereview.chromium.org/1913353002/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_extensions/detail_view.js:16: value: function() {
return { hero: this.$.main }; }
On 2016/04/26 01:32:12, Dan Beam wrote:
> {hero: this.$.main}
> 
> no spaces around braces

Done.

https://codereview.chromium.org/1913353002/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_extensions/detail_view.js:16: value: function() {
return { hero: this.$.main }; }
On 2016/04/26 01:32:12, Dan Beam wrote:
> why the `sharedElements` object when there's only 1 element you're exposing?

Was following the neon pages example.  But if we don't have the sharedElements
property, the hero animation doesn't work.

https://codereview.chromium.org/1913353002/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_extensions/detail_view.js:16: value: function() {
return { hero: this.$.main }; }
On 2016/04/26 01:32:12, Dan Beam wrote:
> maybe just set this inside of ready?  i.e.
> 
> ready: function() {
>   this.heroElement = this.$.main;
> },

That doesn't seem to work (the hero animation looks for the sharedElements
property, I think).  But we can do
ready: function() {
  this.sharedElements = {hero: this.$.main};
}

https://codereview.chromium.org/1913353002/diff/60001/chrome/browser/resource...
File chrome/browser/resources/md_extensions/item_list.js (right):

https://codereview.chromium.org/1913353002/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_extensions/item_list.js:58: 
On 2016/04/26 01:32:12, Dan Beam wrote:
> doc

Done.

https://codereview.chromium.org/1913353002/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_extensions/item_list.js:60: this.sharedElements = {
hero: e.detail.element };
On 2016/04/26 01:32:12, Dan Beam wrote:
> no spaces around braces

Done.

https://codereview.chromium.org/1913353002/diff/60001/chrome/browser/resource...
File chrome/browser/resources/md_extensions/manager.js (right):

https://codereview.chromium.org/1913353002/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_extensions/manager.js:122: computeListHidden_:
function(a, b, c, d) {
On 2016/04/26 01:32:12, Dan Beam wrote:
> why a, b, c, d with no doc?

Whoops, was debugging.  Removed.

https://codereview.chromium.org/1913353002/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_extensions/manager.js:167: 
On 2016/04/26 01:32:12, Dan Beam wrote:
> doc

Done.

https://codereview.chromium.org/1913353002/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_extensions/manager.js:172: 
On 2016/04/26 01:32:12, Dan Beam wrote:
> /** @private */

Done.

https://codereview.chromium.org/1913353002/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_extensions/manager.js:179: * @param
{extensions.Manager} manager
On 2016/04/26 01:32:12, Dan Beam wrote:
> a helper that takes a manager and implements a delegate?
> 
> this code is starting to get a little confusing

It's okay, we don't yet have Factory, Impl, or half a dozen other generic
pattern names! ;)

Really, this is only needed because we have yet to find a
polymeric-closure-compilable-way of having Manager override another class
directly - ListHelper is entirely an implementation detail of the manager.

https://codereview.chromium.org/1913353002/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_extensions/manager.js:190:
this.manager.$['items-list'].set('items', this.manager.extensions);
On 2016/04/26 01:32:12, Dan Beam wrote:
> why not just ask for for an items list element if it's the only property
that's
> used from |this.manager|?

We also use manager.extensions and manager.apps.  We could pass in all three,
but it seemed too verbose.  Again, this is really just an implementation detail,
and would be cleaner if we could just put showApps() and showExtensions()
directly on manager, but we can't.  I'm thinking of these as the equivalent of
private inner classes in C++.

https://codereview.chromium.org/1913353002/diff/60001/chrome/browser/resource...
File chrome/browser/resources/md_extensions/service.js (right):

https://codereview.chromium.org/1913353002/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_extensions/service.js:117: assert(data);
On 2016/04/26 01:32:13, Dan Beam wrote:
> nit: i find this more readable (also, assert() returns what's passed to it)
> 
> function matchesId(extension) { return extension.id == id; }
> var data = assert(this.extensions_.filter(matchesId)[0]);
> this.manager_.showDetailsFor(data);
> 

Done.

https://codereview.chromium.org/1913353002/diff/60001/chrome/test/data/webui/...
File chrome/test/data/webui/extensions/extension_manager_test.js (right):

https://codereview.chromium.org/1913353002/diff/60001/chrome/test/data/webui/...
chrome/test/data/webui/extensions/extension_manager_test.js:118:
manager.listHelper_.showApps();
On 2016/04/26 01:32:13, Dan Beam wrote:
> might be worthwhile to use MockInteractions.tap() instead

I cringe a little at that because it relies on the sidebar element, and I have
some far-off dreams of a day when we could test these elements more efficiently
in greater isolation.  I'd prefer it like this.  (Note that the sidebar test
uses MockInteractions.tap() in order to test that clicking on the sidebar
properly calls showX().)

https://codereview.chromium.org/1913353002/diff/60001/chrome/test/data/webui/...
File chrome/test/data/webui/extensions/extension_sidebar_test.js (right):

https://codereview.chromium.org/1913353002/diff/60001/chrome/test/data/webui/...
chrome/test/data/webui/extensions/extension_sidebar_test.js:32: showExtensions:
function() {},
On 2016/04/26 01:32:13, Dan Beam wrote:
> it's not currently clear from this API that apps and extensions are mutually
> exclusive
> 
> maybe make an enum mode for the page to be so it's clearer that things are
> mutually exclusive, i.e.
> 
> /** @enum {number} */
> var ShowingType = {
>   EXTENSIONS: 1,
>   APPS: 2,
> };

Done.  For now, I've put ShowingType in sidebar.js, but lemme know if you want
it somewhere else.

https://codereview.chromium.org/1913353002/diff/80001/chrome/browser/resource...
File chrome/browser/resources/md_extensions/detail_view.js (right):

https://codereview.chromium.org/1913353002/diff/80001/chrome/browser/resource...
chrome/browser/resources/md_extensions/detail_view.js:11: behaviors:
[Polymer.NeonAnimatableBehavior, I18nBehavior],
I'm getting closure warnings about NeonAnimatableBehavior (here and in
item_list.js):
ERROR - Behaviors must be global, fully qualified names which are declared as
object literals or array literals of other valid Behaviors.
##     behaviors: [Polymer.NeonAnimatableBehavior, I18nBehavior],
ERROR - Property NeonAnimatableBehavior never defined on Polymer
##     behaviors: [Polymer.NeonAnimatableBehavior, I18nBehavior],

The behavior files are included, so I'm a little confused.

https://codereview.chromium.org/1913353002/diff/80001/chrome/browser/resource...
chrome/browser/resources/md_extensions/detail_view.js:16: /** @suppress
{checkTypes} Compiler doesn't recognize this.$.main. */
:(

https://codereview.chromium.org/1913353002/diff/80001/chrome/browser/resource...
File chrome/browser/resources/md_extensions/sidebar.js (right):

https://codereview.chromium.org/1913353002/diff/80001/chrome/browser/resource...
chrome/browser/resources/md_extensions/sidebar.js:38: * @param {*} type (Closure
says "ShowingType" is undefined)
:(

[Devlin, 2016-04-28 02:02:58]

fyi, rebased onto v2 gyp

[Devlin, 2016-05-02 17:06:38]

friendly ping -> dbeam@.

After this patch lands, a lot of the other ones are more independent, but this
is the gatekeeper to most of them, so I'd like to get it in sooner rather than
later. :)

[Dan Beam, 2016-05-03 19:16:49]

looks pretty darn good to me :)

https://codereview.chromium.org/1913353002/diff/120001/chrome/browser/resourc...
File chrome/browser/resources/md_extensions/detail_view.html (right):

https://codereview.chromium.org/1913353002/diff/120001/chrome/browser/resourc...
chrome/browser/resources/md_extensions/detail_view.html:31: border-bottom: 1px
solid #ccc;
why #ccc and not a --paper or --google color?  did bettes@ ask for this
specifically?

https://codereview.chromium.org/1913353002/diff/120001/chrome/browser/resourc...
chrome/browser/resources/md_extensions/detail_view.html:70: <template
is="dom-if" if="[[hasDependentExtensions_(data.*)]]">
is there a reason you need to use .* here?

https://codereview.chromium.org/1913353002/diff/120001/chrome/browser/resourc...
File chrome/browser/resources/md_extensions/detail_view.js (right):

https://codereview.chromium.org/1913353002/diff/120001/chrome/browser/resourc...
chrome/browser/resources/md_extensions/detail_view.js:16: /** @suppress
{checkTypes} Compiler doesn't recognize this.$.main. */
does /** @this {PolymerElement} */ work?

https://codereview.chromium.org/1913353002/diff/120001/chrome/browser/resourc...
chrome/browser/resources/md_extensions/detail_view.js:49: this.fire('close');
i might be crazy, but i vaguely remember 'event-names-with-dashes' bubble
whereas 'nameslikethis' might not?

i didn't find anything about this in the shadow DOM doc other than:
https://www.w3.org/TR/shadow-dom/#events-that-are-not-leaked-into-ancestor-trees

here's how .fire() works:
https://github.com/Polymer/polymer/blob/edb59eb6283074ce54f374b30b884c9f9fcf9...

https://codereview.chromium.org/1913353002/diff/120001/chrome/browser/resourc...
chrome/browser/resources/md_extensions/detail_view.js:52: /** @private */
@return

https://codereview.chromium.org/1913353002/diff/120001/chrome/browser/resourc...
chrome/browser/resources/md_extensions/detail_view.js:57: /** @private */
@return

https://codereview.chromium.org/1913353002/diff/120001/chrome/browser/resourc...
chrome/browser/resources/md_extensions/detail_view.js:58:
computeOptionsSectionShown_: function() {
nit: showOptionSection_ or shouldShowOptionsSection_

https://codereview.chromium.org/1913353002/diff/120001/chrome/browser/resourc...
File chrome/browser/resources/md_extensions/manager.js (right):

https://codereview.chromium.org/1913353002/diff/120001/chrome/browser/resourc...
chrome/browser/resources/md_extensions/manager.js:127: * @return {boolean}
Whether the list should be visible.
@private

https://codereview.chromium.org/1913353002/diff/120001/chrome/browser/resourc...
chrome/browser/resources/md_extensions/manager.js:177: * @param {CustomEvent} e
@private

https://codereview.chromium.org/1913353002/diff/120001/chrome/browser/resourc...
chrome/browser/resources/md_extensions/manager.js:177: * @param {CustomEvent} e
awesome!  i didn't know CustomEvent was a thing -- it's got a type for .detail,
which is super.  we've been doing all types of other crazy stuff

https://codereview.chromium.org/1913353002/diff/120001/chrome/browser/resourc...
File chrome/browser/resources/md_extensions/sidebar.js (right):

https://codereview.chromium.org/1913353002/diff/120001/chrome/browser/resourc...
chrome/browser/resources/md_extensions/sidebar.js:38: * @param {*} type (Closure
says "ShowingType" is undefined)
maybe try:

cr.exportPath('extensions');

/** @enum {number} */
extensions.ShowingType = {
  EXTENSIONS: 0,
  APPS: 1,
};

cr.define('extensions', function() {
  // using extensions.ShowingType should now work
});

[Devlin, 2016-05-03 19:56:55]

https://codereview.chromium.org/1913353002/diff/120001/chrome/browser/resourc...
File chrome/browser/resources/md_extensions/detail_view.html (right):

https://codereview.chromium.org/1913353002/diff/120001/chrome/browser/resourc...
chrome/browser/resources/md_extensions/detail_view.html:31: border-bottom: 1px
solid #ccc;
On 2016/05/03 19:16:49, Dan Beam wrote:
> why #ccc and not a --paper or --google color?  did bettes@ ask for this
> specifically?

Whoops, missed this one.  In general, I'm not going after color perfection quite
yet, because some of the values are still missing from the specs.  But will at
least change this to be a paper color value.

https://codereview.chromium.org/1913353002/diff/120001/chrome/browser/resourc...
chrome/browser/resources/md_extensions/detail_view.html:70: <template
is="dom-if" if="[[hasDependentExtensions_(data.*)]]">
On 2016/05/03 19:16:49, Dan Beam wrote:
> is there a reason you need to use .* here?

Probably not.  I think data.dependentExtensions.splices should work.

https://codereview.chromium.org/1913353002/diff/120001/chrome/browser/resourc...
File chrome/browser/resources/md_extensions/detail_view.js (right):

https://codereview.chromium.org/1913353002/diff/120001/chrome/browser/resourc...
chrome/browser/resources/md_extensions/detail_view.js:16: /** @suppress
{checkTypes} Compiler doesn't recognize this.$.main. */
On 2016/05/03 19:16:49, Dan Beam wrote:
> does /** @this {PolymerElement} */ work?

No. :(

https://codereview.chromium.org/1913353002/diff/120001/chrome/browser/resourc...
chrome/browser/resources/md_extensions/detail_view.js:49: this.fire('close');
On 2016/05/03 19:16:49, Dan Beam wrote:
> i might be crazy, but i vaguely remember 'event-names-with-dashes' bubble
> whereas 'nameslikethis' might not?
> 
> i didn't find anything about this in the shadow DOM doc other than:
>
https://www.w3.org/TR/shadow-dom/#events-that-are-not-leaked-into-ancestor-trees
> 
> here's how .fire() works:
>
https://github.com/Polymer/polymer/blob/edb59eb6283074ce54f374b30b884c9f9fcf9...

Well, according to fire(), bubbles defaults to true and is only changed if we
pass in options saying otherwise.  So I think we're okay?

https://codereview.chromium.org/1913353002/diff/120001/chrome/browser/resourc...
chrome/browser/resources/md_extensions/detail_view.js:52: /** @private */
On 2016/05/03 19:16:49, Dan Beam wrote:
> @return

Done.

https://codereview.chromium.org/1913353002/diff/120001/chrome/browser/resourc...
chrome/browser/resources/md_extensions/detail_view.js:57: /** @private */
On 2016/05/03 19:16:49, Dan Beam wrote:
> @return

Done.

https://codereview.chromium.org/1913353002/diff/120001/chrome/browser/resourc...
chrome/browser/resources/md_extensions/detail_view.js:58:
computeOptionsSectionShown_: function() {
On 2016/05/03 19:16:49, Dan Beam wrote:
> nit: showOptionSection_ or shouldShowOptionsSection_

Done.

https://codereview.chromium.org/1913353002/diff/120001/chrome/browser/resourc...
File chrome/browser/resources/md_extensions/manager.js (right):

https://codereview.chromium.org/1913353002/diff/120001/chrome/browser/resourc...
chrome/browser/resources/md_extensions/manager.js:127: * @return {boolean}
Whether the list should be visible.
On 2016/05/03 19:16:49, Dan Beam wrote:
> @private

Done.

https://codereview.chromium.org/1913353002/diff/120001/chrome/browser/resourc...
chrome/browser/resources/md_extensions/manager.js:177: * @param {CustomEvent} e
On 2016/05/03 19:16:49, Dan Beam wrote:
> awesome!  i didn't know CustomEvent was a thing -- it's got a type for
.detail,
> which is super.  we've been doing all types of other crazy stuff

Another item for your bag o' tricks. ;)

https://codereview.chromium.org/1913353002/diff/120001/chrome/browser/resourc...
chrome/browser/resources/md_extensions/manager.js:177: * @param {CustomEvent} e
On 2016/05/03 19:16:49, Dan Beam wrote:
> @private

Done.

https://codereview.chromium.org/1913353002/diff/120001/chrome/browser/resourc...
File chrome/browser/resources/md_extensions/sidebar.js (right):

https://codereview.chromium.org/1913353002/diff/120001/chrome/browser/resourc...
chrome/browser/resources/md_extensions/sidebar.js:38: * @param {*} type (Closure
says "ShowingType" is undefined)
On 2016/05/03 19:16:49, Dan Beam wrote:
> maybe try:
> 
> cr.exportPath('extensions');
> 
> /** @enum {number} */
> extensions.ShowingType = {
>   EXTENSIONS: 0,
>   APPS: 1,
> };
> 
> cr.define('extensions', function() {
>   // using extensions.ShowingType should now work
> });

Yep, that worked.  Annoying that we can't do it the other way - especially when
other things declared like that (Sidebar, SidebarListDelegate, everything) work
just fine.  Any idea why?

[Dan Beam, 2016-05-03 20:55:04]

lgtm

https://codereview.chromium.org/1913353002/diff/120001/chrome/browser/resourc...
File chrome/browser/resources/md_extensions/detail_view.js (right):

https://codereview.chromium.org/1913353002/diff/120001/chrome/browser/resourc...
chrome/browser/resources/md_extensions/detail_view.js:16: /** @suppress
{checkTypes} Compiler doesn't recognize this.$.main. */
On 2016/05/03 19:56:55, Devlin wrote:
> On 2016/05/03 19:16:49, Dan Beam wrote:
> > does /** @this {PolymerElement} */ work?
> 
> No. :(

note:

value: /** @this {PolymerElement} */ function() {
  ...
},

is probably how it'd have to be annotated

https://codereview.chromium.org/1913353002/diff/120001/chrome/browser/resourc...
File chrome/browser/resources/md_extensions/sidebar.js (right):

https://codereview.chromium.org/1913353002/diff/120001/chrome/browser/resourc...
chrome/browser/resources/md_extensions/sidebar.js:38: * @param {*} type (Closure
says "ShowingType" is undefined)
On 2016/05/03 19:56:55, Devlin wrote:
> On 2016/05/03 19:16:49, Dan Beam wrote:
> > maybe try:
> > 
> > cr.exportPath('extensions');
> > 
> > /** @enum {number} */
> > extensions.ShowingType = {
> >   EXTENSIONS: 0,
> >   APPS: 1,
> > };
> > 
> > cr.define('extensions', function() {
> >   // using extensions.ShowingType should now work
> > });
> 
> Yep, that worked.  Annoying that we can't do it the other way - especially
when
> other things declared like that (Sidebar, SidebarListDelegate, everything)
work
> just fine.  Any idea why?

i think it's a long-standing bug in the old closure typing system

--new_type_inf seems to help fix this, I think? but it previously had other
problems, iirc.

[Devlin, 2016-05-03 22:59:58]

The CQ bit was checked by rdevlin.cronin@chromium.org

[commit-bot: I haz the power, 2016-05-03 23:00:31]

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

[commit-bot: I haz the power, 2016-05-04 00:07:58]

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

[commit-bot: I haz the power, 2016-05-04 00:08:00]

Failed to apply the patch.

[commit-bot: I haz the power, 2016-05-04 00:08:06]

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

[Devlin, 2016-05-04 00:10:21]

Description was changed from

==========
[MD Extensions] Add a details subpage, move to one list

Kick the dust off the MD extensions implementation now that we have
updated mocks.
* Add a "details" subpage that shows the details for a given extension.
  Note that this isn't complete - there are quite a few TODOs left.
* Move to a single list view instead of three lists for extensions,
  apps, and websites.
* Combine the apps and websites sections.

BUG=529395
==========

to

==========
[MD Extensions] Add a details subpage, move to one list

Kick the dust off the MD extensions implementation now that we have
updated mocks.
* Add a "details" subpage that shows the details for a given extension.
  Note that this isn't complete - there are quite a few TODOs left.
* Move to a single list view instead of three lists for extensions,
  apps, and websites.
* Combine the apps and websites sections.

BUG=529395
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

[Devlin, 2016-05-04 00:10:29]

The CQ bit was checked by rdevlin.cronin@chromium.org

[Devlin, 2016-05-04 00:10:30]

The patchset sent to the CQ was uploaded after l-g-t-m from dbeam@chromium.org
Link to the patchset: https://codereview.chromium.org/1913353002/#ps160001
(title: "latest master")

[commit-bot: I haz the power, 2016-05-04 00:11:08]

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

[commit-bot: I haz the power, 2016-05-04 02:10:10]

Committed patchset #7 (id:160001)

[commit-bot: I haz the power, 2016-05-04 02:11:46]

Description was changed from

==========
[MD Extensions] Add a details subpage, move to one list

Kick the dust off the MD extensions implementation now that we have
updated mocks.
* Add a "details" subpage that shows the details for a given extension.
  Note that this isn't complete - there are quite a few TODOs left.
* Move to a single list view instead of three lists for extensions,
  apps, and websites.
* Combine the apps and websites sections.

BUG=529395
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

to

==========
[MD Extensions] Add a details subpage, move to one list

Kick the dust off the MD extensions implementation now that we have
updated mocks.
* Add a "details" subpage that shows the details for a given extension.
  Note that this isn't complete - there are quite a few TODOs left.
* Move to a single list view instead of three lists for extensions,
  apps, and websites.
* Combine the apps and websites sections.

BUG=529395
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/da2ca5ac93171a4ab1352063f5103eae84e5f8ce
Cr-Commit-Position: refs/heads/master@{#391408}
==========

[commit-bot: I haz the power, 2016-05-04 02:11:48]

Patchset 7 (id:??) landed as
https://crrev.com/da2ca5ac93171a4ab1352063f5103eae84e5f8ce
Cr-Commit-Position: refs/heads/master@{#391408}