-
Notifications
You must be signed in to change notification settings - Fork 12.4k
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
NodeGraph: Support icons for nodes #60989
Conversation
I'm not sure if there is a way to set this in the UI... but we do have an editor that will let you pick/search icons:
|
@ryantxu thanks, this though will come handy later. My assumption is that first we may implement some automatic icon mapping per data source (like x-ray probably know what is what). Then we may think about some user config to map based on some rules (probably matching node names) that could add to or override what data source defines in the response |
This pull request has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in 2 weeks if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions! |
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.
Mostly UI/UX suggestions (prob ok if didn't fix em all). Overall, love this.
- Only display e.g. 'Requests per second' if there is a value for it
- Line height between 'Service name' & 'Average response time' is different to line height between 'Average response time' & 'Requests per second' (prob happening because there is no value)
- Hovering over edge shows blank line if no secondary stat i.e. blank line below 13.32 in image
- Title/subtitle case: could prob capitalise here
- Maybe a little more padding for title/subtitle/total time/self time container. So they align better with 'Open in Explore' indentation - although 'Open in Explore' has a different purpose (this is subjective really so whatever change or not you want to make here)
- Should we remove the bottom border
Also
- Could update test data source node graph scenario to include node with icon
| Field name | Type | Description | | ||
| ------------- | ------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | ||
| title | string | Name of the node visible in just under the node. | |
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.
Could prob improve the formatting here. There's a lot of -----
and seems |
does not need to be pushed to a new line.
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.
For me, the IDE does this automatically so that the table is aligned if the source is shown without line wraps, like
|----------|---------------------|
|content |some more things here|
And here some of the descriptions are long so it aligns length to that.
Done
Done
This is based on displayName of a field if it's not set it uses the field name. Not sure if it makes sense to capitalize in that case, would probably keep the distinction so it's clear it's a field name and let users name it as they want.
Agree but seems like that is all in the ContextMenu component so will do a separate PR as that probaly needs review from user essentials.
|
Allow specifying an icon for node data that will then be used inside the node instead of the stats shown currently:
At this moment only icons already specified inside Grafana are allowed so no custom icons.
Stats are now shown also in a context menu so if icon is shown they are not totally lost.
There are some additional small changes:
Future improvement not done here: We don't handle the units of the stats properly atm so this should be fixed using Grafana standard utils.