Flamegraph: Various clean-up in prep for d3.flameGraph update #215
Conversation
| @@ -1,14 +1,13 @@ | |||
| .d3-flame-graph rect { | |||
| stroke: #fff; | |||
| stroke: #EEEEEE; | |||
There was a problem hiding this comment.
Unknown property 'stroke'.
| fill-opacity: .8; | ||
| } | ||
|
|
||
| .d3-flame-graph .frame { | ||
| .d3-flame-graph rect:hover { | ||
| stroke: #474747; |
There was a problem hiding this comment.
Unknown property 'stroke'.
| .d3-flame-graph .frame { | ||
| .d3-flame-graph rect:hover { | ||
| stroke: #474747; | ||
| stroke-width: 0.5; |
There was a problem hiding this comment.
Unknown property 'stroke-width'.
| /* Use white stroke instead of grey, to make them invisible on our background */ | ||
| body .d3-flame-graph rect, | ||
| body .d3-flame-graph rect:hover { | ||
| stroke: #fff; |
There was a problem hiding this comment.
Unknown property 'stroke'.
| body .d3-flame-graph rect, | ||
| body .d3-flame-graph rect:hover { | ||
| stroke: #fff; | ||
| stroke-width: 1px; |
There was a problem hiding this comment.
Unknown property 'stroke-width'.
| background: #fff; | ||
| border-radius: 6px; | ||
| box-shadow: 0 5px 10px rgba(0, 0, 0, 0.2); | ||
| border: 1px solid rgba(0, 0, 0, 0.2); |
There was a problem hiding this comment.
Fallback border (hex or RGB) should precede RGBA border.
| } | ||
| /** | ||
| * PATCH(xhgui): Remove all .d3-flame-graph-tip styles | ||
| * We have our own tip styles in xhgui.css instead. |
There was a problem hiding this comment.
Do we need this comment long term?
There was a problem hiding this comment.
I think it'd make sense to keep yeah.
I'm working on another pull request that updates d3 to version 4, and updates d3-tip and d3-flameGraph. As part of such file-replacing commits, seeing the two PATCH comments in the removal diff is a reminder that the file was modified and needs to be re-modified the same way.
Ideally we wouldn't have these patches (undocumented fork, modification of lib files) as it wouldn't be possible in the first place if this were installed via npm or bower. But given we have the local patch for now, rather than the file just missing it and accidentally adding it back when updating the file, we need to document it.
Alternatively, I could move the d3-flameGraph to a separate directory with a PATCHES file. If you prefer that.
This is already loaded from layout/base.twig, and currently ends up being included (and executed) twice.
Follows-up 71f521e. * Undo the overrides and place them in xhgui.css instead. * Add a class "xhgui-flamegraph" to the main chart element so that we can uniquely address flamegraphs within xhgui with a higher CSS specificity selector (using the same selector as in d3.flameGraph.css won't work given that they have the same weight, and load order would break the tie, which currently means the upstream one applies last, use a stronger selector instead of relying on load order). This is in preparation of upgrading d3.flameGraph to a newer version from upstream. Ref perftools#211.
Follows-up 0ff9af8. * Move the overrides to xhgui.css instead. * Given the complexity of these overrides, don't try to undo each original style, but simply document that all d3-flame-graph-tip styles should be omitted when updating. * Document the d3.flameGraph.js patch. * Add ignore for pre-existing issues with csslint. Since sticker-ci was enabled without passing on current code first. Also, these issues are false negatives, and are bugs in csslint, not bugs in xhgui code. Ref perftools#211.
No description provided.