Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

ng-include with ng-transclude - should be in core #6580

Open
parliament718 opened this issue Mar 6, 2014 · 7 comments
Open

ng-include with ng-transclude - should be in core #6580

parliament718 opened this issue Mar 6, 2014 · 7 comments

Comments

@parliament718
Copy link

I really think ng-transclude should work out of the box with ng-include. The wrapper markup generated by ng-include often breaks many css selectors such as "parent > directChild" and "element + directSibling". For example, a simple bootstrap dropdown:

     <div class="dropdown">
                <button type="button" class="btn btn-primary dropdown-toggle">
                    Dropme <i class="fa fa-chevron-down"></i>
                </button>
                <div ng-include="'template.html'"></div>
            </div>

If template.html contains the <ul class="dropdown-menu">, it will be wrapped in <div class="ng-scope"> and will no longer work. Ng-include should really account for this type of situation either with a replace option or transclude support:

Replace option would put the extra attributes like class="ng-scope" directly on the element instead of using a wrapper:

     <div class="dropdown">
                <button type="button" class="btn btn-primary dropdown-toggle">
                    Dropme <i class="fa fa-chevron-down"></i>
                </button>
                <div ng-include="'template.html'" replace></div>
            </div>

Transclusion option :

 <div class="dropdown" ng-include="'template.html'">
                <button type="button" class="btn btn-primary dropdown-toggle">
                    Dropme <i class="fa fa-chevron-down"></i>
                </button>
                <div ng-transclude-include></div>
            </div>

Any reasons against this?

@gsklee
Copy link
Contributor

gsklee commented Mar 6, 2014

@parliament718 I would argue instead that this is primarily the CSS's fault; I've been tripped over by the very restrictive .open > .dropdown-menu a few times as well and the bad thing is this required element structure is not specified in Bootstrap's doc as well as UI Bootstrap's.

You can easily fix this issue with an .open .dropdown-menu overwrite though. Actually, I would also argue that Bootstrap should change the way this widget works so that the open class is being added onto the .dropdown-menu element directly, which will make the CSS rule into .dropdown-menu.open for better granular control on end user side.

@parliament718
Copy link
Author

@gsklee You do have a point, thanks for weighing in. On the other hand, I would argue that pointless wrappers just for the sake of class="ng-scope" should be avoided across the board. I cannot think of any reason not to put them on the element directly by default and avoid polluting the markup away from the intuitive and intended result.

Restrictive CSS3 selectors are the solution to such problems as nested dropdowns where .open .dropdown-menu would open all nested menus (my case). There is also good reason for placing .open on the entire wrapper to allow for state changes on related markup such as the dropdown button outside the menu (neccesary for my case as well).

@lancecaraccioli
Copy link

I agree, adding DOM nodes is very obtrusive. There should be an option to replace the ng-include element. I'm using angular-ui-bootstrap in my use case. I load navigation items from a service. Some of the items have drop down menu partials, which means I need to dynamically include them, however this is not possible unless I want to make the ng-include element the menu item container and change the partials to only render the menu items. This feels wrong of course.

@Narretz Narretz added this to the Ice Box milestone Jul 3, 2014
@btford btford removed the gh: issue label Aug 20, 2014
@hitautodestruct
Copy link

Just chiming in here about breaking selectors. The .parent > .child selectors are not the only ones that break how about a very useful :nth-child? Or, :first-child and :last-child?

@adammessinger
Copy link

This would make a great addition, and remove the need to create custom directives whose only purpose is to load a partial template. I'm currently working on a dynamic Reveal.js slideshow, and the slideshow framework is quite picky about its use of adjacent and nested <section> elements to create horizontal and vertical slide transitions.

@dietergeerts
Copy link

This should really be included! Also have these issues in my current project.

@frfancha
Copy link

@gsklee
You say "I would argue instead that this is primarily the CSS's fault;"
<li> must be under <ul>, not at all CSS's fault

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

9 participants