From c207ad80fd64bf5d10d457d37e757b85d328997d Mon Sep 17 00:00:00 2001 From: Kara Erickson Date: Fri, 30 Aug 2019 10:16:20 -0700 Subject: [PATCH] refactor(core): move DomAdapter style methods to ServerRenderer (#32408) PR Close #32408 --- packages/common/src/dom_adapter.ts | 5 -- packages/core/test/dom/dom_adapter_spec.ts | 21 -------- packages/core/test/linker/integration_spec.ts | 6 +-- .../test/linker/security_integration_spec.ts | 4 +- packages/core/test/view/element_spec.ts | 4 +- .../src/browser/browser_adapter.ts | 10 ---- .../platform-server/src/domino_adapter.ts | 49 ------------------- .../platform-server/src/server_renderer.ts | 40 ++++++++++++++- .../src/web_workers/worker/worker_adapter.ts | 3 -- .../worker/renderer_v2_integration_spec.ts | 4 +- 10 files changed, 46 insertions(+), 100 deletions(-) diff --git a/packages/common/src/dom_adapter.ts b/packages/common/src/dom_adapter.ts index 2f6bb80277..e362b5dc68 100644 --- a/packages/common/src/dom_adapter.ts +++ b/packages/common/src/dom_adapter.ts @@ -50,11 +50,6 @@ export abstract class DomAdapter { abstract createHtmlDocument(): HTMLDocument; abstract getDefaultDocument(): Document; - // Used by platform-server - abstract getStyle(element: any, styleName: string): any; - abstract setStyle(element: any, styleName: string, styleValue: string): any; - abstract removeStyle(element: any, styleName: string): any; - // Used by Title abstract getTitle(doc: Document): string; abstract setTitle(doc: Document, newTitle: string): any; diff --git a/packages/core/test/dom/dom_adapter_spec.ts b/packages/core/test/dom/dom_adapter_spec.ts index 5ddf254170..0493c10ee6 100644 --- a/packages/core/test/dom/dom_adapter_spec.ts +++ b/packages/core/test/dom/dom_adapter_spec.ts @@ -36,27 +36,6 @@ import {isTextNode} from '@angular/platform-browser/testing/src/browser_util'; expect(() => getDOM().remove(d)).not.toThrow(); }); - it('should parse styles with urls correctly', () => { - const d = getDOM().createElement('div'); - getDOM().setStyle(d, 'background-url', 'url(http://test.com/bg.jpg)'); - expect(getDOM().getStyle(d, 'background-url')).toBe('url(http://test.com/bg.jpg)'); - }); - - // Test for regression caused by angular/angular#22536 - it('should parse styles correctly following the spec', () => { - const d = getDOM().createElement('div'); - getDOM().setStyle(d, 'background-image', 'url("paper.gif")'); - expect(d.style.backgroundImage).toBe('url("paper.gif")'); - expect(d.style.getPropertyValue('background-image')).toBe('url("paper.gif")'); - expect(getDOM().getStyle(d, 'background-image')).toBe('url("paper.gif")'); - }); - - it('should parse camel-case styles correctly', () => { - const d = getDOM().createElement('div'); - getDOM().setStyle(d, 'marginRight', '10px'); - expect(getDOM().getStyle(d, 'margin-right')).toBe('10px'); - }); - if (getDOM().supportsDOMEvents()) { describe('getBaseHref', () => { beforeEach(() => getDOM().resetBaseElement()); diff --git a/packages/core/test/linker/integration_spec.ts b/packages/core/test/linker/integration_spec.ts index 19d01ced8b..999a4cd025 100644 --- a/packages/core/test/linker/integration_spec.ts +++ b/packages/core/test/linker/integration_spec.ts @@ -147,13 +147,11 @@ function declareTests(config?: {useJit: boolean}) { fixture.componentInstance.ctxProp = '10'; fixture.detectChanges(); - expect(getDOM().getStyle(fixture.debugElement.children[0].nativeElement, 'height')) - .toEqual('10px'); + expect(fixture.debugElement.children[0].nativeElement.style['height']).toEqual('10px'); fixture.componentInstance.ctxProp = null !; fixture.detectChanges(); - expect(getDOM().getStyle(fixture.debugElement.children[0].nativeElement, 'height')) - .toEqual(''); + expect(fixture.debugElement.children[0].nativeElement.style['height']).toEqual(''); }); it('should consume binding to property names where attr name and property name do not match', diff --git a/packages/core/test/linker/security_integration_spec.ts b/packages/core/test/linker/security_integration_spec.ts index 6806f0224f..ff3cc0a3ff 100644 --- a/packages/core/test/linker/security_integration_spec.ts +++ b/packages/core/test/linker/security_integration_spec.ts @@ -247,12 +247,12 @@ function declareTests(config?: {useJit: boolean}) { fixture.detectChanges(); // In some browsers, this will contain the full background specification, not just // the color. - expect(getDOM().getStyle(e, 'background')).toMatch(/red.*/); + expect(e.style['background']).toMatch(/red.*/); ci.ctxProp = 'url(javascript:evil())'; fixture.detectChanges(); // Updated value gets rejected, no value change. - expect(getDOM().getStyle(e, 'background')).not.toContain('javascript'); + expect(e.style['background']).not.toContain('javascript'); }); modifiedInIvy('Unknown property error thrown during update mode, not creation mode') diff --git a/packages/core/test/view/element_spec.ts b/packages/core/test/view/element_spec.ts index ab2691054b..94789ff071 100644 --- a/packages/core/test/view/element_spec.ts +++ b/packages/core/test/view/element_spec.ts @@ -164,8 +164,8 @@ const removeEventListener = '__zone_symbol__removeEventListener' as 'removeEvent Services.checkAndUpdateView(view); const el = rootNodes[0]; - expect(getDOM().getStyle(el, 'width')).toBe('10px'); - expect(getDOM().getStyle(el, 'color')).toBe('red'); + expect(el.style['width']).toBe('10px'); + expect(el.style['color']).toBe('red'); }); }); }); diff --git a/packages/platform-browser/src/browser/browser_adapter.ts b/packages/platform-browser/src/browser/browser_adapter.ts index c898c07a16..00f44b1a2b 100644 --- a/packages/platform-browser/src/browser/browser_adapter.ts +++ b/packages/platform-browser/src/browser/browser_adapter.ts @@ -119,16 +119,6 @@ export class BrowserDomAdapter extends GenericBrowserDomAdapter { getElementsByTagName(element: any, name: string): HTMLElement[] { return element.getElementsByTagName(name); } - setStyle(element: any, styleName: string, styleValue: string) { - element.style[styleName] = styleValue; - } - removeStyle(element: any, stylename: string) { - // IE requires '' instead of null - // see https://github.com/angular/angular/issues/7916 - element.style[stylename] = ''; - } - - getStyle(element: any, stylename: string): string { return element.style[stylename]; } getAttribute(element: Element, attribute: string): string|null { return element.getAttribute(attribute); diff --git a/packages/platform-server/src/domino_adapter.ts b/packages/platform-server/src/domino_adapter.ts index c9d9e70c15..f1cac8a047 100644 --- a/packages/platform-server/src/domino_adapter.ts +++ b/packages/platform-server/src/domino_adapter.ts @@ -109,55 +109,6 @@ export class DominoAdapter extends BrowserDomAdapter { return href; } - /** @internal */ - _readStyleAttribute(element: any): {[name: string]: string} { - const styleMap: {[name: string]: string} = {}; - const styleAttribute = element.getAttribute('style'); - if (styleAttribute) { - const styleList = styleAttribute.split(/;+/g); - for (let i = 0; i < styleList.length; i++) { - const style = styleList[i].trim(); - if (style.length > 0) { - const colonIndex = style.indexOf(':'); - if (colonIndex === -1) { - throw new Error(`Invalid CSS style: ${style}`); - } - const name = style.substr(0, colonIndex).trim(); - styleMap[name] = style.substr(colonIndex + 1).trim(); - } - } - } - return styleMap; - } - /** @internal */ - _writeStyleAttribute(element: any, styleMap: {[name: string]: string}) { - let styleAttrValue = ''; - for (const key in styleMap) { - const newValue = styleMap[key]; - if (newValue) { - styleAttrValue += key + ':' + styleMap[key] + ';'; - } - } - element.setAttribute('style', styleAttrValue); - } - - setStyle(element: any, styleName: string, styleValue?: string|null) { - styleName = styleName.replace(/([a-z])([A-Z])/g, '$1-$2').toLowerCase(); - const styleMap = this._readStyleAttribute(element); - styleMap[styleName] = styleValue || ''; - this._writeStyleAttribute(element, styleMap); - } - removeStyle(element: any, styleName: string) { - // IE requires '' instead of null - // see https://github.com/angular/angular/issues/7916 - this.setStyle(element, styleName, ''); - } - - getStyle(element: any, styleName: string): string { - const styleMap = this._readStyleAttribute(element); - return styleMap[styleName] || ''; - } - dispatchEvent(el: Node, evt: any) { el.dispatchEvent(evt); diff --git a/packages/platform-server/src/server_renderer.ts b/packages/platform-server/src/server_renderer.ts index fa751de848..a9d8147bf3 100644 --- a/packages/platform-server/src/server_renderer.ts +++ b/packages/platform-server/src/server_renderer.ts @@ -143,11 +143,16 @@ class DefaultServerRenderer2 implements Renderer2 { removeClass(el: any, name: string): void { el.classList.remove(name); } setStyle(el: any, style: string, value: any, flags: RendererStyleFlags2): void { - getDOM().setStyle(el, style, value); + style = style.replace(/([a-z])([A-Z])/g, '$1-$2').toLowerCase(); + const styleMap = _readStyleAttribute(el); + styleMap[style] = value || ''; + _writeStyleAttribute(el, styleMap); } removeStyle(el: any, style: string, flags: RendererStyleFlags2): void { - getDOM().removeStyle(el, style); + // IE requires '' instead of null + // see https://github.com/angular/angular/issues/7916 + this.setStyle(el, style, '', flags); } // The value was validated already as a property binding, against the property name. @@ -246,3 +251,34 @@ class EmulatedEncapsulationServerRenderer2 extends DefaultServerRenderer2 { return el; } } + +function _readStyleAttribute(element: any): {[name: string]: string} { + const styleMap: {[name: string]: string} = {}; + const styleAttribute = element.getAttribute('style'); + if (styleAttribute) { + const styleList = styleAttribute.split(/;+/g); + for (let i = 0; i < styleList.length; i++) { + const style = styleList[i].trim(); + if (style.length > 0) { + const colonIndex = style.indexOf(':'); + if (colonIndex === -1) { + throw new Error(`Invalid CSS style: ${style}`); + } + const name = style.substr(0, colonIndex).trim(); + styleMap[name] = style.substr(colonIndex + 1).trim(); + } + } + } + return styleMap; +} + +function _writeStyleAttribute(element: any, styleMap: {[name: string]: string}) { + let styleAttrValue = ''; + for (const key in styleMap) { + const newValue = styleMap[key]; + if (newValue) { + styleAttrValue += key + ':' + styleMap[key] + ';'; + } + } + element.setAttribute('style', styleAttrValue); +} diff --git a/packages/platform-webworker/src/web_workers/worker/worker_adapter.ts b/packages/platform-webworker/src/web_workers/worker/worker_adapter.ts index 942bb3c5d8..4a7bffae8c 100644 --- a/packages/platform-webworker/src/web_workers/worker/worker_adapter.ts +++ b/packages/platform-webworker/src/web_workers/worker/worker_adapter.ts @@ -53,9 +53,6 @@ export class WorkerDomAdapter extends DomAdapter { createElement(tagName: any, doc?: any): HTMLElement { throw 'not implemented'; } getHost(el: any): any { throw 'not implemented'; } getElementsByTagName(element: any, name: string): HTMLElement[] { throw 'not implemented'; } - setStyle(element: any, styleName: string, styleValue: string) { throw 'not implemented'; } - removeStyle(element: any, styleName: string) { throw 'not implemented'; } - getStyle(element: any, styleName: string): string { throw 'not implemented'; } getAttribute(element: any, attribute: string): string { throw 'not implemented'; } setAttribute(element: any, name: string, value: string) { throw 'not implemented'; } createHtmlDocument(): HTMLDocument { throw 'not implemented'; } diff --git a/packages/platform-webworker/test/web_workers/worker/renderer_v2_integration_spec.ts b/packages/platform-webworker/test/web_workers/worker/renderer_v2_integration_spec.ts index b2fbe8c1dc..db9ab01ecb 100644 --- a/packages/platform-webworker/test/web_workers/worker/renderer_v2_integration_spec.ts +++ b/packages/platform-webworker/test/web_workers/worker/renderer_v2_integration_spec.ts @@ -115,10 +115,10 @@ let lastCreatedRenderer: Renderer2; expect(hasClass(el, 'a')).toBe(false); lastCreatedRenderer.setStyle(workerEl, 'width', '10px'); - expect(getDOM().getStyle(el, 'width')).toEqual('10px'); + expect(el.style['width']).toEqual('10px'); lastCreatedRenderer.removeStyle(workerEl, 'width'); - expect(getDOM().getStyle(el, 'width')).toEqual(''); + expect(el.style['width']).toEqual(''); lastCreatedRenderer.setAttribute(workerEl, 'someattr', 'someValue'); expect(getDOM().getAttribute(el, 'someattr')).toEqual('someValue');