korbit-ai[bot] commented on code in PR #35989:
URL: https://github.com/apache/superset/pull/35989#discussion_r2491382409
##########
superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx:
##########
@@ -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;
Review Comment:
### Unnecessary recalculation of filter bar width <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The filter bar width calculation is performed on every render, even when the
dependencies haven't changed.
###### Why this matters
This 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 Preview*
Wrap the filter bar width calculations in `useMemo` to memoize the results
based on their dependencies:
```typescript
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
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/6a89a7a5-7f8e-435a-8b7e-9239003dd2d7/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/6a89a7a5-7f8e-435a-8b7e-9239003dd2d7?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/6a89a7a5-7f8e-435a-8b7e-9239003dd2d7?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/6a89a7a5-7f8e-435a-8b7e-9239003dd2d7?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/6a89a7a5-7f8e-435a-8b7e-9239003dd2d7)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:ddf73927-25e6-4e28-abe1-fbcde74c8170 -->
[](ddf73927-25e6-4e28-abe1-fbcde74c8170)
##########
superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx:
##########
@@ -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;
Review Comment:
### Header uses fixed width instead of actual resized filter bar width
<sub></sub>
<details>
<summary>Tell me more</summary>
###### 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 `adjustedWidth` from 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
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/16c864b3-ca33-46ed-b771-d2bfc060759c/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/16c864b3-ca33-46ed-b771-d2bfc060759c?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/16c864b3-ca33-46ed-b771-d2bfc060759c?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/16c864b3-ca33-46ed-b771-d2bfc060759c?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/16c864b3-ca33-46ed-b771-d2bfc060759c)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:dc5d08ce-f981-4f43-b4e0-738916437f8e -->
[](dc5d08ce-f981-4f43-b4e0-738916437f8e)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
