-
Notifications
You must be signed in to change notification settings - Fork 23
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
fix: Truncate sunburst chart's breadcrumbs #3413
Conversation
Bundle ReportChanges will decrease total bundle size by 17 bytes (-0.0%) ⬇️. This is within the configured threshold ✅ Detailed changes
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## main #3413 +/- ##
=======================================
Coverage 99.15% 99.15%
=======================================
Files 809 809
Lines 14275 14281 +6
Branches 3942 3937 -5
=======================================
+ Hits 14154 14160 +6
Misses 112 112
Partials 9 9
Continue to review full report in Codecov by Sentry.
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found. @@ Coverage Diff @@
## main #3413 +/- ##
=======================================
Coverage 99.15% 99.15%
=======================================
Files 809 809
Lines 14275 14281 +6
Branches 3935 3937 +2
=======================================
+ Hits 14154 14160 +6
Misses 112 112
Partials 9 9
Continue to review full report in Codecov by Sentry.
|
Codecov ReportAll modified and coverable lines are covered by tests ✅ ✅ All tests successful. No failed tests found. @@ Coverage Diff @@
## main #3413 +/- ##
=======================================
Coverage 99.15% 99.15%
=======================================
Files 809 809
Lines 14275 14281 +6
Branches 3935 3937 +2
=======================================
+ Hits 14154 14160 +6
Misses 112 112
Partials 9 9
Continue to review full report in Codecov by Sentry.
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found. @@ Coverage Diff @@
## main #3413 +/- ##
=======================================
Coverage 99.15% 99.15%
=======================================
Files 809 809
Lines 14275 14281 +6
Branches 3935 3944 +9
=======================================
+ Hits 14154 14160 +6
Misses 112 112
Partials 9 9
Continue to review full report in Codecov by Sentry.
|
✅ Deploy preview for gazebo ready!Previews expire after 1 month automatically.
|
if (breadcrumbPaths.length <= 1) { | ||
pathsToDisplay = breadcrumbPaths | ||
} else { | ||
pathsToDisplay = [breadcrumbPaths[0], { text: '...' }] |
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.
Can we extend this to more than just one level? Something like 2 or 3 so it matches the depth you can travel while looking at the sunburst? I find it pretty difficult to get any context as to "where" i am in the file structure with just the current level.
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 also poke the design folks, and get their input on this.
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.
Thanks, I'll ping design. This is just a quick and dirty. Really we have too much information trying to be displayed in too little of space. I added some screenshots to the PR description to show what it's like. If it's any consolation, there's the hover tooltip on the sunburst - I can add one to the breadcrumbs as well perhaps.
If we want to do this for real a reusable truncated breadcrumbs component would be ideal imo
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.
Bundle ReportChanges will decrease total bundle size by 17 bytes (-0.0%) ⬇️. This is within the configured threshold ✅ Detailed changes
|
Fix bug with sunburst's breadcrumbs show a "..." if too long. It always display just the last element with "..." if it goes more than 1 deep as it seemed like in most cases only 1 fits on the page. This is a quick and dirty solution. For real we should probably give more real estate for this rich information or make a truncatable Breadcrumbs reusable component that has a resize observer.
Closes codecov/engineering-team#2337
Example screenshots:
here are the befores which just need a bandaid: