-
Notifications
You must be signed in to change notification settings - Fork 8.5k
Fix for Distribution Bar Filter Segments #230084
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
Conversation
|
Pinging @elastic/kibana-cloud-security-posture (Team:Cloud Security) |
opauloh
left a comment
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 work, just a few suggestions
| const parts = stats.map((stat) => { | ||
|
|
||
| const [hoveredKey, setHoveredKey] = useState<string | null>(null); | ||
| const hasCurrentFilter = stats.some((item) => item.isCurrentFilter === true); |
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.
nit: add useMemo to prevent recalculating on every render.:
| const hasCurrentFilter = stats.some((item) => item.isCurrentFilter === true); | |
| const hasCurrentFilter = useMemo( | |
| () => stats.some((item) => item.isCurrentFilter), | |
| [stats] | |
| ); | |
| // Only show tooltip for segments thats hovered OR Set as Filter OR Last | ||
| const showTooltip = | ||
| isHovered || | ||
| (!hoveredKey && | ||
| ((stat.isCurrentFilter ?? false) || (!hideLastTooltip && !hasCurrentFilter && isLast))); |
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.
So the complexity grew here and there are a few business logic in these lines, I suggest splitting into a separate function with each condition split into variables, example (feel free to improve):
// Only show tooltip for segments thats hovered OR Set as Filter OR Last
const shouldShowTooltip = ({
isHovered,
isCurrentFilter,
isLast,
hasFilterActive,
hideLastTooltip,
isHoveringAnyStatsBar,
}: {
isHovered: boolean;
isCurrentFilter: boolean;
isLast: boolean;
hasFilterActive: boolean;
hideLastTooltip?: boolean;
isHoveringAnyStatsBar: boolean;
}) => {
if (isHovered) return true;
const shouldShowBecauseOfFilter = isCurrentFilter;
const shouldShowBecauseItIsLast = isLast && !hideLastTooltip && !hasFilterActive;
return !isHoveringAnyStatsBar && (shouldShowBecauseOfFilter || shouldShowBecauseItIsLast);
};usage
const isCurrentFilter = stat.isCurrentFilter ?? false;
const showTooltip = shouldShowTooltip({
isHovered,
isCurrentFilter,
isLast,
hasFilterActive: hasCurrentFilter,
hideLastTooltip,
isHoveringAnyStatsBar: Boolean(hoveredKey),
});…bana into fix-distribution-bar-filter
opauloh
left a comment
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.
LGTM
💚 Build Succeeded
Metrics [docs]Async chunks
History
|
## Summary This PR addresses the Issue where after user clicks on Distribution bar segments to filter data, the tooltips defaults to Last Segment instead of Chosen filter (It should only default to Last segment IF there are no filter) https://github.com/user-attachments/assets/c3dbc3ac-846d-41ea-8a9a-6b05c53a7cab
## Summary This PR addresses the Issue where after user clicks on Distribution bar segments to filter data, the tooltips defaults to Last Segment instead of Chosen filter (It should only default to Last segment IF there are no filter) https://github.com/user-attachments/assets/c3dbc3ac-846d-41ea-8a9a-6b05c53a7cab
## Summary This PR addresses the Issue where after user clicks on Distribution bar segments to filter data, the tooltips defaults to Last Segment instead of Chosen filter (It should only default to Last segment IF there are no filter) https://github.com/user-attachments/assets/c3dbc3ac-846d-41ea-8a9a-6b05c53a7cab
Summary
This PR addresses the Issue where after user clicks on Distribution bar segments to filter data, the tooltips defaults to Last Segment instead of Chosen filter (It should only default to Last segment IF there are no filter)
Screen.Recording.2025-07-30.at.8.11.25.PM.mov