Use existing sanistiser (html parser) as cleaner to track body clicks - #694
Conversation
|
PRbuilds results: 💚 AMP validation LightHouse Reporting --automated message |
9de0b1e to
d9dda3e
Compare
d1b9d26 to
d9dda3e
Compare
oliverlloyd
left a comment
There was a problem hiding this comment.
+1 for a hard earned, persistent effort in getting this PR through!
Would it be possible to replace escapeData.tsx with this lib?
| const sanitiserOptions = { | ||
| // Defaults: https://www.npmjs.com/package/sanitize-html#what-are-the-default-options | ||
| allowedTags: false, // Leave tags from CAPI alone | ||
| allowedAttributes: false, // Leave attributes from CAPI alone | ||
| transformTags: { | ||
| a: (tagName: string, attribs: {}) => ({ | ||
| tagName, // Just return anchors as is | ||
| attribs: { | ||
| ...attribs, // Merge into the existing attributes | ||
| ...{ | ||
| 'data-link-name': 'in body link', // Add the data-link-name for Ophan to anchors | ||
| }, | ||
| }, | ||
| }), | ||
| }, | ||
| }; | ||
|
|
There was a problem hiding this comment.
Not sure what the scopes or use cases here are but could this live in the sanitise-html lib? Or will the options be different each time? (Assuming there will be next times)
There was a problem hiding this comment.
Or, maybe we could have base options defined at definition and extend them by spreading locally at use?
There was a problem hiding this comment.
Would it be possible to replace
escapeData.tsxwith this lib?
Yeah maybe! Will put a card in to review.
Or will the options be different each time?
I think it's likely it will be sufficiently different each time that base options wouldn't be that useful. This is basically giving us all the info (work on a, keep the attributes, add data-link-name) local to the transformation which I guess is our preferred approach now.
What does this change?
We have the existing htmlSanitiser used in AMP. I've moved this to shared
libso that we can use it in web to adddata-link-nameto anchors. This is nice well-defined solution for modifying (read: cleaning) markup and uses htmlparser2 under the hood (which I was going to use anyway).Why?
We need to add this attribute for Ophan to track body links.
Link to supporting Trello card
https://trello.com/c/eagQlGEf/639-track-onward-journeys-in-ophan
@guardian/dotcom-platform