-
Notifications
You must be signed in to change notification settings - Fork 16.7k
fix(dashboard): fix dashboard header overflow issue #35989
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -96,14 +96,19 @@ const StickyPanel = styled.div<{ width: number }>` | |
| `; | ||
|
|
||
| // @z-index-above-dashboard-popovers (99) + 1 = 100 | ||
| const StyledHeader = styled.div` | ||
| ${({ theme }) => css` | ||
| const StyledHeader = styled.div<{ | ||
| filterBarWidth: number; | ||
| showVerticalFilterBar: boolean; | ||
| }>` | ||
| ${({ theme, filterBarWidth, showVerticalFilterBar }) => css` | ||
| grid-column: 2; | ||
| grid-row: 1; | ||
| position: sticky; | ||
| top: 0; | ||
| z-index: 99; | ||
| max-width: 100vw; | ||
| max-width: ${showVerticalFilterBar | ||
| ? `calc(100vw - ${filterBarWidth}px)` | ||
| : '100vw'}; | ||
|
|
||
| .empty-droptarget:before { | ||
| position: absolute; | ||
|
|
@@ -561,6 +566,15 @@ const DashboardBuilder = () => { | |
| ? theme.sizeUnit * 4 | ||
| : theme.sizeUnit * 8; | ||
|
|
||
| // Calculate filter bar width for header max-width | ||
| const showVerticalFilterBar = | ||
| showFilterBar && filterBarOrientation === FilterBarOrientation.Vertical; | ||
| const headerFilterBarWidth = showVerticalFilterBar | ||
| ? dashboardFiltersOpen | ||
| ? OPEN_FILTER_BAR_WIDTH // Use default open width for header calculation | ||
| : CLOSED_FILTER_BAR_WIDTH | ||
| : 0; | ||
|
Comment on lines
+570
to
+576
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unnecessary recalculation of filter bar width
Tell me moreWhat is the issue?The filter bar width calculation is performed on every render, even when the dependencies haven't changed. Why this mattersThis causes unnecessary computations on each render cycle, potentially impacting performance in components that re-render frequently, especially when dealing with complex dashboard layouts. Suggested change ∙ Feature PreviewWrap the filter bar width calculations in const { showVerticalFilterBar, headerFilterBarWidth } = useMemo(() => {
const showVertical = showFilterBar && filterBarOrientation === FilterBarOrientation.Vertical;
const width = showVertical
? dashboardFiltersOpen
? OPEN_FILTER_BAR_WIDTH
: CLOSED_FILTER_BAR_WIDTH
: 0;
return { showVerticalFilterBar: showVertical, headerFilterBarWidth: width };
}, [showFilterBar, filterBarOrientation, dashboardFiltersOpen]);Provide feedback to improve future suggestions💬 Looking for more details? Reply to this comment to chat with Korbit. |
||
|
|
||
| const renderChild = useCallback( | ||
| adjustedWidth => { | ||
| const filterBarWidth = dashboardFiltersOpen | ||
|
|
@@ -614,7 +628,11 @@ const DashboardBuilder = () => { | |
| </ResizableSidebar> | ||
| </> | ||
| )} | ||
| <StyledHeader ref={headerRef}> | ||
| <StyledHeader | ||
| ref={headerRef} | ||
| filterBarWidth={headerFilterBarWidth} | ||
| showVerticalFilterBar={showVerticalFilterBar} | ||
| > | ||
| {/* @ts-ignore */} | ||
| <Droppable | ||
| data-test="top-level-tabs" | ||
|
|
||
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.
Header uses fixed width instead of actual resized filter bar width
Tell me more
What is the issue?
The header max-width calculation uses a fixed OPEN_FILTER_BAR_WIDTH instead of the actual dynamic width when the filter bar is resizable.
Why this matters
When users resize the vertical filter bar, the header will still use the default width for its max-width calculation, potentially causing layout issues or incorrect spacing. The header should respond to the actual resized width, not just the default width.
Suggested change ∙ Feature Preview
The header should use the same dynamic width calculation as the filter bar itself. Consider passing the actual
adjustedWidthfrom the ResizableSidebar to the header calculation, or use a shared state/ref to track the current filter bar width that both components can access.Provide feedback to improve future suggestions
💬 Looking for more details? Reply to this comment to chat with Korbit.