fix: sanitize data to match edge delivery markup#244
Conversation
| const baseUrl = getProductUrl(baseProduct, context); | ||
| if (Array.isArray(option.values)) { | ||
| option.values = option.values.map((value) => ({ | ||
| ...value, |
There was a problem hiding this comment.
Was this intentional removal of the spread? Figured you would just append title prop after if that's all you need.
There was a problem hiding this comment.
I wanted to make sure that all values are sanitized, and hence removed any "deep copy" / spread.
The template reads only the title and url anyway so that is what I kept.
sirugh
left a comment
There was a problem hiding this comment.
I will not be able to validate/test but code looks fine. One comment regarding a change away from spreading of value props, otherwise LGTM.
| if (!html) return html; | ||
|
|
||
| const allowedInlineTags = [ 'a', 'br', 'code', 'del', 'em', 'img', 'strong', 'sub', 'sup', 'u' ]; | ||
| const allowedAllTags = [ |
There was a problem hiding this comment.
const allowedAllTags = [
'p', 'h1', 'h2', 'h3', 'h4', 'h5', 'h6', 'ul', 'ol', 'li', 'pre',
'div', 'span', 'section', 'article',
'blockquote', 'cite', 'q',
'figure', 'figcaption',
...allowedInlineTags,
'table', 'tbody', 'td', 'th', 'thead', 'tr',
]; or we don't need all tags ?
There was a problem hiding this comment.
We only include what edge delivery supports https://www.aem.live/developer/markup-reference
Fixes #194