-
Notifications
You must be signed in to change notification settings - Fork 13.7k
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(dashboard): Highlight tabs that contain a chart in scope of focused native filter #14865
feat(dashboard): Highlight tabs that contain a chart in scope of focused native filter #14865
Conversation
Codecov Report
@@ Coverage Diff @@
## master #14865 +/- ##
==========================================
- Coverage 77.62% 77.58% -0.05%
==========================================
Files 963 963
Lines 49236 49304 +68
Branches 6192 6213 +21
==========================================
+ Hits 38219 38252 +33
- Misses 10817 10851 +34
- Partials 200 201 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
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.
Yet another awesome improvement to the already awesome highlighting feature! 🤩 LGTM, minor non-blocking comment
) { | ||
return; | ||
} | ||
dashboardLayout[childId].children.forEach(childId => |
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.
Just to avoid confusion, would it make sense to call the second childId
something like subChildId
?
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.
Nice catch, I haven't noticed that name clash. Fixed
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.
product signoff, thank you so much!🙏 i will take a better look after all PR merged.
…sed native filter (apache#14865) * feat(dashboard): Highlight tabs that contain a chart in scope of focused native filter * Optimizations and improvements * Use Set instead of array * Simplify logic * Change variable name
…sed native filter (apache#14865) * feat(dashboard): Highlight tabs that contain a chart in scope of focused native filter * Optimizations and improvements * Use Set instead of array * Simplify logic * Change variable name
…sed native filter (apache#14865) * feat(dashboard): Highlight tabs that contain a chart in scope of focused native filter * Optimizations and improvements * Use Set instead of array * Simplify logic * Change variable name
SUMMARY
When a user focuses a native filter, tabs that contain charts in scope of that filter should get highlighted to indicate that the native filter affects contents of those tabs.
I didn't highlight active tabs as I thought it would be an overkill, since we also highlight visible charts
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Screen.Recording.2021-05-27.at.13.03.33.mov
TESTING INSTRUCTIONS
DASHBOARD_NATIVE_FILTERS
feature flag to trueADDITIONAL INFORMATION
CC: @villebro @junlincc @graceguo-supercat