Skip to content

Don't render a view for empty content #557

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

Don't render a view for empty content #557

wants to merge 1 commit into from

Conversation

jonrimmer
Copy link

Speculative fix for #534.

My original plan to revert to the old-style behaviour proved impossible: The ngAnimate module in 1.2 will only apply animations on the ui-view element itself, based on its class attribute, meaning the element must participate in the cloning/adding/removing process.

Instead, I have modified the directive to not render anything if both the view template and the default content are empty strings. In this way, it is possible to wrap the ui-view in a container element that will match the CSS :empty selector when there is no view loaded.

@timkindberg
Copy link
Contributor

That seems like a much better solution, and gets you to your goal of using :empty.

@jonrimmer
Copy link
Author

Yes, although now I've been looking at @dlukez 1.2 refactor in PR #553, and I now foresee a problem with this approach: If the view content is transcluded and runs lower priority directives, as it should do, then the initial view may not really be "empty", even if at first it contains no HTML. These other directives could insert content into it at any point.

I am now thinking that, rather than trying to figure out implicitly that the initial view is empty, it would be better to have an explicit attribute, called something like 'no-initial-view' (better naming suggestions welcome) that can be set on the ui-view element. The view directive would check for this attribute and, if present, would skip the transclusion and rendering of the initial content, allowing the ui-view to render nothing as its default state.

If this seems reasonable, I could submit a new PR that includes @dlukez's changes, plus the approach described above?

Edit: Or actually, you guys could accept that PR and I'll reimplement on top of it.

@timkindberg
Copy link
Contributor

Hmm that is starting to seem a bit hacky. I liked the idea of it just
working. If someone wants to use :empty wouldn't they just take care
not to put those sorts of directives inside?

@jonrimmer
Copy link
Author

I think the requirements here are a little subtle. Someone might want to include directives on the ui-view that transclude to the rendered views. And whether they style on :empty or not, they might want those directives to also apply to the initial view, or they might not want to render the initial view. Even if they don't use additional directives on the initial view, and it's completely empty, they might still want it to be rendered.

For example:

<div ui-view="main" class="main" x-diagram></div>

This ui-view has no content, but does have an x-diagram directive. Imagine that x-diagram inserts some SVG diagram using the result of an ajax call. In this case, what should the view directive render when there is no active view? The initial view will be empty at compile/link time, but might get some content later via the x-diagram directive. There's no way for the view directive to implicitly know for certain whether the initial view should be rendered or not. It depends on the developer's intentions.

Therefore, I see two options:

  1. Render the initial view based on the markup content of the transcluded initial view at compile/link time. If it's empty, don't render it. If contains anything, render it. If the developer had directives they wanted to apply to an otherwise empty initial view, then they would have to add something, such as a whitespace character or an empty element, between the ui-view's start and end tags, so the directive didn't recognise it as empty.
  2. Include an explicit switch via an attribute on the ui-view to let the developer say "don't transclude and render the initial view". This would let them format the ui-view element as they please, including having whitespace and other content inside it.

I don't have a strong preference between the two options. The first will "just work" in the majority of cases, but may catch a few people out if they don't realise that empty views don't render, and those containing whitespace do. The second shouldn't catch anybody out, but will be a little more verbose.

@timkindberg
Copy link
Contributor

That all makes sense. 3rd option would be to have it work like 1 with a flag that is opposite of your 2, have a switch that turned it on instead of off. So its a cross between 2 and 1, because instead of putting in whitespace or a comment or whatever, they would set an attribute saying 'Allow Other Directives To Transclude'.

I'm not the guy hitting this brick wall at the moment, so which do you prefer as an end user (taking into consideration that we want to keep the API elegant)?

I'm just thinking of the docs and where the hell I'm gonna put this, I guess with default view content part of the Templates section.

@jonrimmer
Copy link
Author

Your suggested 3rd option sounds good to me. I will implement it on top of #553 when that gets pulled.

Fortunately we're not blocked on this atm, as we're still using 1.1.5, but I want to switch to 1.2 sooner rather than later. Happy to help out with the docs as well, if needed.

@nateabele
Copy link
Contributor

Someone might want to include directives on the ui-view that transclude to the rendered views.

Not possible. You can't have two transcludes on the same element.

@jonrimmer jonrimmer closed this Nov 5, 2013
@jonrimmer jonrimmer reopened this Nov 5, 2013
@jonrimmer
Copy link
Author

Argh, "close" is not "comment".

I'm not sure you mean about having two transcludes? In #553, the transclude function is used to generate the elements which are then populated with the view template and compiled to become the rendered view elements.

The idea is that directives, such as ng-class, etc. can be placed on the ui-view element, and will be applied on the rendered view elements as and when they are created, rather than applying to the original element.

@nateabele
Copy link
Contributor

@jonrimmer Okay, I follow you now.

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

Successfully merging this pull request may close these issues.

3 participants