sadpandajoe commented on code in PR #36120:
URL: https://github.com/apache/superset/pull/36120#discussion_r2528568098


##########
superset-frontend/src/features/home/Menu.tsx:
##########
@@ -261,73 +231,63 @@ export function Menu({
   const standalone = getUrlParam(URL_PARAMS.standalone);
   if (standalone || uiConfig.hideNav) return <></>;
 
-  const renderSubMenu = ({
+  const buildMenuItem = ({
     label,
     childs,
     url,
-    index,
     isFrontendRoute,
-  }: MenuObjectProps) => {
+  }: MenuObjectProps): MenuItem => {
     if (url && isFrontendRoute) {
-      return (
-        <MainNav.Item key={label} role="presentation">
+      return {
+        key: label,
+        label: (
           <NavLink role="button" to={url} activeClassName="is-active">
             {label}
           </NavLink>
-        </MainNav.Item>
-      );
+        ),
+      };
     }
+
     if (url) {
-      return (
-        <MainNav.Item key={label}>
-          <Typography.Link href={url}>{label}</Typography.Link>
-        </MainNav.Item>
-      );
+      return {
+        key: label,
+        label: <Typography.Link href={url}>{label}</Typography.Link>,
+      };
     }
-    return (
-      <StyledSubMenu
-        key={label}
-        title={label}
-        popupOffset={[0, -8]}
-        icon={
-          showMenu === 'inline' ? <></> : <Icons.DownOutlined iconSize="xs" />
-        }
-      >
-        {childs?.map((child: MenuObjectChildProps | string, index1: number) => 
{
-          if (typeof child === 'string' && child === '-' && label !== 'Data') {
-            return <MainNav.Divider key={`$${index1}`} />;
-          }
-          if (typeof child !== 'string') {
-            return (
-              <MainNav.Item key={`${child.label}`}>
-                {child.isFrontendRoute ? (
-                  <NavLink
-                    to={child.url || ''}
-                    exact
-                    activeClassName="is-active"
-                  >
-                    {child.label}
-                  </NavLink>
-                ) : (
-                  <Typography.Link href={child.url}>
-                    {child.label}
-                  </Typography.Link>
-                )}
-              </MainNav.Item>
-            );
-          }
-          return null;
-        })}
-      </StyledSubMenu>
-    );
+
+    const childItems: MenuItem[] = [];
+    childs?.forEach((child: MenuObjectChildProps | string, index1: number) => {
+      if (typeof child === 'string' && child === '-' && label !== 'Data') {
+        childItems.push({ type: 'divider', key: `divider-${index1}` });
+      } else if (typeof child !== 'string') {
+        childItems.push({
+          key: `${child.label}`,
+          label: child.isFrontendRoute ? (
+            <NavLink to={child.url || ''} exact activeClassName="is-active">
+              {child.label}
+            </NavLink>
+          ) : (
+            <Typography.Link href={child.url}>{child.label}</Typography.Link>
+          ),
+        });
+      }
+    });
+
+    return {
+      key: label,
+      label,
+      icon: <Icons.DownOutlined iconSize="xs" />,
+      popupOffset: [0, -8],

Review Comment:
   nit: we use this offset in a lot of places, wondering if it makes sense to 
make this a shared constant. Would this offset ever change for other things?



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