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

fix($compile): retain CSS classes added in clone fn on async directive #5617

Closed
wants to merge 1 commit into from

Conversation

caitp
Copy link
Contributor

@caitp caitp commented Jan 3, 2014

In synchronous directives (without a templateUrl), classes added to the node during the clone attach function persist in the post-link function.

Before this patch, classes addedd to asynchronous templates during the clone attach function would not persist after the node is replaced with the template prior to linking.

Fixes #5439

@caitp caitp closed this Jan 3, 2014
@caitp caitp reopened this Jan 3, 2014
@caitp
Copy link
Contributor Author

caitp commented Jan 3, 2014

oops.

// it was cloned therefore we have to clone as well.
linkNode = jqLiteClone(compileNode);
replaceWith(linkRootElement, jqLite(beforeTemplateLinkNode), linkNode);

// Copy in CSS classes from original node
safeAddClass(jqLite(linkNode), oldClasses);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to make a change to safeAddClass so that I can make sure it works correctly for SVG elements and SVGAnimatedString class names.

That can wait for a different PR, though.

@ghost ghost assigned tbosch and matsko Jan 4, 2014
@IgorMinar
Copy link
Contributor

@matsko can you please review?

@matsko
Copy link
Contributor

matsko commented Jan 8, 2014

Hey guys. Sorry for the delay. I am happy to review tomorrow night and/or Thursday.

@IgorMinar
Copy link
Contributor

the test is quite hard to follow any chance it could be simplified.

if not then this lgtm

@caitp
Copy link
Contributor Author

caitp commented Jan 31, 2014

I'll see if I can simplify the test a bit

In synchronous directives (without a templateUrl), classes added to the node
during the clone attach function persist in the post-link function.

Before this patch, classes addedd to asynchronous templates during the clone
attach function would not persist after the node is replaced with the template
prior to linking.

Fixes angular#5439
@caitp
Copy link
Contributor Author

caitp commented Jan 31, 2014

Comments addressed, I'm going to merge this when travis is green

@caitp caitp closed this in 5ed721b Jan 31, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.