fix(core): correctly concatenate static and dynamic binding to class
when shadowed (#35350)
Given: ``` <div class="s1" [class]="null" [ngClass]="exp"> ``` Notice that `[class]` binding is not a `string`. As a result the existing logic would not concatenate `[class]` with `class="s1"`. The resulting falsy value would than be sent to `ngClass` which would promptly clear all styles on the `<div>` The new logic correctly handles falsy values for `[class]` bindings. Fix #35335 PR Close #35350
This commit is contained in:
parent
be5e75c804
commit
8c75f2155d
@ -261,8 +261,9 @@ export function checkStylingMap(
|
|||||||
ngDevMode && isClassBased === false && staticPrefix !== null &&
|
ngDevMode && isClassBased === false && staticPrefix !== null &&
|
||||||
assertEqual(
|
assertEqual(
|
||||||
staticPrefix.endsWith(';'), true, 'Expecting static portion to end with \';\'');
|
staticPrefix.endsWith(';'), true, 'Expecting static portion to end with \';\'');
|
||||||
if (typeof value === 'string') {
|
if (staticPrefix !== null) {
|
||||||
value = concatStringsWithSpace(staticPrefix, value as string);
|
// We want to make sure that falsy values of `value` become empty strings.
|
||||||
|
value = concatStringsWithSpace(staticPrefix, value ? value : '');
|
||||||
}
|
}
|
||||||
// Given `<div [style] my-dir>` such that `my-dir` has `@Input('style')`.
|
// Given `<div [style] my-dir>` such that `my-dir` has `@Input('style')`.
|
||||||
// This takes over the `[style]` binding. (Same for `[class]`)
|
// This takes over the `[style]` binding. (Same for `[class]`)
|
||||||
|
@ -316,6 +316,52 @@ describe('styling', () => {
|
|||||||
expect(div2.getAttribute('shadow-class')).toEqual('s2 d2');
|
expect(div2.getAttribute('shadow-class')).toEqual('s2 d2');
|
||||||
});
|
});
|
||||||
|
|
||||||
|
onlyInIvy('shadow bindings include static portion')
|
||||||
|
.it('should bind [class] as input to directive when both static and falsy dynamic values are present',
|
||||||
|
() => {
|
||||||
|
@Component({
|
||||||
|
template: `
|
||||||
|
<div class="s1" [class]="classBinding" dir-shadows-class-input></div>
|
||||||
|
`
|
||||||
|
})
|
||||||
|
class Cmp {
|
||||||
|
classBinding: any = undefined;
|
||||||
|
}
|
||||||
|
|
||||||
|
@Directive({selector: '[dir-shadows-class-input]'})
|
||||||
|
class DirectiveShadowsClassInput {
|
||||||
|
constructor(private elementRef: ElementRef) {}
|
||||||
|
@Input('class')
|
||||||
|
set klass(value: string) {
|
||||||
|
this.elementRef.nativeElement.setAttribute('shadow-class', value);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
TestBed.configureTestingModule({declarations: [Cmp, DirectiveShadowsClassInput]});
|
||||||
|
const fixture = TestBed.createComponent(Cmp);
|
||||||
|
fixture.detectChanges();
|
||||||
|
|
||||||
|
const div = fixture.nativeElement.querySelector('div');
|
||||||
|
expect(div.className).toEqual('s1');
|
||||||
|
expect(div.getAttribute('shadow-class')).toEqual('s1');
|
||||||
|
|
||||||
|
fixture.componentInstance.classBinding = null;
|
||||||
|
fixture.detectChanges();
|
||||||
|
expect(div.className).toEqual('s1');
|
||||||
|
expect(div.getAttribute('shadow-class')).toEqual('s1');
|
||||||
|
|
||||||
|
fixture.componentInstance.classBinding = false;
|
||||||
|
fixture.detectChanges();
|
||||||
|
expect(div.className).toEqual('s1');
|
||||||
|
expect(div.getAttribute('shadow-class')).toEqual('s1');
|
||||||
|
|
||||||
|
|
||||||
|
fixture.componentInstance.classBinding = {toString: () => 'd1'};
|
||||||
|
fixture.detectChanges();
|
||||||
|
expect(div.className).toEqual('s1');
|
||||||
|
expect(div.getAttribute('shadow-class')).toEqual('s1 d1');
|
||||||
|
});
|
||||||
|
|
||||||
|
|
||||||
modifiedInIvy('shadow bindings include static portion')
|
modifiedInIvy('shadow bindings include static portion')
|
||||||
.it('should bind [style] as input to directive', () => {
|
.it('should bind [style] as input to directive', () => {
|
||||||
@ -1113,7 +1159,7 @@ describe('styling', () => {
|
|||||||
// instruction. We don't think this is worth it, and we are just going to live
|
// instruction. We don't think this is worth it, and we are just going to live
|
||||||
// with this.
|
// with this.
|
||||||
);
|
);
|
||||||
expect(capturedClassBindingValue).toEqual(null);
|
expect(capturedClassBindingValue !).toEqual('static-val');
|
||||||
expect(capturedMyClassBindingCount).toEqual(1);
|
expect(capturedMyClassBindingCount).toEqual(1);
|
||||||
expect(capturedMyClassBindingValue !).toEqual('foo');
|
expect(capturedMyClassBindingValue !).toEqual('foo');
|
||||||
|
|
||||||
|
Loading…
x
Reference in New Issue
Block a user