feat(flamegraph): Redesign flamegraph toolbar to allow for more interactions #1674

pavelpashkovsky · 2022-11-03T14:16:11Z

Brief

Redesign flamegraph toolbar to allow for more interactions #1636

Changes

DEMO: https://pr-1674.pyroscope.io/

github-actions · 2022-11-03T14:20:58Z

size-limit report 📦

Path	Size	Loading time (3g)	Running time (snapdragon)	Total time
webapp/public/assets/app.js	490.01 KB (+0.39% 🔺)	9.9 s (+0.39% 🔺)	3.4 s (-3.39% 🔽)	13.2 s
webapp/public/assets/app.css	19.28 KB (+2.46% 🔺)	386 ms (+2.46% 🔺)	0 ms (+100% 🔺)	386 ms
webapp/public/assets/styles.css	9.52 KB (-0.19% 🔽)	191 ms (-0.19% 🔽)	0 ms (+100% 🔺)	191 ms
packages/pyroscope-flamegraph/dist/index.js	131.66 KB (+1.23% 🔺)	2.7 s (+1.23% 🔺)	1.6 s (+19.71% 🔺)	4.3 s
packages/pyroscope-flamegraph/dist/index.node.js	132.34 KB (+1.18% 🔺)	2.7 s (+1.18% 🔺)	543 ms (-8.11% 🔽)	3.2 s
packages/pyroscope-flamegraph/dist/index.css	8.28 KB (+4.61% 🔺)	166 ms (+4.61% 🔺)	0 ms (+100% 🔺)	166 ms

pavelpashkovsky · 2022-11-03T14:32:51Z

Let me know if some sizes, shapes or colors are incorrect.
There is no enough space to place everything on Toolbars on Comparison page. Any ideas?

comparison_not_enough_space.mp4

What to do with SELF | TOTAL | DIFF buttons on DiffView page. Does it make sense to transform these button in the same way as TABLE | FLAMEGRAPH+TABLE | FLAMEGRAPH (on the right side)?
This selector looks like font-size selector, right?
@eh-am After some updates live reload for storybook doesn't work anymore. Is there easy way to fix it?
Actual Export button icon differs from required icon in this task. Guys told that you have arranged to leave current button. Just to confirm.
On the mockup toolbar looks kind of divided.
@eh-am can you please help w/ code review. It's not a final version, but we have to be on the same page
Can you please explain a bit more about about this set of colors+buttons

pavelpashkovsky · 2022-11-03T14:32:58Z

/create-server

codecov · 2022-11-03T14:38:00Z

Codecov Report

Base: 66.19% // Head: 66.22% // Increases project coverage by +0.04% 🎉

Coverage data is based on head (41f34ef) compared to base (9560869).
Patch coverage: 82.82% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1674      +/-   ##
==========================================
+ Coverage   66.19%   66.22%   +0.04%     
==========================================
  Files         166      164       -2     
  Lines        5462     5441      -21     
  Branches     1231     1220      -11     
==========================================
- Hits         3615     3603      -12     
+ Misses       1840     1831       -9     
  Partials        7        7

Impacted Files	Coverage Δ
...e-flamegraph/src/FlameGraph/FlameGraphRenderer.tsx	`49.71% <ø> (-0.29%)`	⬇️
...oscope-flamegraph/src/SharedQueryInput.module.scss	`61.54% <0.00%> (ø)`
...ebapp/javascript/components/ExportData.module.scss	`61.54% <ø> (ø)`
webapp/javascript/components/ExportData.tsx	`37.78% <0.00%> (+0.70%)`	⬆️
...kages/pyroscope-flamegraph/src/Toolbar.module.scss	`61.54% <61.54%> (ø)`
packages/pyroscope-flamegraph/src/Toolbar.tsx	`83.21% <91.31%> (-0.57%)`	⬇️
...graph/src/FlameGraph/FlameGraphComponent/index.tsx	`81.89% <100.00%> (ø)`
packages/pyroscope-flamegraph/src/Icons.tsx	`100.00% <100.00%> (ø)`
webapp/javascript/ui/Button.tsx	`72.98% <0.00%> (-5.40%)`	⬇️
... 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.

Rperry2174 · 2022-11-04T07:39:57Z

Let me know if some sizes, shapes or colors are incorrect.

see picture above for color adjustments / styling placement adjustments

There is no enough space to place everything on Toolbars on Comparison page. Any ideas?

When there's not enough space these should turn to dropdowns:

head first / tail first tabs
table / both / flamegraph / sandwich tabs

For now, let's just make everything else just be an icon with no words. Eventually we will use the tooltip that @eh-am added and things will work similar to google sheets.

Screen.Recording.2022-11-04.at.12.35.03.AM.mov

So this means that the following should be changed to just be their respective icons 100% of the time (we may change this later):

Collapse nodes above
reset view

What to do with SELF | TOTAL | DIFF buttons on DiffView page. Does it make sense to transform these button in the same way as TABLE | FLAMEGRAPH+TABLE | FLAMEGRAPH (on the right side)?

Let's worry about this later...

This selector looks like font-size selector, right?

yes this is just for the mockup don't worry about it for now we will add font size later maybe

Actual Export button icon differs from required icon in this task. Guys told that you have arranged to leave current button. Just to confirm.

Yes we decided on the new one no need to change the export button again

On the mockup toolbar looks kind of divided.

yes toolbar is divided see picture at top of this message

@eh-am can you please help w/ code review. It's not a final version, but we have to be on the same page

Can you please explain a bit more about about this set of colors+buttons

Just for reference in case we don't like the colors chosen

pavelpashkovsky · 2022-11-04T14:28:40Z

/create-server

Rperry2174 · 2022-11-07T20:55:03Z

When it is flamegraph only or table only then we should merge the toolbar so that its one bar across:

we should use our styled dropdown component here not the generic one

Screen.Recording.2022-11-07.at.12.48.31.PM.mov

Lets try these two icons for headfirst and tail first instead of the ones we're currently using
Selected items shouldn't turn "grey" on-hover -- lets just have no effect for them on hover
https://user-images.githubusercontent.com/23323466/200413059-2bf02d35-9698-4021-befd-17ad273b9861.mov
lets try making the inner svg be 32x32 (or just a little bit larger than they currently are)

pavelpashkovsky · 2022-11-08T15:27:20Z

/create-server

pavelpashkovsky · 2022-11-09T10:23:04Z

/create-server

pavelpashkovsky · 2022-11-10T11:21:30Z

/create-server

Rperry2174

LGTM! Nice work :)

feat: toolbar redesign [WIP]

c433da7

Ivanikitin added the frontend Mostly JS code label Nov 3, 2022

pavelpashkovsky added 2 commits November 4, 2022 12:56

feat: appearance for buttons

3f148ec

feat: adaptating toolbar for diff render cases, buttons, tooltips

50b2bd2

github-actions bot added the has conflicts label Nov 6, 2022

pavelpashkovsky added 2 commits November 8, 2022 15:49

refactor: toolbar adjustments

e30bfce

Merge branch 'main' into toolbar-redesign

31668c6

pavelpashkovsky removed the has conflicts label Nov 8, 2022

refactor: js tests

b452512

pavelpashkovsky added 6 commits November 9, 2022 11:04

feat: toolbar light mode

d93328a

fix: buttons appearance adjustments

2d6ab97

chore: export data btn adjustment

826d72f

chore: upd snaphots for svg icons

4e6ff7e

refactor: cy tests

52f3f47

fix: changing view mode shift buttons

b1da0fb

pavelpashkovsky marked this pull request as ready for review November 9, 2022 10:29

pavelpashkovsky changed the title ~~feat(flamegraph): Redesign flamegraph toolbar to allow for more interactions [WIP]~~ feat(flamegraph): Redesign flamegraph toolbar to allow for more interactions Nov 9, 2022

pavelpashkovsky requested a review from eh-am November 9, 2022 10:29

github-actions bot added the has conflicts label Nov 9, 2022

pavelpashkovsky added 2 commits November 10, 2022 11:10

Merge branch 'main' into toolbar-redesign

b2c6a8e

fix: remove cl

a8caf6c

pavelpashkovsky added 2 commits November 10, 2022 14:05

refactor: toolbar adjustments

35cdaf9

fix: upd test

41f34ef

Rperry2174 approved these changes Nov 10, 2022

View reviewed changes

Rperry2174 merged commit 646501a into main Nov 10, 2022

Rperry2174 deleted the toolbar-redesign branch November 10, 2022 15:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(flamegraph): Redesign flamegraph toolbar to allow for more interactions #1674

feat(flamegraph): Redesign flamegraph toolbar to allow for more interactions #1674

pavelpashkovsky commented Nov 3, 2022 •

edited

Loading

github-actions bot commented Nov 3, 2022 •

edited

Loading

pavelpashkovsky commented Nov 3, 2022 •

edited

Loading

pavelpashkovsky commented Nov 3, 2022

codecov bot commented Nov 3, 2022 •

edited

Loading

Rperry2174 commented Nov 4, 2022

pavelpashkovsky commented Nov 4, 2022

Rperry2174 commented Nov 7, 2022 •

edited by pavelpashkovsky

Loading

pavelpashkovsky commented Nov 8, 2022

pavelpashkovsky commented Nov 9, 2022

pavelpashkovsky commented Nov 10, 2022

Rperry2174 left a comment

feat(flamegraph): Redesign flamegraph toolbar to allow for more interactions #1674

feat(flamegraph): Redesign flamegraph toolbar to allow for more interactions #1674

Conversation

pavelpashkovsky commented Nov 3, 2022 • edited Loading

Brief

Changes

github-actions bot commented Nov 3, 2022 • edited Loading

size-limit report 📦

pavelpashkovsky commented Nov 3, 2022 • edited Loading

pavelpashkovsky commented Nov 3, 2022

codecov bot commented Nov 3, 2022 • edited Loading

Codecov Report

Rperry2174 commented Nov 4, 2022

pavelpashkovsky commented Nov 4, 2022

Rperry2174 commented Nov 7, 2022 • edited by pavelpashkovsky Loading

pavelpashkovsky commented Nov 8, 2022

pavelpashkovsky commented Nov 9, 2022

pavelpashkovsky commented Nov 10, 2022

Rperry2174 left a comment

Choose a reason for hiding this comment

pavelpashkovsky commented Nov 3, 2022 •

edited

Loading

github-actions bot commented Nov 3, 2022 •

edited

Loading

pavelpashkovsky commented Nov 3, 2022 •

edited

Loading

codecov bot commented Nov 3, 2022 •

edited

Loading

Rperry2174 commented Nov 7, 2022 •

edited by pavelpashkovsky

Loading