Skip to content
This repository has been archived by the owner on May 29, 2019. It is now read-only.

fix(modal): add a work-around for transclusion scope #2143

Closed
wants to merge 1 commit into from

Conversation

pkozlowski-opensource
Copy link
Member

One of the most common complains about the $modal service is that it is using a transclusion scope for its content. While this is how "AngularJS works" and the created scopes layout makes perfect sense, this transclusion scope still seems to be very confusing for many people... So, here is my attempt at fixing it. The crux of the idea is to use a custom transclusion directive (modalTransclude). The fix is rather small and should be "safe". Any feedback highly appreciated!

Fixes #2110
Closes #2134

@Foxandxss
Copy link
Contributor

Yeah, that is one way to do that. We just need to watch Angular because technically this behavior would change some day.

@pkozlowski-opensource
Copy link
Member Author

@Foxandxss yeh, I'm really not feeling too great about adding this work-around but it really seems to confuse people :-/ When AngularJS decides to remove transclusion scope we will just need to remove this additional directive and change a template...

But yes, this is the situation where we seem to add a work-around for something that is "technically right", but still people find it confusing. But I guess the "customer is always right"....

@Foxandxss
Copy link
Contributor

Agree. The problem is that we are offering a similar behavior than the router but we add an extra layer of scoping (transclusion) and we are not telling anybody about that. That and also that scope inheritance is still the biggest PITA in angular for all users.

I am 👍 with this PR, that will remove the problem and technically is not a breaking change. When Angular updates itself, we remove it and no one notices :)

@paulpflug
Copy link

much better than my workaround.
One piece is missing, though:
The scope isn't cleaned up on close.

@pkozlowski-opensource
Copy link
Member Author

@paulpflug which scope are you talking about? I was looking today into scope-related issues and I couldn't find any leaking scopes. Could you please try to 0.11.0 release and let me know if you can see any leaking scopes?

A word of warning: if you are using AngularJS Batarang to track leaking scopes you need to be careful, as this plugin is rather buggy and doesn't always show the right data....

@paulpflug
Copy link

yep you are right and I'm wrong ;) very well. awesome fix.

@chrisirhc
Copy link
Contributor

Looks good to me. 👍

@pkozlowski-opensource
Copy link
Member Author

OK, good, I'm going to merge it later today, then.

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

Successfully merging this pull request may close these issues.

$modal does not provide proper scope to templates.
4 participants