treeherder icon indicating copy to clipboard operation
treeherder copied to clipboard

Make the perfherder graph width sized dynamically, and refine the tooltip

Open gregtatum opened this issue 4 years ago • 3 comments

This PR changes some of the visual layout behavior of the perfherder graph. This change was motivated with the inconsistent feeling of looking at the data on different monitor setups. I frequently move between smaller screens, larger ones, and split views. The graphs were frequently not visible.

#6861 needs to land first, and then this needs to rebase through it, as they are touching similar parts of the code.

Each commit contains a message explaining more in detail what the changes are doing, so I would recommend looking at each commit individually with the message. Here are some screenshots detailing the changes.

Commit 1 - Chart width

Large width:

Before, the view was maxed out at 1350px.

image

After, the graph will take all available width.

image

Small width:

Before, the view scaled the entire chart down, including labels. For me, the labels were frequently too small to read. The tooltip also scaled according to the size of the chart, moving outside of the range of legibility.

image

After, the labels are always consistently sized, and the font doesn't change size.

image

Commit 2 - Tooltip

Normally, I would break something like this into a separate PR, but commit 1 regressed the tooltip behavior, so that it would render off the side of the screen. I needed to fix this issue, and as I did, I included some usability fixes as well to make the tooltips more consistent when mousing over the graph.

Typical view:

image

Left overflow:

image

Right overflow:

image

Fixed vertical positioning

Before, it was done through constant values, which would be weird for certain content. image

After, the divs can dynamically grow up. image

gregtatum avatar Nov 11 '20 21:11 gregtatum

The only thing I found not ok while testing this PR is:

  1. Go to perfherder Graph view
  2. Click on Add test data button
  3. Change the repo filter from mozilla-central to autoland (to get any test so you can select one)
  4. Select first test (basically any test) and click Plot graphs
  5. You'll have your graphs plotted but the Add test data modal appears again like you clicked on something like Plot graphs and add more tests. Like I suggested at # 4 above, this should happen with any filter you use and any number of tests you add Screenshot from 2020-11-18 08-54-40

I agree with @sarah-clements, this looks pretty nice. Few polishes here and there and it should be ready to land.

alexandru-io avatar Nov 18 '20 06:11 alexandru-io

Sorry, I missed the feedback here, as I was looking at the review status flag, and hadn't configured notifications for individual comments.

I switched teams from the Perf Tools team to the Internationalization team, so this is a bit of a low priority for me at the moment. I'll keep it in my backlog to finish up, so it's worth keeping this PR open, but I don't have a specific timeframe, as I still have a few things in my profiler backlog. If someone wanted to fix up the comments and merge it, I wouldn't be opposed, even if it's not as typical to do that with Mozilla patches. Otherwise I'll keep it in my backlog.

gregtatum avatar Jan 15 '21 15:01 gregtatum

@gregtatum I see your last comment about switching teams, did you want to finish this up? anyone we could ask to finish it up?

jmaher avatar Sep 14 '22 16:09 jmaher