grafana / explore-profiles

Explore Profiles is a native Grafana application designed to integrate seamlessly with Pyroscope, the open-source continuous profiling platform, providing a smooth, query-less experience for browsing and analyzing profiling data.
GNU Affero General Public License v3.0
18 stars 1 forks source link

feat(DiffFlameGraph): Add "how to" infos #228

Closed grafakus closed 1 week ago

grafakus commented 1 week ago

✨ Description

Related issue(s): relates to https://github.com/grafana/explore-profiles/issues/224

This PR adds a collapsible section when landing on the "Diff flame graph" view and the user has not selected both flame graphs time ranges. The collapsed section includes an animated gif:

Before After
image Screenshot 2024-10-18 at 11 34 11
Screenshot 2024-10-18 at 11 34 47

📖 Summary of the changes

The E2E screenshots have also been updated. See diff tab for specific comments.

🧪 How to test?

-

github-actions[bot] commented 1 week ago

Unit test coverage

Lines Statements Branches Functions
Coverage: 11%
11.39% (424/3721) 9.03% (120/1328) 8.43% (100/1185)
Rperry2174 commented 1 week ago

I think this looks great!

  1. let's make the "how" uncollapsed by default -- one less click for the user. without it being visibile by default we run the same issue of someone potentially missing it and not knowing what to do
  2. Let's add a button that allows the users to "quick select" time periods on the graphs in case they don't want to manually do it (and it could default to "dumb" periods)

Also I think the "time picker" mode is kind of confusing and I would opt to remove that, but that is out of scope for this PR. image

wdyt?

bryanhuhta commented 1 week ago
  1. let's make the "how" uncollapsed by default -- one less click for the user. without it being visibile by default we run the same issue of someone potentially missing it and not knowing what to do

Just my 2¢, but this is a double-edged sword. Having the section expanded by default will likely help newcomers understand how to use the diff view on their first impression. However beyond that, I expect an auto-expanded "how-to" like this would be extraordinarily annoying and disruptive beyond the first one or two page lands. Once a user understands the flow, prominently displaying a large gif on how to use the tool which you must scroll by or manually collapse will not be a great experience. This is definitely a trade-off between "good first experience" and "good general experience." Do we want to design for the new user or for the general user? I would recommend against an auto-expanded help, but I will not die on this hill.

grafakus commented 1 week ago

I agree with Bryan, it's one more click for the user, once.

Let's try to make baby steps and divide & conquer. We can change and iterate later on.

@Rperry2174 for your 2nd suggestion, I'm working on https://github.com/grafana/explore-profiles/issues/200 and I'm also considering adding an "Auto-diff" button to simplify the flow.

Then we can add clear CTAs in the info banner so that the users have the choice to:

If none of these work well, we can consider auto-selecting by default, like the legacy plugin did.