From 8c75f2155d6802a43982291037adc108fdf2db53 Mon Sep 17 00:00:00 2001 From: Misko Hevery Date: Tue, 11 Feb 2020 16:31:12 -0800 Subject: [PATCH] fix(core): correctly concatenate static and dynamic binding to `class` when shadowed (#35350) Given: ```
``` 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 `
` The new logic correctly handles falsy values for `[class]` bindings. Fix #35335 PR Close #35350 --- .../core/src/render3/instructions/styling.ts | 5 +- packages/core/test/acceptance/styling_spec.ts | 48 ++++++++++++++++++- 2 files changed, 50 insertions(+), 3 deletions(-) diff --git a/packages/core/src/render3/instructions/styling.ts b/packages/core/src/render3/instructions/styling.ts index c8d9e0ed67..4e042aa96c 100644 --- a/packages/core/src/render3/instructions/styling.ts +++ b/packages/core/src/render3/instructions/styling.ts @@ -261,8 +261,9 @@ export function checkStylingMap( ngDevMode && isClassBased === false && staticPrefix !== null && assertEqual( staticPrefix.endsWith(';'), true, 'Expecting static portion to end with \';\''); - if (typeof value === 'string') { - value = concatStringsWithSpace(staticPrefix, value as string); + if (staticPrefix !== null) { + // We want to make sure that falsy values of `value` become empty strings. + value = concatStringsWithSpace(staticPrefix, value ? value : ''); } // Given `
` such that `my-dir` has `@Input('style')`. // This takes over the `[style]` binding. (Same for `[class]`) diff --git a/packages/core/test/acceptance/styling_spec.ts b/packages/core/test/acceptance/styling_spec.ts index cee91e5718..9a9f1bef06 100644 --- a/packages/core/test/acceptance/styling_spec.ts +++ b/packages/core/test/acceptance/styling_spec.ts @@ -316,6 +316,52 @@ describe('styling', () => { 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: ` +
+ ` + }) + 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') .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 // with this. ); - expect(capturedClassBindingValue).toEqual(null); + expect(capturedClassBindingValue !).toEqual('static-val'); expect(capturedMyClassBindingCount).toEqual(1); expect(capturedMyClassBindingValue !).toEqual('foo');