diff --git a/goldens/size-tracking/aio-payloads.json b/goldens/size-tracking/aio-payloads.json index f92c20b32d..716bf6c5e4 100755 --- a/goldens/size-tracking/aio-payloads.json +++ b/goldens/size-tracking/aio-payloads.json @@ -12,7 +12,7 @@ "master": { "uncompressed": { "runtime-es2015": 3037, - "main-es2015": 448922, + "main-es2015": 449483, "polyfills-es2015": 52415 } } diff --git a/packages/core/src/render3/node_manipulation.ts b/packages/core/src/render3/node_manipulation.ts index 26dac17c43..a08fbce88c 100644 --- a/packages/core/src/render3/node_manipulation.ts +++ b/packages/core/src/render3/node_manipulation.ts @@ -987,6 +987,13 @@ function applyContainer( } } +// TODO(misko): Can't import RendererStyleFlags2.DashCase as it causes imports to be resolved +// in different order which causes failures. Duplicating for now. +const enum TempRendererStyleFlags2 { + Important = 1 << 0, + DashCase = 1 << 1 +} + /** * Writes class/style to element. * @@ -1019,9 +1026,7 @@ export function applyStyling( } } } else { - // TODO(misko): Can't import RendererStyleFlags2.DashCase as it causes imports to be resolved - // in different order which causes failures. Using direct constant as workaround for now. - const flags = prop.indexOf('-') == -1 ? undefined : 2 /* RendererStyleFlags2.DashCase */; + let flags = prop.indexOf('-') === -1 ? undefined : TempRendererStyleFlags2.DashCase as number; if (value == null /** || value === undefined */) { ngDevMode && ngDevMode.rendererRemoveStyle++; if (isProcedural) { @@ -1030,12 +1035,22 @@ export function applyStyling( (rNode as HTMLElement).style.removeProperty(prop); } } else { + // A value is important if it ends with `!important`. The style + // parser strips any semicolons at the end of the value. + const isImportant = typeof value === 'string' ? value.endsWith('!important') : false; + + if (isImportant) { + // !important has to be stripped from the value for it to be valid. + value = value.slice(0, -10); + flags! |= TempRendererStyleFlags2.Important; + } + ngDevMode && ngDevMode.rendererSetStyle++; if (isProcedural) { (renderer as Renderer2).setStyle(rNode, prop, value, flags); } else { ngDevMode && assertDefined((rNode as HTMLElement).style, 'HTMLElement expected'); - (rNode as HTMLElement).style.setProperty(prop, value); + (rNode as HTMLElement).style.setProperty(prop, value, isImportant ? 'important' : ''); } } } diff --git a/packages/core/test/acceptance/styling_spec.ts b/packages/core/test/acceptance/styling_spec.ts index df12c82c9b..e6ac22da21 100644 --- a/packages/core/test/acceptance/styling_spec.ts +++ b/packages/core/test/acceptance/styling_spec.ts @@ -12,6 +12,7 @@ import {ngDevModeResetPerfCounters} from '@angular/core/src/util/ng_dev_mode'; import {TestBed} from '@angular/core/testing'; import {getElementClasses, getElementStyles, getSortedClassName, getSortedStyle} from '@angular/core/testing/src/styling'; import {By, DomSanitizer, SafeStyle} from '@angular/platform-browser'; +import {browserDetection} from '@angular/platform-browser/testing/src/browser_util'; import {expect} from '@angular/platform-browser/testing/src/matchers'; import {ivyEnabled, modifiedInIvy, onlyInIvy} from '@angular/private/testing'; @@ -801,6 +802,154 @@ describe('styling', () => { }); }); + onlyInIvy('ViewEngine only supports !important when set through the `style` attribute') + .it('should set !important on a single property', () => { + // We go through `CSSStyleDeclaration.setProperty` to set `!important`. Values set through + // `setProperty` aren't propagated to the `style` attribute on IE11, even though the style + // is applied, so we have to skip the test there since we don't have a way of asserting. + if (browserDetection.isIE) { + return; + } + + @Component({template: '
'}) + class Cmp { + width!: string; + } + + TestBed.configureTestingModule({declarations: [Cmp]}); + const fixture = TestBed.createComponent(Cmp); + fixture.componentInstance.width = '50px !important'; + fixture.detectChanges(); + const html = fixture.nativeElement.innerHTML; + + // We have to check the `style` attribute, because `element.style.prop` doesn't include + // `!important`. Use a regex, because the different renderers produce different whitespace. + expect(html).toMatch(/style=["|']width:\s*50px\s*!important/); + + onlyInIvy('perf counters').expectPerfCounters({ + rendererSetStyle: 1, + tNode: 2, + }); + }); + + onlyInIvy('ViewEngine only supports !important when set through the `style` attribute') + .it('should set !important that is not preceded by a space', () => { + // We go through `CSSStyleDeclaration.setProperty` to set `!important`. Values set through + // `setProperty` aren't propagated to the `style` attribute on IE11, even though the style + // is applied, so we have to skip the test there since we don't have a way of asserting. + if (browserDetection.isIE) { + return; + } + + @Component({template: ''}) + class Cmp { + width!: string; + } + + TestBed.configureTestingModule({declarations: [Cmp]}); + const fixture = TestBed.createComponent(Cmp); + fixture.componentInstance.width = '50px!important'; + fixture.detectChanges(); + const html = fixture.nativeElement.innerHTML; + + // We have to check the `style` attribute, because `element.style.prop` doesn't include + // `!important`. Use a regex, because the different renderers produce different whitespace. + expect(html).toMatch(/style=["|']width:\s*50px\s*!important/); + + onlyInIvy('perf counters').expectPerfCounters({ + rendererSetStyle: 1, + tNode: 2, + }); + }); + + onlyInIvy('ViewEngine only supports !important when set through the `style` attribute') + .it('should set !important on a dash-case property', () => { + // We go through `CSSStyleDeclaration.setProperty` to set `!important`. Values set through + // `setProperty` aren't propagated to the `style` attribute on IE11, even though the style + // is applied, so we have to skip the test there since we don't have a way of asserting. + if (browserDetection.isIE) { + return; + } + + @Component({template: ''}) + class Cmp { + marginRight!: string; + } + + TestBed.configureTestingModule({declarations: [Cmp]}); + const fixture = TestBed.createComponent(Cmp); + fixture.componentInstance.marginRight = '5px !important'; + fixture.detectChanges(); + const html = fixture.nativeElement.innerHTML; + + // We have to check the `style` attribute, because `element.style.prop` doesn't include + // `!important`. Use a regex, because the different renderers produce different whitespace. + expect(html).toMatch(/style=["|']margin-right:\s*5px\s*!important/); + + onlyInIvy('perf counters').expectPerfCounters({ + rendererSetStyle: 1, + tNode: 2, + }); + }); + + it('should set !important on multiple properties', () => { + // We go through `CSSStyleDeclaration.setProperty` to set `!important`. Values set through + // `setProperty` aren't propagated to the `style` attribute on IE11, even though the style is + // applied, so we have to skip the test there since we don't have a way of asserting. + if (browserDetection.isIE) { + return; + } + + @Component({template: ''}) + class Cmp { + styles!: string; + } + + TestBed.configureTestingModule({declarations: [Cmp]}); + const fixture = TestBed.createComponent(Cmp); + fixture.componentInstance.styles = 'height: 25px !important; width: 50px !important;'; + fixture.detectChanges(); + const html = fixture.nativeElement.innerHTML; + + // We have to check the `style` attribute, because `element.style.prop` doesn't include + // `!important`. Use a regex, because the different renderers produce different whitespace. + expect(html).toMatch(/style=["|']height:\s*25px\s*!important;\s*width:\s*50px\s*!important/); + + onlyInIvy('perf counters').expectPerfCounters({ + rendererSetStyle: 2, + tNode: 2, + }); + }); + + it('should set !important if some properties are !important and other are not', () => { + // We go through `CSSStyleDeclaration.setProperty` to set `!important`. Values set through + // `setProperty` aren't propagated to the `style` attribute on IE11, even though the style is + // applied, so we have to skip the test there since we don't have a way of asserting. + if (browserDetection.isIE) { + return; + } + + @Component({template: ''}) + class Cmp { + styles!: string; + } + + TestBed.configureTestingModule({declarations: [Cmp]}); + const fixture = TestBed.createComponent(Cmp); + fixture.componentInstance.styles = 'height: 25px; width: 50px !important;'; + fixture.detectChanges(); + const html = fixture.nativeElement.innerHTML; + + // We have to check the `style` attribute, because `element.style.prop` doesn't include + // `!important`. Use a regex, because the different renderers produce different whitespace. + expect(html).toMatch(/style=["|']height:\s*25px;\s*width:\s*50px\s*!important/); + + onlyInIvy('perf counters').expectPerfCounters({ + rendererSetStyle: 2, + tNode: 2, + }); + }); + it('should not write to the native element if a directive shadows the class input', () => { // This ex is a bit contrived. In real apps, you might have a shared class that is extended // both by components with host elements and by directives on template nodes. In that case, the diff --git a/packages/platform-browser/src/dom/dom_renderer.ts b/packages/platform-browser/src/dom/dom_renderer.ts index 33d0bc67bc..b9f8e66cf4 100644 --- a/packages/platform-browser/src/dom/dom_renderer.ts +++ b/packages/platform-browser/src/dom/dom_renderer.ts @@ -234,9 +234,8 @@ class DefaultDomRenderer2 implements Renderer2 { } setStyle(el: any, style: string, value: any, flags: RendererStyleFlags2): void { - if (flags & RendererStyleFlags2.DashCase) { - el.style.setProperty( - style, value, !!(flags & RendererStyleFlags2.Important) ? 'important' : ''); + if (flags & (RendererStyleFlags2.DashCase | RendererStyleFlags2.Important)) { + el.style.setProperty(style, value, flags & RendererStyleFlags2.Important ? 'important' : ''); } else { el.style[style] = value; } diff --git a/packages/platform-server/src/server_renderer.ts b/packages/platform-server/src/server_renderer.ts index fb6334ad67..33f21fa4ab 100644 --- a/packages/platform-server/src/server_renderer.ts +++ b/packages/platform-server/src/server_renderer.ts @@ -160,6 +160,9 @@ class DefaultServerRenderer2 implements Renderer2 { setStyle(el: any, style: string, value: any, flags: RendererStyleFlags2): void { style = style.replace(/([a-z])([A-Z])/g, '$1-$2').toLowerCase(); const styleMap = _readStyleAttribute(el); + if (flags & RendererStyleFlags2.Important) { + value += ' !important'; + } styleMap[style] = value == null ? '' : value; _writeStyleAttribute(el, styleMap); }