report: allow CRC to use renderTextUrl #9237
Conversation
There was a problem hiding this comment.
I started to review this and then realized you just wanted to merge into @connorjclark 's PR, soooo feel free to ignore.
| * @return {Node} | ||
| */ | ||
| static createChainNode(dom, tmpl, segment) { | ||
| static createChainNode(dom, tmpl, segment, detailsRenderer) { |
There was a problem hiding this comment.
this definitely feels weird and passing this to anything smells, but I don't really have a better suggestion that doesn't involve totally refactoring how details renderer is structured, sooooooo. sure, why not :)
There was a problem hiding this comment.
maybe this class should extend DR? but i agree on deferring, this shouldnt be handled within this pr
| @@ -182,7 +183,7 @@ class CriticalRequestChainRenderer { | |||
| for (const key of Object.keys(root.tree)) { | |||
| const segment = CriticalRequestChainRenderer.createSegment(root.tree, key, | |||
| root.startTime, root.transferSize); | |||
| CriticalRequestChainRenderer.buildTree(dom, tmpl, segment, containerEl, details); | |||
| CriticalRequestChainRenderer.buildTree(dom, tmpl, segment, containerEl, details, detRenderer); | |||
There was a problem hiding this comment.
det !? we at least gotta go with deets 😆
is this just coming up on 100 then?
|
|
detailsRendererwas first used in snippetRenderer, so I figured why not.