mistercrunch commented on code in PR #33002:
URL: https://github.com/apache/superset/pull/33002#discussion_r2196098702


##########
superset-frontend/packages/superset-ui-core/src/components/Dropdown/index.tsx:
##########
@@ -110,8 +110,61 @@ export const NoAnimationDropdown = (props: 
NoAnimationDropdownProps) => {
   );
 };
 
+const CustomDropdownButton = (props: ButtonProps & DropdownProps) => {

Review Comment:
   Mmmmh, so 
`superset-frontend/packages/superset-ui-core/src/components/DropdownButton/index.tsx`
 doesn't work as expected? I just looked at it in storybook and it seems broken 
there. 
   
   But I'd say we shouldn't use `antd.Dropdown.Button` directly ideally as it 
would bypass our own `<Button />` wrapper and potentially look different and 
diverge from other buttons in the app.
   
   I'd say ideally 
`superset-frontend/packages/superset-ui-core/src/components/DropdownButton/index.tsx`
 should be fixed up to use our own `Button` instead of `Dropdown.Button` 
(assuming that's possible), and consistently using it across the codebase. 
   
   Sorry to put you through this but we've been doing a lot of component 
library refactor as we upgraded from antd v4 to v5, and many components are not 
what they should be, or aren't used properly in the app. Trying to avoid adding 
yet another layer in there.



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