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

ui: Flamegraph tooltip improvements #2600

Merged
merged 19 commits into from
Feb 21, 2023
Merged

Conversation

manojVivek
Copy link
Contributor

No description provided.

@manojVivek manojVivek requested a review from a team as a code owner February 15, 2023 17:36
@@ -50,3 +50,11 @@ export const truncateString = (str: string, num: number): string => {

return str.slice(0, num) + '...';
};

export const truncateStringReverse = (str: string, num: number): string => {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is a good example of something that could live in the @parca/utils package once that is in place :) cc @yomete

Copy link
Contributor

Choose a reason for hiding this comment

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

will look out for it!

@monicawoj
Copy link
Contributor

In general, looks great to me!

My only concern is the empty whitespace in the tooltip when hovering over the root node (or a node with very little information to show).. I don't assume the user will spending much time hovering over root, but just wanted to bring it up and get some others' thoughts on this.
Screenshot 2023-02-15 at 22 19 33

@manojVivek
Copy link
Contributor Author

My only concern is the empty whitespace in the tooltip when hovering over the root node (or a node with very little information to show).. I don't assume the user will spending much time hovering over root, but just wanted to bring it up and get some others' thoughts on this.

Yes, I had the concern as well. Should we try listing all fields everywhere and mark that there's no data wherever we don't have the info?

@brancz Let me know what do you think as well.

@brancz
Copy link
Member

brancz commented Feb 16, 2023

Yes, I had the concern as well. Should we try listing all fields everywhere and mark that there's no data wherever we don't have the info?

I think this is a good idea.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants