Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(webapp): Make explore page show precise numbers in table #1695

Merged
merged 4 commits into from
Nov 15, 2022

Conversation

pavelpashkovsky
Copy link
Contributor

@pavelpashkovsky pavelpashkovsky commented Nov 10, 2022

@github-actions
Copy link
Contributor

github-actions bot commented Nov 10, 2022

size-limit report 📦

Path Size Loading time (3g) Running time (snapdragon) Total time
webapp/public/assets/app.js 491.04 KB (+0.09% 🔺) 9.9 s (+0.09% 🔺) 3.8 s (+5.06% 🔺) 13.6 s
webapp/public/assets/app.css 19.36 KB (+0.17% 🔺) 388 ms (+0.17% 🔺) 0 ms (+100% 🔺) 388 ms
webapp/public/assets/styles.css 9.54 KB (0%) 191 ms (0%) 0 ms (+100% 🔺) 191 ms
packages/pyroscope-flamegraph/dist/index.js 131.97 KB (+0.05% 🔺) 2.7 s (+0.05% 🔺) 1.6 s (+2.87% 🔺) 4.2 s
packages/pyroscope-flamegraph/dist/index.node.js 132.61 KB (+0.05% 🔺) 2.7 s (+0.05% 🔺) 527 ms (-6.8% 🔽) 3.2 s
packages/pyroscope-flamegraph/dist/index.css 8.33 KB (0%) 167 ms (0%) 0 ms (+100% 🔺) 167 ms

@codecov
Copy link

codecov bot commented Nov 10, 2022

Codecov Report

Base: 66.26% // Head: 66.39% // Increases project coverage by +0.14% 🎉

Coverage data is based on head (e5f6f54) compared to base (358e7f7).
Patch coverage: 75.68% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1695      +/-   ##
==========================================
+ Coverage   66.26%   66.39%   +0.14%     
==========================================
  Files         165      166       +1     
  Lines        5464     5506      +42     
  Branches     1224     1237      +13     
==========================================
+ Hits         3620     3655      +35     
- Misses       1837     1844       +7     
  Partials        7        7              
Impacted Files Coverage Δ
packages/pyroscope-flamegraph/src/format/format.ts 72.73% <42.86%> (-0.24%) ⬇️
webapp/javascript/pages/formatTableData.ts 95.66% <95.66%> (ø)
...graph/src/FlameGraph/FlameGraphComponent/index.tsx 80.54% <0.00%> (-1.09%) ⬇️
packages/pyroscope-models/src/profile.ts 94.12% <0.00%> (ø)
...kages/pyroscope-flamegraph/src/Tooltip/Tooltip.tsx 82.76% <0.00%> (ø)
...raph/src/FlameGraph/FlameGraphComponent/Header.tsx 90.48% <0.00%> (ø)
...e-flamegraph/src/FlameGraph/FlameGraphRenderer.tsx 50.00% <0.00%> (+0.60%) ⬆️

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.

@pavelpashkovsky
Copy link
Contributor Author

/create-server

@eh-am
Copy link
Collaborator

eh-am commented Nov 10, 2022

Can you add few tests to this
https://github.com/pyroscope-io/pyroscope/blob/0db1872abf49aaa3c3cfa3bd7d79d1502ba3562c/packages/pyroscope-flamegraph/src/format/format.spec.ts ? It's a bit hard to grasp the logic by just looking at the code.

@Ivanikitin Ivanikitin added the frontend Mostly JS code label Nov 10, 2022
@Rperry2174
Copy link
Contributor

also not sure how hard this is to do, but can you make it so that all of the decimal places are alligned?

@petethepig
Copy link
Member

/create-server

@Rperry2174
Copy link
Contributor

adding this for reference: https://alistapart.com/article/web-typography-tables/

@Rperry2174
Copy link
Contributor

/create-server

@pavelpashkovsky
Copy link
Contributor Author

/create-server

Copy link
Contributor

@Rperry2174 Rperry2174 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Rperry2174 Rperry2174 merged commit 5b47c71 into main Nov 15, 2022
@Rperry2174 Rperry2174 deleted the precise_numbers_table branch November 15, 2022 11:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
frontend Mostly JS code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants