search

grafana / grafana

The open and composable observability and data visualization platform. Visualize metrics, logs, and traces from multiple sources like Prometheus, Loki, Elasticsearch, InfluxDB, Postgres and many more.
https://grafana.com
GNU Affero General Public License v3.0
60.72k stars 11.61k forks source link

Flamegraph: Add diff mode color legend #87319

Closed aocenas closed 1 week ago

aocenas commented 2 weeks ago

Closes: https://github.com/grafana/grafana/issues/87314

This changes the color scheme switch button in diff mode to also show a legend with a bit of explanation as it may not be clear why positive numbers are red.

Additional small fixes:

https://github.com/grafana/grafana/assets/1014802/ba83d371-3d82-410d-9ae7-aebed04b148a

staton-hyse11 commented 2 weeks ago

@aocenas is there a way to provide affordances that this is clickable? Currently, it doesn't appear to be a dropdown and it is at risk of going unnoticed. Changing this from a button to a basic select might be an option. WDYT?

aocenas commented 1 week ago

@staton-hyse11 not sure what you mean. It uses a Dropdown component that wraps a Button component. Isn't that how dropdowns should be used? Also it is a toolbar where every element is a way to control the visualization so I think there is an assumption that stuff in toolbar is clickable and non clickable element are the exceptions.

Like if it was only a button that would automatically cycle between states it would look like this and I would assume it being a Button component should be enough for users to suggest it is clickable. If not then all the buttons would be problematic.

That said I didn't use select because this is not a variable sized list and I don't most of the select functionality (like search, custom values, loading etc) so from purely engineering standpoint this seemed better fit and I want the element to be visually similar in the toolbar.

aocenas commented 1 week ago

@staton-hyse11 I am going to merge this to prevent merge conflicts with some other work but if you think it's important to change this to a Select let me know and we can do that afterwards.