Skip to content

Commit

Permalink
fix($compile): correctly merge class attr for replace directives
Browse files Browse the repository at this point in the history
Merging of interpolated class attribute from directive template with replace:true works

Closes angular#1006
  • Loading branch information
maxmart authored and IgorMinar committed Jun 7, 2012
1 parent a57141f commit efd736b
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 0 deletions.
3 changes: 3 additions & 0 deletions src/ng/compile.js
Original file line number Diff line number Diff line change
Expand Up @@ -844,6 +844,7 @@ function $CompileProvider($provide) {
var srcAttr = src.$attr,
dstAttr = dst.$attr,
$element = dst.$$element;

// reapply the old attributes to the new element
forEach(dst, function(value, key) {
if (key.charAt(0) != '$') {
Expand All @@ -853,10 +854,12 @@ function $CompileProvider($provide) {
dst.$set(key, value, true, srcAttr[key]);
}
});

// copy the new attributes on the old attrs object
forEach(src, function(value, key) {
if (key == 'class') {
safeAddClass($element, value);
dst.class = (dst.class ? dst.class + ' ' : '') + value;

This comment has been minimized.

Copy link
@IgorMinar

IgorMinar Jun 7, 2012

Owner

Vojta, Misko: do we need to update dstAttr.class as well? I wasn't sure and if I tried it, I started getting DOM exceptions.

This comment has been minimized.

Copy link
@mhevery

mhevery Jun 7, 2012

I don't think so, I think class is special and can be ignored

} else if (key == 'style') {
$element.attr('style', $element.attr('style') + ';' + value);
} else if (key.charAt(0) != '$' && !dst.hasOwnProperty(key)) {
Expand Down
16 changes: 16 additions & 0 deletions test/ng/compileSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -378,6 +378,14 @@ describe('$compile', function() {
expect(element).toBe(attr.$$element);
}
}));
$compileProvider.directive('replaceWithInterpolatedClass', valueFn({
replace: true,
template: '<div class="class_{{1+1}}">Replace with interpolated class!</div>',
compile: function(element, attr) {
attr.$set('compiled', 'COMPILED');
expect(element).toBe(attr.$$element);
}
}));
}));


Expand Down Expand Up @@ -456,6 +464,14 @@ describe('$compile', function() {
}));


it('should handle interpolated css from replacing directive', inject(
function($compile, $rootScope) {
element = $compile('<div replace-with-interpolated-class></div>')($rootScope);
$rootScope.$digest();
expect(element).toHaveClass('class_2');
}));


it('should merge interpolated css class', inject(function($compile, $rootScope) {
element = $compile('<div class="one {{cls}} three" replace></div>')($rootScope);

Expand Down

1 comment on commit efd736b

@mhevery
Copy link

@mhevery mhevery commented on efd736b Jun 7, 2012

Choose a reason for hiding this comment

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

LGTM

Please sign in to comment.