You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by ma...@apache.org on 2025/08/16 22:22:35 UTC

(superset) branch menu-caret created (now a9f98f4f08)

This is an automated email from the ASF dual-hosted git repository.

maximebeauchemin pushed a change to branch menu-caret
in repository https://gitbox.apache.org/repos/asf/superset.git


      at a9f98f4f08 fix: standardize menu caret positioning with shared styled component

This branch includes the following new commits:

     new a9f98f4f08 fix: standardize menu caret positioning with shared styled component

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.



(superset) 01/01: fix: standardize menu caret positioning with shared styled component

Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

maximebeauchemin pushed a commit to branch menu-caret
in repository https://gitbox.apache.org/repos/asf/superset.git

commit a9f98f4f08a4cf8f98f14eb5b61544ac7d3f3d5b
Author: Maxime Beauchemin <ma...@gmail.com>
AuthorDate: Sat Aug 16 15:21:38 2025 -0700

    fix: standardize menu caret positioning with shared styled component
    
    - Create shared StyledDropdownSubMenu component with consistent caret styling
    - Update left menu (Menu.tsx) to use shared component instead of local implementation
    - Refactor right menu (RightMenu.tsx) from MenuItem API to SubMenu components for consistency
    - Fix caret positioning so all dropdown menus show caret on right side of label
    - Remove code duplication between left and right menu implementations
    
    🤖 Generated with [Claude Code](https://claude.ai/code)
    
    Co-Authored-By: Claude <no...@anthropic.com>
---
 superset-frontend/src/features/home/Menu.tsx       |  27 +-
 superset-frontend/src/features/home/RightMenu.tsx  | 516 ++++++++++-----------
 .../src/features/home/StyledDropdownSubMenu.tsx    |  45 ++
 3 files changed, 301 insertions(+), 287 deletions(-)

diff --git a/superset-frontend/src/features/home/Menu.tsx b/superset-frontend/src/features/home/Menu.tsx
index acfedb981b..c8588ba23d 100644
--- a/superset-frontend/src/features/home/Menu.tsx
+++ b/superset-frontend/src/features/home/Menu.tsx
@@ -17,7 +17,7 @@
  * under the License.
  */
 import { useState, useEffect } from 'react';
-import { styled, css, useTheme } from '@superset-ui/core';
+import { styled, useTheme } from '@superset-ui/core';
 import { debounce } from 'lodash';
 import { getUrlParam } from 'src/utils/urlUtils';
 import { MainNav, MenuMode } from '@superset-ui/core/components/Menu';
@@ -34,6 +34,7 @@ import {
   MenuData,
 } from 'src/types/bootstrapTypes';
 import RightMenu from './RightMenu';
+import { StyledDropdownSubMenu } from './StyledDropdownSubMenu';
 
 interface MenuProps {
   data: MenuData;
@@ -125,26 +126,6 @@ const StyledHeader = styled.header`
       }
   `}
 `;
-const { SubMenu } = MainNav;
-
-const StyledSubMenu = styled(SubMenu)`
-  ${({ theme }) => css`
-    [data-icon="caret-down"] {
-      color: ${theme.colors.grayscale.base};
-      font-size: ${theme.fontSizeXS}px;
-      margin-left: ${theme.sizeUnit}px;
-    }
-    &.ant-menu-submenu {
-        padding: ${theme.sizeUnit * 2}px ${theme.sizeUnit * 4}px;
-        display: flex;
-        align-items: center;
-        height: 100%;  &.ant-menu-submenu-active {
-    .ant-menu-title-content {
-      color: ${theme.colorPrimary};
-    }
-  }
-  `}
-`;
 const { useBreakpoint } = Grid;
 
 export function Menu({
@@ -228,7 +209,7 @@ export function Menu({
       );
     }
     return (
-      <StyledSubMenu
+      <StyledDropdownSubMenu
         key={index}
         title={label}
         icon={
@@ -264,7 +245,7 @@ export function Menu({
           }
           return null;
         })}
-      </StyledSubMenu>
+      </StyledDropdownSubMenu>
     );
   };
   const renderBrand = () => {
diff --git a/superset-frontend/src/features/home/RightMenu.tsx b/superset-frontend/src/features/home/RightMenu.tsx
index b86f98d415..5e2994239e 100644
--- a/superset-frontend/src/features/home/RightMenu.tsx
+++ b/superset-frontend/src/features/home/RightMenu.tsx
@@ -16,7 +16,7 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import { useState, useEffect, FC, PureComponent, useMemo } from 'react';
+import { useState, useEffect, FC, PureComponent } from 'react';
 import rison from 'rison';
 import { useSelector } from 'react-redux';
 import { Link } from 'react-router-dom';
@@ -39,14 +39,12 @@ import {
   Typography,
   TelemetryPixel,
 } from '@superset-ui/core/components';
-import type { MenuItem } from '@superset-ui/core/components/Menu';
 import { ensureAppRoot } from 'src/utils/pathUtils';
 import { findPermission } from 'src/utils/findPermission';
 import { isUserAdmin } from 'src/dashboard/util/permissionUtils';
 import {
   MenuObjectProps,
   UserWithPermissionsAndRoles,
-  MenuObjectChildProps,
 } from 'src/types/bootstrapTypes';
 import { RootState } from 'src/dashboard/types';
 import DatabaseModal from 'src/features/databases/DatabaseModal';
@@ -55,6 +53,7 @@ import { uploadUserPerms } from 'src/views/CRUD/utils';
 import { useThemeContext } from 'src/theme/ThemeProvider';
 import { useThemeMenuItems } from 'src/hooks/useThemeMenuItems';
 import { useLanguageMenuItems } from './LanguagePicker';
+import { StyledDropdownSubMenu } from './StyledDropdownSubMenu';
 import {
   ExtensionConfigs,
   GlobalMenuDataOptions,
@@ -307,24 +306,6 @@ const RightMenu = ({
     "Enable 'Allow file uploads to database' in any database's settings",
   );
 
-  const buildMenuItem = (item: MenuObjectChildProps): MenuItem => ({
-    key: item.name || item.label,
-    label: item.disable ? (
-      <StyledMenuItem disabled>
-        <Tooltip placement="top" title={tooltipText}>
-          {item.label}
-        </Tooltip>
-      </StyledMenuItem>
-    ) : item.url ? (
-      <Typography.Link href={ensureAppRoot(item.url)}>
-        {item.label}
-      </Typography.Link>
-    ) : (
-      item.label
-    ),
-    disabled: item.disable,
-  });
-
   const onMenuOpen = (openKeys: string[]) => {
     // We should query the API only if opening Data submenus
     // because the rest don't need this information. Not using
@@ -367,247 +348,6 @@ const RightMenu = ({
     languages: navbarRight.languages || {},
   });
 
-  // Build main menu items
-  const menuItems = useMemo(() => {
-    // Build menu items for the new dropdown
-    const buildNewDropdownItems = (): MenuItem[] => {
-      const items: MenuItem[] = [];
-
-      dropdownItems?.forEach(menu => {
-        const canShowChild = menu.childs?.some(
-          item => typeof item === 'object' && !!item.perm,
-        );
-
-        if (menu.childs) {
-          if (canShowChild) {
-            const childItems: MenuItem[] = [];
-            menu.childs.forEach((item, idx) => {
-              if (typeof item !== 'string' && item.name && item.perm) {
-                if (idx === 3) {
-                  childItems.push({ type: 'divider', key: `divider-${idx}` });
-                }
-                childItems.push(buildMenuItem(item));
-              }
-            });
-
-            items.push({
-              key: `sub2_${menu.label}`,
-              label: menu.label,
-              icon: menu.icon,
-              children: childItems,
-            });
-          } else if (menu.url) {
-            if (
-              findPermission(menu.perm as string, menu.view as string, roles)
-            ) {
-              items.push({
-                key: menu.label,
-                label: isFrontendRoute(menu.url) ? (
-                  <Link to={menu.url || ''}>
-                    {menu.icon} {menu.label}
-                  </Link>
-                ) : (
-                  <Typography.Link href={ensureAppRoot(menu.url || '')}>
-                    {menu.icon} {menu.label}
-                  </Typography.Link>
-                ),
-              });
-            }
-          }
-        } else if (
-          findPermission(menu.perm as string, menu.view as string, roles)
-        ) {
-          items.push({
-            key: menu.label,
-            label: isFrontendRoute(menu.url) ? (
-              <Link to={menu.url || ''}>
-                {menu.icon} {menu.label}
-              </Link>
-            ) : (
-              <Typography.Link href={ensureAppRoot(menu.url || '')}>
-                {menu.icon} {menu.label}
-              </Typography.Link>
-            ),
-          });
-        }
-      });
-
-      return items;
-    };
-
-    // Build settings menu items
-    const buildSettingsMenuItems = (): MenuItem[] => {
-      const items: MenuItem[] = [];
-
-      settings?.forEach((section, index) => {
-        const sectionItems: MenuItem[] = [];
-
-        section.childs?.forEach(child => {
-          if (typeof child !== 'string') {
-            const menuItemDisplay = RightMenuItemIconExtension ? (
-              <StyledMenuItemWithIcon>
-                {child.label}
-                <RightMenuItemIconExtension menuChild={child} />
-              </StyledMenuItemWithIcon>
-            ) : (
-              child.label
-            );
-
-            sectionItems.push({
-              key: child.label,
-              label: isFrontendRoute(child.url) ? (
-                <Link to={child.url || ''}>{menuItemDisplay}</Link>
-              ) : (
-                <Typography.Link href={child.url || ''}>
-                  {menuItemDisplay}
-                </Typography.Link>
-              ),
-            });
-          }
-        });
-
-        items.push({
-          type: 'group',
-          label: section.label,
-          key: section.label,
-          children: sectionItems,
-        });
-
-        if (index < settings.length - 1) {
-          items.push({ type: 'divider', key: `divider_${index}` });
-        }
-      });
-
-      if (!navbarRight.user_is_anonymous) {
-        items.push({ type: 'divider', key: 'user-divider' });
-
-        const userItems: MenuItem[] = [];
-        if (navbarRight.user_info_url) {
-          userItems.push({
-            key: 'info',
-            label: (
-              <Typography.Link href={navbarRight.user_info_url}>
-                {t('Info')}
-              </Typography.Link>
-            ),
-          });
-        }
-        userItems.push({
-          key: 'logout',
-          label: (
-            <Typography.Link href={navbarRight.user_logout_url}>
-              {t('Logout')}
-            </Typography.Link>
-          ),
-          onClick: handleLogout,
-        });
-
-        items.push({
-          type: 'group',
-          label: t('User'),
-          key: 'user-section',
-          children: userItems,
-        });
-      }
-
-      if (navbarRight.version_string || navbarRight.version_sha) {
-        items.push({ type: 'divider', key: 'version-info-divider' });
-
-        items.push({
-          type: 'group',
-          label: t('About'),
-          key: 'about-section',
-          children: [
-            {
-              key: 'about-info',
-              label: (
-                <div className="about-section">
-                  {navbarRight.show_watermark && (
-                    <div css={versionInfoStyles}>
-                      {t('Powered by Apache Superset')}
-                    </div>
-                  )}
-                  {navbarRight.version_string && (
-                    <div css={versionInfoStyles}>
-                      {t('Version')}: {navbarRight.version_string}
-                    </div>
-                  )}
-                  {navbarRight.version_sha && (
-                    <div css={versionInfoStyles}>
-                      {t('SHA')}: {navbarRight.version_sha}
-                    </div>
-                  )}
-                  {navbarRight.build_number && (
-                    <div css={versionInfoStyles}>
-                      {t('Build')}: {navbarRight.build_number}
-                    </div>
-                  )}
-                </div>
-              ),
-            },
-          ],
-        });
-      }
-
-      return items;
-    };
-
-    const items: MenuItem[] = [];
-
-    if (RightMenuExtension) {
-      items.push({
-        key: 'extension',
-        label: <RightMenuExtension />,
-      });
-    }
-
-    if (!navbarRight.user_is_anonymous && showActionDropdown) {
-      items.push({
-        key: 'new-dropdown',
-        label: (
-          <Icons.PlusOutlined
-            iconColor={theme.colorPrimary}
-            data-test="new-dropdown-icon"
-          />
-        ),
-        icon: <Icons.CaretDownOutlined iconSize="xs" />,
-        children: buildNewDropdownItems(),
-        ...{ 'data-test': 'new-dropdown' },
-      });
-    }
-
-    if (canSetMode()) {
-      items.push(themeMenuItem);
-    }
-
-    if (navbarRight.show_language_picker && languageMenuItem) {
-      items.push(languageMenuItem);
-    }
-
-    items.push({
-      key: 'settings',
-      label: t('Settings'),
-      icon: <Icons.CaretDownOutlined iconSize="xs" />,
-      children: buildSettingsMenuItems(),
-    });
-
-    return items;
-  }, [
-    RightMenuExtension,
-    navbarRight,
-    showActionDropdown,
-    canSetMode,
-    theme.colorPrimary,
-    themeMenuItem,
-    languageMenuItem,
-    dropdownItems,
-    roles,
-    settings,
-    RightMenuItemIconExtension,
-    buildMenuItem,
-    handleLogout,
-  ]);
-
   return (
     <StyledDiv align={align}>
       {canDatabase && (
@@ -679,8 +419,256 @@ const RightMenu = ({
         onClick={handleMenuSelection}
         onOpenChange={onMenuOpen}
         disabledOverflow
-        items={menuItems}
-      />
+      >
+        {RightMenuExtension && (
+          <Menu.Item key="extension">
+            <RightMenuExtension />
+          </Menu.Item>
+        )}
+
+        {!navbarRight.user_is_anonymous && showActionDropdown && (
+          <StyledDropdownSubMenu
+            key="new-dropdown"
+            title={
+              <Icons.PlusOutlined
+                iconColor={theme.colorPrimary}
+                data-test="new-dropdown-icon"
+              />
+            }
+            icon={<Icons.CaretDownOutlined iconSize="xs" />}
+            data-test="new-dropdown"
+          >
+            {dropdownItems?.map(menu => {
+              const canShowChild = menu.childs?.some(
+                item => typeof item === 'object' && !!item.perm,
+              );
+
+              if (menu.childs) {
+                if (canShowChild) {
+                  return (
+                    <Menu.SubMenu
+                      key={`sub2_${menu.label}`}
+                      title={menu.label}
+                      icon={menu.icon}
+                    >
+                      {menu.childs.map((item, idx) => {
+                        if (
+                          typeof item !== 'string' &&
+                          item.name &&
+                          item.perm
+                        ) {
+                          if (idx === 3) {
+                            return <Menu.Divider key={`divider-${idx}`} />;
+                          }
+                          return (
+                            <Menu.Item
+                              key={item.name || item.label}
+                              disabled={item.disable}
+                            >
+                              {item.disable ? (
+                                <StyledMenuItem disabled>
+                                  <Tooltip placement="top" title={tooltipText}>
+                                    {item.label}
+                                  </Tooltip>
+                                </StyledMenuItem>
+                              ) : item.url ? (
+                                <Typography.Link href={ensureAppRoot(item.url)}>
+                                  {item.label}
+                                </Typography.Link>
+                              ) : (
+                                item.label
+                              )}
+                            </Menu.Item>
+                          );
+                        }
+                        return null;
+                      })}
+                    </Menu.SubMenu>
+                  );
+                }
+                if (menu.url) {
+                  if (
+                    findPermission(
+                      menu.perm as string,
+                      menu.view as string,
+                      roles,
+                    )
+                  ) {
+                    return (
+                      <Menu.Item key={menu.label}>
+                        {isFrontendRoute(menu.url) ? (
+                          <Link to={menu.url || ''}>
+                            {menu.icon} {menu.label}
+                          </Link>
+                        ) : (
+                          <Typography.Link href={ensureAppRoot(menu.url || '')}>
+                            {menu.icon} {menu.label}
+                          </Typography.Link>
+                        )}
+                      </Menu.Item>
+                    );
+                  }
+                }
+              } else if (
+                findPermission(menu.perm as string, menu.view as string, roles)
+              ) {
+                return (
+                  <Menu.Item key={menu.label}>
+                    {isFrontendRoute(menu.url) ? (
+                      <Link to={menu.url || ''}>
+                        {menu.icon} {menu.label}
+                      </Link>
+                    ) : (
+                      <Typography.Link href={ensureAppRoot(menu.url || '')}>
+                        {menu.icon} {menu.label}
+                      </Typography.Link>
+                    )}
+                  </Menu.Item>
+                );
+              }
+              return null;
+            })}
+          </StyledDropdownSubMenu>
+        )}
+
+        {canSetMode() && themeMenuItem && (
+          <StyledDropdownSubMenu
+            key={themeMenuItem.key}
+            title={(themeMenuItem as any).label}
+            icon={<Icons.CaretDownOutlined iconSize="xs" />}
+          >
+            {(themeMenuItem as any).children?.map((child: any) => {
+              if (child.type === 'group') {
+                return (
+                  <Menu.ItemGroup key={child.key} title={child.label}>
+                    {child.children?.map((groupChild: any) => (
+                      <Menu.Item
+                        key={groupChild.key}
+                        onClick={groupChild.onClick}
+                      >
+                        {groupChild.label}
+                      </Menu.Item>
+                    ))}
+                  </Menu.ItemGroup>
+                );
+              }
+              if (child.type === 'divider') {
+                return <Menu.Divider key={child.key} />;
+              }
+              return (
+                <Menu.Item key={child.key} onClick={child.onClick}>
+                  {child.label}
+                </Menu.Item>
+              );
+            })}
+          </StyledDropdownSubMenu>
+        )}
+
+        {navbarRight.show_language_picker && languageMenuItem && (
+          <StyledDropdownSubMenu
+            key={languageMenuItem.key}
+            title={(languageMenuItem as any).label}
+            icon={<Icons.CaretDownOutlined iconSize="xs" />}
+          >
+            {(languageMenuItem as any).children?.map((child: any) => (
+              <Menu.Item key={child.key} onClick={child.onClick}>
+                {child.label}
+              </Menu.Item>
+            ))}
+          </StyledDropdownSubMenu>
+        )}
+
+        <StyledDropdownSubMenu
+          key="settings"
+          title={t('Settings')}
+          icon={<Icons.CaretDownOutlined iconSize="xs" />}
+        >
+          {settings?.map((section, index) => [
+            <Menu.ItemGroup key={section.label} title={section.label}>
+              {section.childs?.map(child => {
+                if (typeof child !== 'string') {
+                  const menuItemDisplay = RightMenuItemIconExtension ? (
+                    <StyledMenuItemWithIcon>
+                      {child.label}
+                      <RightMenuItemIconExtension menuChild={child} />
+                    </StyledMenuItemWithIcon>
+                  ) : (
+                    child.label
+                  );
+
+                  return (
+                    <Menu.Item key={child.label}>
+                      {isFrontendRoute(child.url) ? (
+                        <Link to={child.url || ''}>{menuItemDisplay}</Link>
+                      ) : (
+                        <Typography.Link href={child.url || ''}>
+                          {menuItemDisplay}
+                        </Typography.Link>
+                      )}
+                    </Menu.Item>
+                  );
+                }
+                return null;
+              })}
+            </Menu.ItemGroup>,
+            index < settings.length - 1 ? (
+              <Menu.Divider key={`divider_${index}`} />
+            ) : null,
+          ])}
+
+          {!navbarRight.user_is_anonymous && (
+            <>
+              <Menu.Divider key="user-divider" />
+              <Menu.ItemGroup key="user-section" title={t('User')}>
+                {navbarRight.user_info_url && (
+                  <Menu.Item key="info">
+                    <Typography.Link href={navbarRight.user_info_url}>
+                      {t('Info')}
+                    </Typography.Link>
+                  </Menu.Item>
+                )}
+                <Menu.Item key="logout" onClick={handleLogout}>
+                  <Typography.Link href={navbarRight.user_logout_url}>
+                    {t('Logout')}
+                  </Typography.Link>
+                </Menu.Item>
+              </Menu.ItemGroup>
+            </>
+          )}
+
+          {(navbarRight.version_string || navbarRight.version_sha) && (
+            <>
+              <Menu.Divider key="version-info-divider" />
+              <Menu.ItemGroup key="about-section" title={t('About')}>
+                <Menu.Item key="about-info">
+                  <div className="about-section">
+                    {navbarRight.show_watermark && (
+                      <div css={versionInfoStyles}>
+                        {t('Powered by Apache Superset')}
+                      </div>
+                    )}
+                    {navbarRight.version_string && (
+                      <div css={versionInfoStyles}>
+                        {t('Version')}: {navbarRight.version_string}
+                      </div>
+                    )}
+                    {navbarRight.version_sha && (
+                      <div css={versionInfoStyles}>
+                        {t('SHA')}: {navbarRight.version_sha}
+                      </div>
+                    )}
+                    {navbarRight.build_number && (
+                      <div css={versionInfoStyles}>
+                        {t('Build')}: {navbarRight.build_number}
+                      </div>
+                    )}
+                  </div>
+                </Menu.Item>
+              </Menu.ItemGroup>
+            </>
+          )}
+        </StyledDropdownSubMenu>
+      </Menu>
       {navbarRight.documentation_url && (
         <>
           <StyledAnchor
diff --git a/superset-frontend/src/features/home/StyledDropdownSubMenu.tsx b/superset-frontend/src/features/home/StyledDropdownSubMenu.tsx
new file mode 100644
index 0000000000..af1fb14abb
--- /dev/null
+++ b/superset-frontend/src/features/home/StyledDropdownSubMenu.tsx
@@ -0,0 +1,45 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { styled, css } from '@superset-ui/core';
+import { MainNav } from '@superset-ui/core/components/Menu';
+
+const { SubMenu } = MainNav;
+
+/**
+ * Shared styled submenu component with consistent caret positioning.
+ * Positions the caret icon on the right side of the label for dropdown menus.
+ */
+export const StyledDropdownSubMenu = styled(SubMenu)`
+  ${({ theme }) => css`
+    [data-icon="caret-down"] {
+      color: ${theme.colors.grayscale.base};
+      font-size: ${theme.fontSizeXS}px;
+      margin-left: ${theme.sizeUnit}px;
+    }
+    &.ant-menu-submenu {
+        padding: ${theme.sizeUnit * 2}px ${theme.sizeUnit * 4}px;
+        display: flex;
+        align-items: center;
+        height: 100%;  &.ant-menu-submenu-active {
+    .ant-menu-title-content {
+      color: ${theme.colorPrimary};
+    }
+  }
+  `}
+`;