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


##########
superset-frontend/src/dashboard/components/DashboardBuilder/state.ts:
##########
@@ -68,8 +69,11 @@ export const useNativeFilters = () => {
 
   useEffect(() => {
     if (
-      expandFilters === false ||
-      (filterValues.length === 0 && nativeFiltersEnabled)
+      isFeatureEnabled(FeatureFlag.FilterBarClosedByDefault) &&
+      expandFilters === null
+        ? true
+        : expandFilters === false ||
+          (filterValues.length === 0 && nativeFiltersEnabled)
     ) {

Review Comment:
   ### Incorrect Filter Panel Default State Logic <sub>![category 
Functionality](https://img.shields.io/badge/Functionality-0284c7)</sub>
   
   <details>
     <summary>Tell me more</summary>
   
   ###### What is the issue?
   The conditional logic has incorrect operator precedence and confusing 
ternary usage that doesn't match the intended functionality of hiding the 
filter panel by default.
   
   ###### Why this matters
   The current logic will cause the filter panel to remain open in scenarios 
where it should be closed, not fulfilling the requirement of having filters 
closed by default when the feature flag is enabled.
   
   ###### Suggested change ∙ *Feature Preview*
   Restructure the condition to properly handle the feature flag and 
expandFilters cases:
   ```typescript
   if (
     (isFeatureEnabled(FeatureFlag.FilterBarClosedByDefault) && expandFilters 
=== null) ||
     expandFilters === false ||
     (filterValues.length === 0 && nativeFiltersEnabled)
   )
   ```
   
   
   ###### 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/d90e19bd-7755-44e9-81e7-d92e1daecd69/upvote)
 
[![Incorrect](https://img.shields.io/badge/👎%20Incorrect-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/d90e19bd-7755-44e9-81e7-d92e1daecd69?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/d90e19bd-7755-44e9-81e7-d92e1daecd69?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/d90e19bd-7755-44e9-81e7-d92e1daecd69?what_not_in_standard=true)
 
[![Other](https://img.shields.io/badge/👎%20Other-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/d90e19bd-7755-44e9-81e7-d92e1daecd69)
   </details>
   
   <sub>
   
   💬 Looking for more details? Reply to this comment to chat with Korbit.
   </sub>
   
   <!--- korbi internal id:91d35a5c-5ea6-4265-8cdc-1fe03f12fe1e -->
   
   [](91d35a5c-5ea6-4265-8cdc-1fe03f12fe1e)



-- 
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: notifications-unsubscr...@superset.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org
For additional commands, e-mail: notifications-h...@superset.apache.org

Reply via email to