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
Add Storage Dashboard visualizations for histories #14820
Conversation
7ddf4c0
to
084fdcb
Compare
f8f3c46
to
1f9e1d2
Compare
1f9e1d2
to
7aa2f63
Compare
Great capability and the demo video looks cool. A couple of comments: how about adding an option to show top 10, 20, 50 datasets? Or make it configurable by the user? Fixed 10 seems like a low number. Second is the use of a pie chart. Use of pie charts is discouraged for data viz (here's an example article https://medium.com/analytics-vidhya/dont-use-pie-charts-in-data-analysis-6c005723e657 but if you search you'll find plenty more) so it would probably be best to swap it from the start to a different type of a plot. |
Thank you @afgane for your feedback! Those are great ideas, I will definitely implement them 👍 |
bd05e84
to
95baf8e
Compare
I've updated the PR description. Now it displays Bar Charts instead of Pie Charts and includes an option to select the number of items to display (up to 50). |
The bar graph looks really nice. Well done. (now if we could also have a Runtime Dashboard that shows runtime of jobs within a history, that'd be neat ;) ) |
That sounds like a nice idea! but definitely for a different PR 😆 |
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.
This looks very informative, well done!
General feedback:
The bar charts y axis labels are a bit small.
Would it be possible to indicate if a history is deleted/recoverable storage space within the main bar chart view, without hovering, to make them more obvious?
It can be quite challenging to hover over these smaller bars. would it be possible to trigger the tooltip on hovering over the column?
I find this link a bit too hidden. Maybe add a small arrow to the left, or change it to a button
client/src/components/User/DiskUsage/Visualizations/Charts/utils.ts
Outdated
Show resolved
Hide resolved
client/src/components/User/DiskUsage/Visualizations/HistoriesStorageOverview.vue
Outdated
Show resolved
Hide resolved
client/src/components/User/DiskUsage/Visualizations/HistoryStorageOverview.vue
Outdated
Show resolved
Hide resolved
Allow more information to be passed to the PieChart component, and emit events for possible tooltips.
Make tooltips optional and use slots to customize the tooltip
If the value of the slice is too small in proportion to total (tends to 0 but it is not 0) it will not render the chart at all.
Instead of going to Storage Dashboard main page, the button now goes to the History Overview page to display the history size graph.
Those fields are the most commonly used ones so it's worth defining them, the rest are dynamic for now. Also, it requires to exclude the unset fields from the payload, otherwise those will be set to None by pydantic and the server logic will assume the user wants to set them to None.
Refactor SelectedHistoryActions to SelectedItemActions for reusability
Thank you for the reviews! I'll go and make the requested changes 👍 |
Avoid using `_` in names.
95baf8e
to
ed8ab68
Compare
@ElectronicBlueberry, I think I've addressed all or most of the comments with the following nuances:
I wanted to make the BarChart somewhat generic so the only information available in a
I couldn't find an easy way to make the whole column hoverable without too much juggling and even doing so it felt like hovering over an invisible column could make it even more confusing, so I decided to increase the minimum size of the bar "from the bottom" just to make the hovering and click area bigger. Hope this is better.
When converting the headings as you suggested, I decided to also align them to the left so it is more consistent with other pages, I think doing so now makes the router link more obvious. What do you think? |
prevent tooltip from leaving window
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.
We resolved the history hover issue, by making it more obvious that the legend can be clicked. The bar chart overflow was reverted.
Thank you so much @ElectronicBlueberry! |
First simple visualizations (Bar Charts) of the disk space taken by your histories. The charts are integrated into the Storage Dashboard to help to have an overview of the space taken and potentially recoverable.
You can have a general overview of all your histories or an overview of one particular history and its datasets. Some charts are interactive and you can select the different entries to get more information or perform some actions.
When clicking on the
History Size
icon in the History counter, now you will get directed to that particular history storage overview. You can still access the main Storage Dashboard page from the Quota meter at the top right of the page.StorageDashboardVisualizations.mp4
TODO
How to test the changes?
http://127.0.0.1:8080/storage/histories
and visually explore your dataLicense