Re: [PR] fix(modals): restore App wrapper to fix Modal.confirm dark mode theming [superset]

Thu, 18 Sep 2025 15:09:31 -0700 


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


##########
superset-frontend/packages/superset-ui-core/src/components/Modal/Modal.tsx:
##########
@@ -368,9 +368,13 @@ const CustomModal = ({
 };
 CustomModal.displayName = 'Modal';
 
+// Theme-aware confirmation modal - now inherits theme through App wrapper in 
SupersetThemeProvider
+const themedConfirm = (props: Parameters<typeof AntdModal.confirm>[0]) =>
+  AntdModal.confirm(props);

Review Comment:
   ### Unnecessary function wrapper adds runtime overhead <sub>![category 
Performance](https://img.shields.io/badge/Performance-4f46e5)</sub>
   
   <details>
     <summary>Tell me more</summary>
   
   ###### What is the issue?
   The themedConfirm wrapper creates an unnecessary function call layer without 
adding any functionality, as it directly passes through all props to 
AntdModal.confirm without modification.
   
   
   ###### Why this matters
   This adds runtime overhead by creating an extra function call for every 
Modal.confirm invocation, while the comment suggests theme inheritance should 
happen through App wrapper in SupersetThemeProvider, not through this 
passthrough function.
   
   ###### Suggested change ∙ *Feature Preview*
   Either remove the wrapper and use `AntdModal.confirm` directly, or implement 
actual theme-aware logic within the wrapper. If theme inheritance happens at 
the provider level as the comment suggests, the wrapper serves no purpose:
   
   ```typescript
   export const Modal = Object.assign(CustomModal, {
     error: AntdModal.error,
     warning: AntdModal.warning,
     confirm: AntdModal.confirm, // Direct reference
     useModal: AntdModal.useModal,
   });
   ```
   
   
   ###### 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/2938b91e-8ae9-449f-8b0c-a645b1944940/upvote)
 
[![Incorrect](https://img.shields.io/badge/👎%20Incorrect-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/2938b91e-8ae9-449f-8b0c-a645b1944940?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/2938b91e-8ae9-449f-8b0c-a645b1944940?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/2938b91e-8ae9-449f-8b0c-a645b1944940?what_not_in_standard=true)
 
[![Other](https://img.shields.io/badge/👎%20Other-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/2938b91e-8ae9-449f-8b0c-a645b1944940)
   </details>
   
   <sub>
   
   💬 Looking for more details? Reply to this comment to chat with Korbit.
   </sub>
   
   <!--- korbi internal id:93e7ee9d-8861-4145-b4d1-8ee72fb66c1b -->
   
   
   [](93e7ee9d-8861-4145-b4d1-8ee72fb66c1b)



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