Cleanup of tracing graph #9564
Cleanup of tracing graph #9564
Conversation
bramkragten
force-pushed
the
tracing-cleanup
branch
from
87ad832
to
cb38855
Compare
| M 0 ${SPACING + NODE_SIZE + 1} | ||
| L 0 0 | ||
| V 0 |
| .iconPath=${mdiAsterisk} | ||
| tabindex=${tracked ? "0" : "-1"} | ||
| tabindex=${track ? "0" : "-1"} |
There was a problem hiding this comment.
Maybe tabindex should actually be removed from the graphs entirely. I added it for accessibility, but in the current implementation it makes no sense and may actually be a hindrance.
I'll raise this as a wider issue somewhere.
There was a problem hiding this comment.
Yeah, that should probably be a separate PR
| this.renderedNodes[path] = { config, path }; | ||
| if (trace) { | ||
| if (this.trace && path in this.trace.trace) { | ||
| this.trackedNodes[path] = this.renderedNodes[path]; |
There was a problem hiding this comment.
This is pretty much the only place (except the choose node) where trackedNodes is modified in a renderer...
Come to think of it, the choose node could probably be made to better recursively use render_node which would make render_condition the only special case for this. It may be worth trying to refactor this out as well and just use render_condition_node directly instead.
There was a problem hiding this comment.
render_node does the same right? Also adding nodes during rendering? I don't really see the benefit here?
There was a problem hiding this comment.
I was just thinking it's an almost unique side effect to modify trackedNodes here, which I think is cosidered a code smell?
There was a problem hiding this comment.
Yeah... I think it is ok here...
| if (trackPass && trackFailed) { | ||
| break; | ||
| } | ||
| } |
Proposed change
Some cleanup of the script tracing graph
Type of change
Example configuration
Additional information
Checklist
If user exposed functionality or configuration variables are added/changed: