Re: [PR] refactor(Dropdown): Migrate Dropdown to Ant Design 5 [superset]

["geido (via GitHub)" <gi...@apache.org>]

geido commented on code in PR #31972:
URL: https://github.com/apache/superset/pull/31972#discussion_r1943352935


##########
superset-frontend/src/components/Dropdown/index.tsx:
##########
@@ -126,8 +124,13 @@ export const NoAnimationDropdown = (props: NoAnimationDropdownProps) => {
   });
 
   return (
-    <Antd5Dropdown overlayStyle={props.overlayStyle} {...rest}>
+    <AntdDropdown overlayStyle={props.overlayStyle} {...rest}>
       {childrenWithProps}
-    </Antd5Dropdown>
+    </AntdDropdown>
   );
 };
+
+export const Dropdown = Object.assign(AntdDropdown, {
+  Button: AntdDropdown.Button,

Review Comment:
   Is this assignment actually needed?



##########
superset-frontend/src/components/Chart/MenuItemWithTruncation.tsx:
##########
@@ -28,22 +28,25 @@ export type MenuItemWithTruncationProps = {
   children: ReactNode;
   onClick?: MenuItemProps['onClick'];
   style?: CSSProperties;
+  menuKey?: string;
 };
 
 export const MenuItemWithTruncation = ({
   tooltipText,
   children,
-  ...props
+  menuKey,
+  ...rest
 }: MenuItemWithTruncationProps) => {
   const [itemRef, itemIsTruncated] = useCSSTextTruncation<HTMLDivElement>();
 
   return (
     <Menu.Item
+      {...rest}

Review Comment:
   Nit: Should this go at the end?



##########
superset-frontend/src/features/home/RightMenu.tsx:
##########
@@ -409,6 +409,7 @@ const RightMenu = ({
         {RightMenuExtension && <RightMenuExtension />}
         {!navbarRight.user_is_anonymous && showActionDropdown && (
           <StyledSubMenu
+            key="sub1"

Review Comment:
   Do we have a better name for this key? Just making sure it is consistent with naming conventions across the codebase.



##########
superset-frontend/src/dashboard/components/menu/ShareMenuItems/index.tsx:
##########
@@ -52,8 +53,10 @@ const ShareMenuItems = (props: ShareMenuItemProps) => {
     addSuccessToast,
     dashboardId,
     dashboardComponentId,
-    key,
     title,
+    disabled,
+    submenuKey,

Review Comment:
   Is there a reason why these keys need to come from the component that's calling? 



-- 
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-unsubscribe@superset.apache.org

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


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