Hide span details in structure tab #291
Conversation
| @@ -393,6 +393,21 @@ const getStyles = (theme: GrafanaTheme2) => { | |||
| 'div[data-testid="span-detail-component"] > :nth-child(4) > :nth-child(1)': { | |||
| display: 'none', | |||
| }, | |||
|
|
|||
| // Hide span details row | |||
| '.span-detail-row': { | |||
There was a problem hiding this comment.
Where's that class being used? 🤔 Couldn't find reference to it anywhere.
Hiding with CSS may break if class names change in the TraceView so if we want to go this route I think we should have a test that would ensure elements are really hidden. Also, wouldn't it be nicer if we added a new property to TraceView to control it?
There was a problem hiding this comment.
I've added it to the trace view in the Grafana repo PR.
I've also had a look at creating a test for this e.g. render the Traces panel and make sure that some of the child components of the span detail are not visible but it's kind of tricky to mock / provide dummy data for the Structure Tab :( do you have any suggestions?
Adding a new property is a nice touch and was almost going to do that but thought it's a bit overkill for this. We already hide the minimap, header, span and resource accordions via styling in Explore Traces and wanted to hide span details in the same way rather than introduce a new prop that will probably only be used by the app and may confuse users.
There was a problem hiding this comment.
We already hide the minimap, header, span and resource accordions via styling in Explore Traces and wanted to hide span details in the same way rather than introduce a new prop that will probably only be used by the app and may confuse users.
The risk is that if TraceView is refactored it may affect the app. Adding tests could help to catch if it happens but only if we tested the app against the latest version of Grafana so it's still quite fragile. I think it's also confusing that we have CSS classes in Grafana that are used outside of Grafana. They become part of a public contract we rely on.
To me it looks like tech debt that can fire back at some point. For backwards compatibility we may need to keep CSS hacks anyway so feel free to go with it but I'd suggest switching to something more explicit in long run 👍
Fixes #253