Re: [PR] fix: database model Collapse state [superset]

[via GitHub]

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


##########
superset-frontend/src/features/databases/DatabaseModal/ExtraOptions.tsx:
##########
@@ -88,12 +88,21 @@ const ExtraOptions = ({
   const isAllowRunAsyncDisabled = isFeatureEnabled(
     FeatureFlag.ForceSqlLabRunAsync,
   );
+  const [activeKey, setActiveKey] = useState<string[] | undefined>();
+
+  useEffect(() => {
+    if (!expandableModalIsOpen) {
+      setActiveKey(undefined);
+    }
+  }, [expandableModalIsOpen]);

Review Comment:
   ### Unnecessary State Updates in useEffect <sub>![category 
Performance](https://img.shields.io/badge/Performance-4f46e5)</sub>
   
   <details>
     <summary>Tell me more</summary>
   
   ###### What is the issue?
   The component re-renders unnecessarily when expandableModalIsOpen changes, 
even if activeKey is already undefined.
   
   
   ###### Why this matters
   This causes unnecessary re-renders and DOM updates, impacting performance 
when the modal is frequently toggled.
   
   ###### Suggested change ∙ *Feature Preview*
   Add a condition to check if activeKey is already undefined before setting it:
   ```typescript
   useEffect(() => {
       if (!expandableModalIsOpen && activeKey !== undefined) {
         setActiveKey(undefined);
       }
   }, [expandableModalIsOpen, activeKey]);
   ```
   
   
   ###### 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/74ab9509-840d-4dc3-9875-43d7c987b8da/upvote)
 
[![Incorrect](https://img.shields.io/badge/👎%20Incorrect-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/74ab9509-840d-4dc3-9875-43d7c987b8da?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/74ab9509-840d-4dc3-9875-43d7c987b8da?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/74ab9509-840d-4dc3-9875-43d7c987b8da?what_not_in_standard=true)
 
[![Other](https://img.shields.io/badge/👎%20Other-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/74ab9509-840d-4dc3-9875-43d7c987b8da)
   </details>
   
   <sub>
   
   💬 Looking for more details? Reply to this comment to chat with Korbit.
   </sub>
   
   <!--- korbi internal id:7dc20ddf-cffc-4c30-9b82-6f0aafe914e1 -->
   
   
   [](7dc20ddf-cffc-4c30-9b82-6f0aafe914e1)



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