Scroll item horizontally in the virtuallist, taking into account the fixed size

[julienw]

This was bothering me for so much time!

Here is the fix to the following STR:

1. open the call tree completely (with the key "*" for example)
2. scroll down with the keyboard (down or right arrow) to some item with a big depth, that should trigger a horizontal scroll.

This also fixes the STR where clicking in the activity graph should now properly horizontally scroll to the item.

When testing, depending on the size of your screen, you may need to reduce the size of your window, or run the profiler in Firefox mobile view to reduce its viewport, so that horizontally scrolling is necessary for some depths.
You can also increase the size of some columns (not available in production).

production
deploy preview

[codecov]

Codecov Report

Base: 88.55% // Head: 88.55% // Increases project coverage by +0.00% 🎉

Coverage data is based on head (9a78046) compared to base (0646fb5).
Patch coverage: 100.00% of modified lines in pull request are covered.

❗ Current head 9a78046 differs from pull request most recent head f1ae1fb. Consider uploading reports for the commit f1ae1fb to get more accurate results

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #4401   +/-   ##
=======================================
  Coverage   88.55%   88.55%           
=======================================
  Files         283      282    -1     
  Lines       25532    25478   -54     
  Branches     6878     6855   -23     
=======================================
- Hits        22610    22563   -47     
+ Misses       2715     2708    -7     
  Partials      207      207           

Impacted Files	Coverage Δ
src/components/shared/TreeView.js	82.50% <100.00%> (-0.06%)	⬇️
src/components/shared/VirtualList.js	93.27% <100.00%> (+0.05%)	⬆️
src/components/flame-graph/FlameGraph.js	74.50% <0.00%> (-3.93%)	⬇️
src/components/stack-chart/index.js	85.71% <0.00%> (-1.18%)	⬇️
src/components/shared/CallNodeContextMenu.js	89.51% <0.00%> (-0.73%)	⬇️
src/components/timeline/Selection.js	78.86% <0.00%> (-0.67%)	⬇️
...rc/components/timeline/TrackVisualProgressGraph.js	96.90% <0.00%> (-0.13%)	⬇️
src/profile-logic/import/chrome.js	94.52% <0.00%> (-0.07%)	⬇️
src/profile-logic/process-profile.js	90.95% <0.00%> (-0.06%)	⬇️
... and 7 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[canova]

Thanks, this indeed looks like a lot better when I'm using it.

[canova]

I'm curious why we don't add the offsetX to itemRight here. It looks like we do the same operation twice below (first we remove the offsetX from the other side of the comparison and then add it later).

[julienw]

I think itemRight itself should stay like this, as this represents properly the right edge for the item, but I can introduce another variable that would be itemRight - (container.clientWidth - offsetX), just not sure what its ideal name is. container.clientWidth - offsetX itself could be called scrollingColumnWidth, so maybe itemRight - scrollingColumnWidth is minimumLeftScrollingToSeeRightEdge? I'm not sure :D I'll think about this some more.

[julienw]

I could add some ascii art for the container.

[canova]

Well, I think it's not a big deal. So happy to land it like this too.

[julienw]

I decided to only add scrollingColumnWidth because I thought the reasoning process was easier to follow with keeping the calculations, but I also added some more comments. Hopefully this should make it easier to reason when looking at this in the future :-)