korbit-ai[bot] commented on code in PR #35984:
URL: https://github.com/apache/superset/pull/35984#discussion_r2490359844


##########
superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx:
##########
@@ -426,6 +426,7 @@ const DashboardBuilder = () => {
     isReport;
 
   const [barTopOffset, setBarTopOffset] = useState(0);
+  const [currentFilterBarWidth, setCurrentFilterBarWidth] = useState(0);

Review Comment:
   ### Initial filter bar width state mismatch <sub>![category 
Functionality](https://img.shields.io/badge/Functionality-0284c7)</sub>
   
   <details>
     <summary>Tell me more</summary>
   
   ###### What is the issue?
   The currentFilterBarWidth state is initialized to 0 but may not reflect the 
actual filter bar width on initial render when dashboardFiltersOpen is true.
   
   
   ###### Why this matters
   This could cause the header's max-width calculation to be incorrect during 
the initial render, potentially leading to layout issues or content overflow 
until the state is properly updated.
   
   ###### Suggested change ∙ *Feature Preview*
   Initialize the state with the correct width based on the initial 
dashboardFiltersOpen state:
   
   ```typescript
   const [currentFilterBarWidth, setCurrentFilterBarWidth] = useState(
     dashboardFiltersOpen ? OPEN_FILTER_BAR_WIDTH : CLOSED_FILTER_BAR_WIDTH
   );
   ```
   
   
   ###### Provide feedback to improve future suggestions
   [![Nice 
Catch](https://img.shields.io/badge/👍%20Nice%20Catch-71BC78)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/8a57db0d-ce5a-4b7c-8d3f-cac9aa520452/upvote)
 
[![Incorrect](https://img.shields.io/badge/👎%20Incorrect-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/8a57db0d-ce5a-4b7c-8d3f-cac9aa520452?what_not_true=true)
  [![Not in 
Scope](https://img.shields.io/badge/👎%20Out%20of%20PR%20scope-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/8a57db0d-ce5a-4b7c-8d3f-cac9aa520452?what_out_of_scope=true)
 [![Not in coding 
standard](https://img.shields.io/badge/👎%20Not%20in%20our%20standards-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/8a57db0d-ce5a-4b7c-8d3f-cac9aa520452?what_not_in_standard=true)
 
[![Other](https://img.shields.io/badge/👎%20Other-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/8a57db0d-ce5a-4b7c-8d3f-cac9aa520452)
   </details>
   
   <sub>
   
   💬 Looking for more details? Reply to this comment to chat with Korbit.
   </sub>
   
   <!--- korbi internal id:ed982a52-9ffb-4fea-aea2-28574594e2e3 -->
   
   
   [](ed982a52-9ffb-4fea-aea2-28574594e2e3)



##########
superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx:
##########
@@ -426,6 +426,7 @@ const DashboardBuilder = () => {
     isReport;
 
   const [barTopOffset, setBarTopOffset] = useState(0);
+  const [currentFilterBarWidth, setCurrentFilterBarWidth] = useState(0);

Review Comment:
   ### Fragmented State Management <sub>![category 
Design](https://img.shields.io/badge/Design-0d9488)</sub>
   
   <details>
     <summary>Tell me more</summary>
   
   ###### What is the issue?
   Component manages multiple pieces of related state separately that could be 
combined into a single state object.
   
   
   ###### Why this matters
   Scattered state management increases complexity and makes it harder to 
maintain state consistency across the component.
   
   ###### Suggested change ∙ *Feature Preview*
   Combine related state into a single object:
   ```typescript
   interface LayoutState {
     barTopOffset: number;
     filterBarWidth: number;
   }
   
   const [layoutState, setLayoutState] = useState<LayoutState>({
     barTopOffset: 0,
     filterBarWidth: 0
   });
   ```
   
   
   ###### Provide feedback to improve future suggestions
   [![Nice 
Catch](https://img.shields.io/badge/👍%20Nice%20Catch-71BC78)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/5596f4af-169b-48f8-b36b-ea07d08921a3/upvote)
 
[![Incorrect](https://img.shields.io/badge/👎%20Incorrect-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/5596f4af-169b-48f8-b36b-ea07d08921a3?what_not_true=true)
  [![Not in 
Scope](https://img.shields.io/badge/👎%20Out%20of%20PR%20scope-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/5596f4af-169b-48f8-b36b-ea07d08921a3?what_out_of_scope=true)
 [![Not in coding 
standard](https://img.shields.io/badge/👎%20Not%20in%20our%20standards-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/5596f4af-169b-48f8-b36b-ea07d08921a3?what_not_in_standard=true)
 
[![Other](https://img.shields.io/badge/👎%20Other-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/5596f4af-169b-48f8-b36b-ea07d08921a3)
   </details>
   
   <sub>
   
   💬 Looking for more details? Reply to this comment to chat with Korbit.
   </sub>
   
   <!--- korbi internal id:8a6ab628-45bb-443e-8d71-5c6222a18d9b -->
   
   
   [](8a6ab628-45bb-443e-8d71-5c6222a18d9b)



-- 
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]

Reply via email to