Add a category breakdown to the sidebar #1627
Conversation
| /> | ||
| ) : ( | ||
| <div /> | ||
| )} |
There was a problem hiding this comment.
This was easier to style the grid with an empty element here. Also I find it easier and still nice with the color information at the end of the line.
Codecov Report
@@ Coverage Diff @@
## master #1627 +/- ##
==========================================
+ Coverage 81.66% 81.68% +0.01%
==========================================
Files 174 174
Lines 11781 11796 +15
Branches 2843 2852 +9
==========================================
+ Hits 9621 9635 +14
- Misses 1965 1966 +1
Partials 195 195
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #1627 +/- ##
==========================================
+ Coverage 81.34% 81.36% +0.01%
==========================================
Files 179 179
Lines 11877 11894 +17
Branches 2867 2874 +7
==========================================
+ Hits 9661 9677 +16
- Misses 2012 2013 +1
Partials 204 204
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Thanks, this will be handy to have.
I find the "running time" term to be confusing here. The long phrase I would use to describe the categories here would be: "the categories of the self time of the leaf-most nodes for this call node". Can we just say "categories" here?
| class CategoryBreakdown extends React.PureComponent<CategoryBreakdownProps> { | ||
| render() { | ||
| const { breakdown, categoryList } = this.props; | ||
| const data = breakdown.map((value, categoryIndex) => { |
There was a problem hiding this comment.
I think this would be more valuable if it was sorted by percentage, with the heaviest categories at the top. It would make the list much more scannable.
There was a problem hiding this comment.
If/when we have histograms we can revisit this. I also find it nice to have them in always the same order, but I agree that without histograms this makes it more difficult to look at.
src/test/unit/profile-data.test.js
Outdated
| totalTime: { | ||
| value: 5, | ||
| breakdownByImplementation: { native: 2, baseline: 1, ion: 2 }, | ||
| breakdownByCategory: [1, 3, 1], // [Idle, Other, Layout] |
There was a problem hiding this comment.
Please provide variable bindings for the numbers. This will help with the readability.
const IDLE = 1;
const OTHER = 3;
There was a problem hiding this comment.
Well, this is not what these values mean :) 1 is the number of samples in the Idle category, 3 in the Other category, etc. This is not very readable because this is an array, but in the same time this is the best structure to use in the code because we use the category indexes.
There was a problem hiding this comment.
Used Array(categories.length).fill(0) as we discussed
src/components/sidebar/sidebar.css
Outdated
| @@ -13,13 +13,14 @@ | |||
|
|
|||
| .sidebar-calltree .sidebar-contents-wrapper { | |||
| display: grid; | |||
| grid-template-columns: 1fr 1fr; | |||
| grid-template-columns: min-content 1fr 1fr; | |||
There was a problem hiding this comment.
I realize this is wrong now: I wrote this when I've put the category square first, but now it's at the end... As a result everything "jumps" sometimes.
Text-only implementation of firefox-devtools#1080
julienw
force-pushed
the
categories-in-sidebar
branch
from
d3f0369
to
a03219f
Compare
Text-only implementation of #1080
Deploy preview