-
Notifications
You must be signed in to change notification settings - Fork 626
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: Heatmap should show error message if no data is returned #1565
Conversation
size-limit report 📦
|
Codecov ReportBase: 66.63% // Head: 66.58% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #1565 +/- ##
==========================================
- Coverage 66.63% 66.58% -0.05%
==========================================
Files 147 147
Lines 4998 4999 +1
Branches 1155 1156 +1
==========================================
- Hits 3330 3328 -2
- Misses 1664 1667 +3
Partials 4 4
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. |
{dataUnavailable ? ( | ||
<NoData dataName="profiling" /> | ||
) : ( | ||
panes.map((pane) => pane) | ||
)} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this can be simplified to simply No data available
, without specifying a qualifier.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No data available
instead of No profiling data available for this application / time range.
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think so, then we don't need all this new code. cc @Rperry2174
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there was a NoProfilingData
component which is div + span + styles to add paddings and center text, i moved the whole component to the webapp/ui folder....so its not that new code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah I meant allowing custom title (profile
| heatmap
), that's a level of customization I don't think we really need it.
* add common NoData component * change "no data" message * handle heatmap null value * hide flamegraph when heatmap is null
59d34aa
to
c444f7f
Compare
/create-server |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm!
Brief
Changes
Screen.Recording.2022-09-28.at.16.20.43.mov
Concerns