Skip to content

Commit c207ad8

Browse files
karamhevery
authored andcommitted
refactor(core): move DomAdapter style methods to ServerRenderer (angular#32408)
PR Close angular#32408
1 parent 970b58b commit c207ad8

File tree

10 files changed

+46
-100
lines changed

10 files changed

+46
-100
lines changed

packages/common/src/dom_adapter.ts

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -50,11 +50,6 @@ export abstract class DomAdapter {
5050
abstract createHtmlDocument(): HTMLDocument;
5151
abstract getDefaultDocument(): Document;
5252

53-
// Used by platform-server
54-
abstract getStyle(element: any, styleName: string): any;
55-
abstract setStyle(element: any, styleName: string, styleValue: string): any;
56-
abstract removeStyle(element: any, styleName: string): any;
57-
5853
// Used by Title
5954
abstract getTitle(doc: Document): string;
6055
abstract setTitle(doc: Document, newTitle: string): any;

packages/core/test/dom/dom_adapter_spec.ts

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -36,27 +36,6 @@ import {isTextNode} from '@angular/platform-browser/testing/src/browser_util';
3636
expect(() => getDOM().remove(d)).not.toThrow();
3737
});
3838

39-
it('should parse styles with urls correctly', () => {
40-
const d = getDOM().createElement('div');
41-
getDOM().setStyle(d, 'background-url', 'url(http://test.com/bg.jpg)');
42-
expect(getDOM().getStyle(d, 'background-url')).toBe('url(http://test.com/bg.jpg)');
43-
});
44-
45-
// Test for regression caused by angular/angular#22536
46-
it('should parse styles correctly following the spec', () => {
47-
const d = getDOM().createElement('div');
48-
getDOM().setStyle(d, 'background-image', 'url("paper.gif")');
49-
expect(d.style.backgroundImage).toBe('url("paper.gif")');
50-
expect(d.style.getPropertyValue('background-image')).toBe('url("paper.gif")');
51-
expect(getDOM().getStyle(d, 'background-image')).toBe('url("paper.gif")');
52-
});
53-
54-
it('should parse camel-case styles correctly', () => {
55-
const d = getDOM().createElement('div');
56-
getDOM().setStyle(d, 'marginRight', '10px');
57-
expect(getDOM().getStyle(d, 'margin-right')).toBe('10px');
58-
});
59-
6039
if (getDOM().supportsDOMEvents()) {
6140
describe('getBaseHref', () => {
6241
beforeEach(() => getDOM().resetBaseElement());

packages/core/test/linker/integration_spec.ts

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -147,13 +147,11 @@ function declareTests(config?: {useJit: boolean}) {
147147

148148
fixture.componentInstance.ctxProp = '10';
149149
fixture.detectChanges();
150-
expect(getDOM().getStyle(fixture.debugElement.children[0].nativeElement, 'height'))
151-
.toEqual('10px');
150+
expect(fixture.debugElement.children[0].nativeElement.style['height']).toEqual('10px');
152151

153152
fixture.componentInstance.ctxProp = null !;
154153
fixture.detectChanges();
155-
expect(getDOM().getStyle(fixture.debugElement.children[0].nativeElement, 'height'))
156-
.toEqual('');
154+
expect(fixture.debugElement.children[0].nativeElement.style['height']).toEqual('');
157155
});
158156

159157
it('should consume binding to property names where attr name and property name do not match',

packages/core/test/linker/security_integration_spec.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -247,12 +247,12 @@ function declareTests(config?: {useJit: boolean}) {
247247
fixture.detectChanges();
248248
// In some browsers, this will contain the full background specification, not just
249249
// the color.
250-
expect(getDOM().getStyle(e, 'background')).toMatch(/red.*/);
250+
expect(e.style['background']).toMatch(/red.*/);
251251

252252
ci.ctxProp = 'url(javascript:evil())';
253253
fixture.detectChanges();
254254
// Updated value gets rejected, no value change.
255-
expect(getDOM().getStyle(e, 'background')).not.toContain('javascript');
255+
expect(e.style['background']).not.toContain('javascript');
256256
});
257257

258258
modifiedInIvy('Unknown property error thrown during update mode, not creation mode')

packages/core/test/view/element_spec.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -164,8 +164,8 @@ const removeEventListener = '__zone_symbol__removeEventListener' as 'removeEvent
164164
Services.checkAndUpdateView(view);
165165

166166
const el = rootNodes[0];
167-
expect(getDOM().getStyle(el, 'width')).toBe('10px');
168-
expect(getDOM().getStyle(el, 'color')).toBe('red');
167+
expect(el.style['width']).toBe('10px');
168+
expect(el.style['color']).toBe('red');
169169
});
170170
});
171171
});

packages/platform-browser/src/browser/browser_adapter.ts

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -119,16 +119,6 @@ export class BrowserDomAdapter extends GenericBrowserDomAdapter {
119119
getElementsByTagName(element: any, name: string): HTMLElement[] {
120120
return element.getElementsByTagName(name);
121121
}
122-
setStyle(element: any, styleName: string, styleValue: string) {
123-
element.style[styleName] = styleValue;
124-
}
125-
removeStyle(element: any, stylename: string) {
126-
// IE requires '' instead of null
127-
// see https://github.com/angular/angular/issues/7916
128-
element.style[stylename] = '';
129-
}
130-
131-
getStyle(element: any, stylename: string): string { return element.style[stylename]; }
132122

133123
getAttribute(element: Element, attribute: string): string|null {
134124
return element.getAttribute(attribute);

packages/platform-server/src/domino_adapter.ts

Lines changed: 0 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -109,55 +109,6 @@ export class DominoAdapter extends BrowserDomAdapter {
109109
return href;
110110
}
111111

112-
/** @internal */
113-
_readStyleAttribute(element: any): {[name: string]: string} {
114-
const styleMap: {[name: string]: string} = {};
115-
const styleAttribute = element.getAttribute('style');
116-
if (styleAttribute) {
117-
const styleList = styleAttribute.split(/;+/g);
118-
for (let i = 0; i < styleList.length; i++) {
119-
const style = styleList[i].trim();
120-
if (style.length > 0) {
121-
const colonIndex = style.indexOf(':');
122-
if (colonIndex === -1) {
123-
throw new Error(`Invalid CSS style: ${style}`);
124-
}
125-
const name = style.substr(0, colonIndex).trim();
126-
styleMap[name] = style.substr(colonIndex + 1).trim();
127-
}
128-
}
129-
}
130-
return styleMap;
131-
}
132-
/** @internal */
133-
_writeStyleAttribute(element: any, styleMap: {[name: string]: string}) {
134-
let styleAttrValue = '';
135-
for (const key in styleMap) {
136-
const newValue = styleMap[key];
137-
if (newValue) {
138-
styleAttrValue += key + ':' + styleMap[key] + ';';
139-
}
140-
}
141-
element.setAttribute('style', styleAttrValue);
142-
}
143-
144-
setStyle(element: any, styleName: string, styleValue?: string|null) {
145-
styleName = styleName.replace(/([a-z])([A-Z])/g, '$1-$2').toLowerCase();
146-
const styleMap = this._readStyleAttribute(element);
147-
styleMap[styleName] = styleValue || '';
148-
this._writeStyleAttribute(element, styleMap);
149-
}
150-
removeStyle(element: any, styleName: string) {
151-
// IE requires '' instead of null
152-
// see https://github.com/angular/angular/issues/7916
153-
this.setStyle(element, styleName, '');
154-
}
155-
156-
getStyle(element: any, styleName: string): string {
157-
const styleMap = this._readStyleAttribute(element);
158-
return styleMap[styleName] || '';
159-
}
160-
161112
dispatchEvent(el: Node, evt: any) {
162113
el.dispatchEvent(evt);
163114

packages/platform-server/src/server_renderer.ts

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -143,11 +143,16 @@ class DefaultServerRenderer2 implements Renderer2 {
143143
removeClass(el: any, name: string): void { el.classList.remove(name); }
144144

145145
setStyle(el: any, style: string, value: any, flags: RendererStyleFlags2): void {
146-
getDOM().setStyle(el, style, value);
146+
style = style.replace(/([a-z])([A-Z])/g, '$1-$2').toLowerCase();
147+
const styleMap = _readStyleAttribute(el);
148+
styleMap[style] = value || '';
149+
_writeStyleAttribute(el, styleMap);
147150
}
148151

149152
removeStyle(el: any, style: string, flags: RendererStyleFlags2): void {
150-
getDOM().removeStyle(el, style);
153+
// IE requires '' instead of null
154+
// see https://github.com/angular/angular/issues/7916
155+
this.setStyle(el, style, '', flags);
151156
}
152157

153158
// The value was validated already as a property binding, against the property name.
@@ -246,3 +251,34 @@ class EmulatedEncapsulationServerRenderer2 extends DefaultServerRenderer2 {
246251
return el;
247252
}
248253
}
254+
255+
function _readStyleAttribute(element: any): {[name: string]: string} {
256+
const styleMap: {[name: string]: string} = {};
257+
const styleAttribute = element.getAttribute('style');
258+
if (styleAttribute) {
259+
const styleList = styleAttribute.split(/;+/g);
260+
for (let i = 0; i < styleList.length; i++) {
261+
const style = styleList[i].trim();
262+
if (style.length > 0) {
263+
const colonIndex = style.indexOf(':');
264+
if (colonIndex === -1) {
265+
throw new Error(`Invalid CSS style: ${style}`);
266+
}
267+
const name = style.substr(0, colonIndex).trim();
268+
styleMap[name] = style.substr(colonIndex + 1).trim();
269+
}
270+
}
271+
}
272+
return styleMap;
273+
}
274+
275+
function _writeStyleAttribute(element: any, styleMap: {[name: string]: string}) {
276+
let styleAttrValue = '';
277+
for (const key in styleMap) {
278+
const newValue = styleMap[key];
279+
if (newValue) {
280+
styleAttrValue += key + ':' + styleMap[key] + ';';
281+
}
282+
}
283+
element.setAttribute('style', styleAttrValue);
284+
}

packages/platform-webworker/src/web_workers/worker/worker_adapter.ts

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,9 +53,6 @@ export class WorkerDomAdapter extends DomAdapter {
5353
createElement(tagName: any, doc?: any): HTMLElement { throw 'not implemented'; }
5454
getHost(el: any): any { throw 'not implemented'; }
5555
getElementsByTagName(element: any, name: string): HTMLElement[] { throw 'not implemented'; }
56-
setStyle(element: any, styleName: string, styleValue: string) { throw 'not implemented'; }
57-
removeStyle(element: any, styleName: string) { throw 'not implemented'; }
58-
getStyle(element: any, styleName: string): string { throw 'not implemented'; }
5956
getAttribute(element: any, attribute: string): string { throw 'not implemented'; }
6057
setAttribute(element: any, name: string, value: string) { throw 'not implemented'; }
6158
createHtmlDocument(): HTMLDocument { throw 'not implemented'; }

packages/platform-webworker/test/web_workers/worker/renderer_v2_integration_spec.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -115,10 +115,10 @@ let lastCreatedRenderer: Renderer2;
115115
expect(hasClass(el, 'a')).toBe(false);
116116

117117
lastCreatedRenderer.setStyle(workerEl, 'width', '10px');
118-
expect(getDOM().getStyle(el, 'width')).toEqual('10px');
118+
expect(el.style['width']).toEqual('10px');
119119

120120
lastCreatedRenderer.removeStyle(workerEl, 'width');
121-
expect(getDOM().getStyle(el, 'width')).toEqual('');
121+
expect(el.style['width']).toEqual('');
122122

123123
lastCreatedRenderer.setAttribute(workerEl, 'someattr', 'someValue');
124124
expect(getDOM().getAttribute(el, 'someattr')).toEqual('someValue');

0 commit comments

Comments
 (0)