Plugin-outlet incorrectly handles multiple plugins at the same outlet

(Discourse.PRO) #1

Wrong code is:

Must be Ember.ContainerView.extend({tagName: ''}) instead of Ember.ContainerView.
The current wrong code breaks HTML markup in cases where HTML syntax does not allow wrapper DIV tag.
An example is admin-menu plugin outlet:

If there are multiple admin-menu connectors then Discourse wrongly renders them as:

	<!-- standard menu items: -->
	<!-- admin-menu connectors: -->

Please lose the divs in the lists
Style broken on user-activity-bottom-outlet
Inconsistent margin for solved user summary
(Mittineague) #2

I noticed this last May with the hamburger menu’s (aka sitemap) plugin-outlet but never got around to digging into it.

(Sam Saffron) #3

What you are asking for here is a container view without a container view… @eviltrout this is gonna be might tricky if we decide to support this.

(Robin Ward) #4

A container view can have a tagName, I just have to pass it through. Should not be too hard.

(Robin Ward) #5

Oops I see what you mean Sam. If I apply the tagname to the ContainerView it’s not what you want, because then you are expecting the children to have li too.

I’m gonna ask people about this. Maybe I can append to the Morph.

(Discourse.PRO) #6

Replacing Ember.ContainerView to Ember.ContainerView.extend({tagName: ''}) as I described in the first message definitely works, I have tested it for my multiple plugins at admin-menu outlet.

(Robin Ward) #7

It didn’t work when I tried it. If you pass the tagName to the ContainerView the container gets the li tag, but then the children get div tags. So adding two <li> doesn’t work.

(cpradio) #8

Except, I think that is valid HTML now (as of HTML 5). I think li can contain block elements (such as div), so that is less of a problem than the li being wrapped in a div.

(Discourse.PRO) #9

<li id="ember1719" class="ember-view">  		
	<a id="ember1993" class="ember-view" href="/admin/files">Files</a>
<li id="ember1724" class="ember-view">  		
	<a id="ember2020" class="ember-view" href="/admin/shop">Shop</a>


import NavItem from 'discourse/components/nav-item';
export default NavItem.extend({
	_init: function() {
		this.set('route', 'adminFiles');
		this.set('label', 'df.files.title');


{{#if routeParam}}
	{{#link-to route routeParam}}{{i18n label}}{{/link-to}}
	{{#if route}}
		{{#link-to route}}{{i18n label}}{{/link-to}}
		{{#if path}}
			<a href="{{unbound fullPath}}" data-auto-route="true">{{i18n label}}</a>
			<a href="{{unbound href}}" data-auto-route="true">{{i18n label}}</a>

(Robin Ward) #10

Can you show more of your HTML snippet? I want to see what the <li> tags are wrapped in. I am seeing different results.

(Discourse.PRO) #11

Wrapped in UL, of course.

(Nik Davis) #12

Any progress on this? I’d be happy if I could get one plugin to render w/o a wrapping div.

(Sam Saffron) #13

I think the only way to do this would be to only allow widgets for these cases, @eviltrout would that be doable?

(Robin Ward) #14

Even widgets require a master container div. @nikdavis is your situation the same menu as above? Or elsewhere?

(Nik Davis) #15

Different. I’m working on the user-activity-bottom plugin outlet. The .nav-stacked styling uses a child selector which breaks on the outlet.

(Sam Saffron) #16

In theory with a widget container we could use metamorphs optionally for the extend a ul option

(Robin Ward) #17

Sure a widget could be mounted with a metamorph or something similar - the problem is you always need a {{mount-widget}} to start hosting them and that needs a tag.

(Sam Saffron) split this topic #18

A post was split to a new topic: Allow registrations of components as plugin outlets

(Sam Saffron) #19

Also, important to note, the OP is resolved here as of latest, so I might as well close and split.

(Sam Saffron) closed #20