fix(platform-server): add styles to elements correctly #22527
Conversation
|
| const styleMap = {}; | ||
| const styleAttribute = element.getAttribute('style'); | ||
| if (styleAttribute) { | ||
| const styleList = styleAttribute.split(/;+/g); |
There was a problem hiding this comment.
nit: what about /(\s*;\s*)+/g here ?
There was a problem hiding this comment.
oh would not work either so maybe
1- split on /;+/g
2- trim() the parts
3- ignore empty parts
There was a problem hiding this comment.
I changed the logic, let me know if that's what you had in mind
|
@vicb changed commit message and opened a new issue |
|
@vicb any updates on merging this PR? |
| if (styleAttribute) { | ||
| const styleList = styleAttribute.split(/;+/g); | ||
| for (let i = 0; i < styleList.length; i++) { | ||
| if (styleList[i].length > 0) { |
There was a problem hiding this comment.
triming before checking the length would probably make more sense ?
| if (!style) { | ||
| continue; | ||
| } | ||
| const colon = style.indexOf(':'); |
| if (colon === -1) { | ||
| throw new Error(`Invalid CSS style: ${style}`); | ||
| } | ||
| (styleMap as any)[style.substr(0, colon).trim()] = style.substr(colon + 1).trim(); |
There was a problem hiding this comment.
nit: extract const name = style.substr(0, colon).trim();
There was a problem hiding this comment.
there should be as any needed here
| @@ -135,17 +135,53 @@ export class DominoAdapter extends BrowserDomAdapter { | |||
| return href; | |||
| } | |||
|
|
|||
| /** @internal */ | |||
| _readStyleAttribute(element: any) { | |||
| const styleMap = {}; | |||
There was a problem hiding this comment.
const styleMap: {[name: string]: string} = {} and then remove un-needed "as any"
| @@ -135,17 +135,53 @@ export class DominoAdapter extends BrowserDomAdapter { | |||
| return href; | |||
| } | |||
|
|
|||
| /** @internal */ | |||
| _readStyleAttribute(element: any) { | |||
| setStyle(element: any, styleName: string, styleValue?: string|null) { | ||
| styleName = styleName.replace(/([a-z])([A-Z])/g, '$1-$2').toLowerCase(); | ||
| element.style[styleName] = styleValue; | ||
| const styleMap = this._readStyleAttribute(element); | ||
| (styleMap as any)[styleName] = styleValue; |
There was a problem hiding this comment.
no need for any if types are specified upstream
| } | ||
| getStyle(element: any, styleName: string): string { | ||
| return element.style[styleName] || element.style.getPropertyValue(styleName); | ||
| const styleMap = this._readStyleAttribute(element); | ||
| return styleMap.hasOwnProperty(styleName) ? (styleMap as any)[styleName] : ''; |
|
fb9f9ec to
7be1a4a
Compare
|
Should be |
| setStyle(element: any, styleName: string, styleValue?: string|null) { | ||
| styleName = styleName.replace(/([a-z])([A-Z])/g, '$1-$2').toLowerCase(); | ||
| element.style[styleName] = styleValue; | ||
| const styleMap = this._readStyleAttribute(element); | ||
| styleMap[styleName] = styleValue + ''; |
There was a problem hiding this comment.
do we really want "undefined" as a possible value ?
| } | ||
| getStyle(element: any, styleName: string): string { | ||
| return element.style[styleName] || element.style.getPropertyValue(styleName); | ||
| const styleMap = this._readStyleAttribute(element); | ||
| return styleMap.hasOwnProperty(styleName) ? styleMap[styleName] : ''; |
There was a problem hiding this comment.
return styleMap[styleName] || ''; ?
* Partially reverts angular#22263 due to lack of total spec compliance on the server * Maintains the camel-case styles fix PR Close angular#22527
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
on the server
Fixes #22536