EnxDev commented on code in PR #32587:
URL: https://github.com/apache/superset/pull/32587#discussion_r2149789765


##########
superset-frontend/src/components/Dropdown/index.tsx:
##########
@@ -82,7 +82,7 @@ export enum IconOrientation {
 }
 
 export interface MenuDotsDropdownProps extends AntdDropdownProps {
-  overlay: ReactElement;
+  overlay?: ReactElement;

Review Comment:
   Just wondering, is this prop necessary here?



##########
superset-frontend/src/components/GridTable/HeaderMenu.tsx:
##########
@@ -41,129 +41,199 @@ type Params = {
   onVisibleChange: DropdownProps['onOpenChange'];
 };
 
-const HeaderMenu: React.FC<Params> = ({
+const HeaderMenu: React.FC<HeaderMenuProps> = ({
   colId,
   api,
   pinnedLeft,
   pinnedRight,
   invisibleColumns,
   isMain,
   onVisibleChange,
-}: Params) => {
+}: HeaderMenuProps) => {
   const pinColumn = useCallback(
     (pinLoc: ColumnPinnedType) => {
       api.setColumnsPinned([colId], pinLoc);
     },
     [api, colId],
   );
 
-  const unHideAction = invisibleColumns.length > 0 && (
-    <Menu.SubMenu title={t('Unhide')} icon={<Icons.EyeOutlined iconSize="m" 
/>}>
-      {invisibleColumns.length > 1 && (
-        <Menu.Item
-          onClick={() => {
-            api.setColumnsVisible(invisibleColumns, true);
-          }}
-        >
-          <b>{t('All %s hidden columns', invisibleColumns.length)}</b>
-        </Menu.Item>
-      )}
-      {invisibleColumns.map(c => (
-        <Menu.Item
-          key={c.getColId()}
-          onClick={() => {
-            api.setColumnsVisible([c.getColId()], true);
-          }}
-        >
-          {c.getColDef().headerName}
-        </Menu.Item>
-      ))}
-    </Menu.SubMenu>
-  );
+  const unHideAction: MenuItem = {
+    label: t('Unhide'),
+    key: 'unHideSubMenu',
+    icon: <Icons.EyeInvisibleOutlined iconSize="m" />,
+    children: [
+      invisibleColumns.length > 1 && {
+        key: 'allHidden',
+        label: <b>{t('All %s hidden columns', invisibleColumns.length)}</b>,
+        onClick: () => {
+          api.setColumnsVisible(invisibleColumns, true);
+        },
+      },
+      ...invisibleColumns.map(c => ({
+        key: c.getColId(),
+        label: c.getColDef().headerName,
+        onClick: () => {
+          api.setColumnsVisible([c.getColId()], true);
+        },
+      })),
+    ].filter(Boolean) as MenuItem[],
+  };
+
+  const mainMenuItems: MenuItem[] = [
+    {
+      key: 'copyData',
+      label: t('Copy the current data'),
+      icon: <Icons.CopyOutlined iconSize="m" />,
+      onClick: () => {
+        copyTextToClipboard(
+          () =>
+            new Promise((resolve, reject) => {
+              const data = api.getDataAsCsv({
+                columnKeys: api
+                  .getAllDisplayedColumns()
+                  .map(c => c.getColId())
+                  .filter(id => id !== colId),
+                suppressQuotes: true,
+                columnSeparator: '\t',
+              });
+              if (data) {
+                resolve(data);
+              } else {
+                reject();

Review Comment:
   Should we add an error message?



##########
superset-frontend/src/components/GridTable/HeaderMenu.tsx:
##########
@@ -41,129 +41,199 @@ type Params = {
   onVisibleChange: DropdownProps['onOpenChange'];
 };
 
-const HeaderMenu: React.FC<Params> = ({
+const HeaderMenu: React.FC<HeaderMenuProps> = ({
   colId,
   api,
   pinnedLeft,
   pinnedRight,
   invisibleColumns,
   isMain,
   onVisibleChange,
-}: Params) => {
+}: HeaderMenuProps) => {
   const pinColumn = useCallback(
     (pinLoc: ColumnPinnedType) => {
       api.setColumnsPinned([colId], pinLoc);
     },
     [api, colId],
   );
 
-  const unHideAction = invisibleColumns.length > 0 && (
-    <Menu.SubMenu title={t('Unhide')} icon={<Icons.EyeOutlined iconSize="m" 
/>}>
-      {invisibleColumns.length > 1 && (
-        <Menu.Item
-          onClick={() => {
-            api.setColumnsVisible(invisibleColumns, true);
-          }}
-        >
-          <b>{t('All %s hidden columns', invisibleColumns.length)}</b>
-        </Menu.Item>
-      )}
-      {invisibleColumns.map(c => (
-        <Menu.Item
-          key={c.getColId()}
-          onClick={() => {
-            api.setColumnsVisible([c.getColId()], true);
-          }}
-        >
-          {c.getColDef().headerName}
-        </Menu.Item>
-      ))}
-    </Menu.SubMenu>
-  );
+  const unHideAction: MenuItem = {
+    label: t('Unhide'),
+    key: 'unHideSubMenu',
+    icon: <Icons.EyeInvisibleOutlined iconSize="m" />,
+    children: [
+      invisibleColumns.length > 1 && {
+        key: 'allHidden',
+        label: <b>{t('All %s hidden columns', invisibleColumns.length)}</b>,
+        onClick: () => {
+          api.setColumnsVisible(invisibleColumns, true);
+        },
+      },
+      ...invisibleColumns.map(c => ({
+        key: c.getColId(),
+        label: c.getColDef().headerName,
+        onClick: () => {
+          api.setColumnsVisible([c.getColId()], true);
+        },
+      })),
+    ].filter(Boolean) as MenuItem[],
+  };
+
+  const mainMenuItems: MenuItem[] = [
+    {
+      key: 'copyData',
+      label: t('Copy the current data'),
+      icon: <Icons.CopyOutlined iconSize="m" />,
+      onClick: () => {
+        copyTextToClipboard(
+          () =>
+            new Promise((resolve, reject) => {
+              const data = api.getDataAsCsv({
+                columnKeys: api
+                  .getAllDisplayedColumns()
+                  .map(c => c.getColId())
+                  .filter(id => id !== colId),
+                suppressQuotes: true,
+                columnSeparator: '\t',
+              });
+              if (data) {
+                resolve(data);
+              } else {
+                reject();
+              }
+            }),
+        );
+      },
+    },
+    {
+      key: 'downloadCsv',
+      label: t('Download to CSV'),
+      icon: <Icons.DownloadOutlined iconSize="m" />,
+      onClick: () => {
+        api.exportDataAsCsv({
+          columnKeys: api
+            .getAllDisplayedColumns()
+            .map(c => c.getColId())
+            .filter(id => id !== colId),
+        });
+      },
+    },
+    {
+      type: 'divider',
+    },
+    {
+      key: 'autoSizeAllColumns',
+      label: t('Autosize all columns'),
+      icon: <Icons.ColumnWidthOutlined iconSize="m" />,
+      onClick: () => {
+        api.autoSizeAllColumns();
+      },
+    },
+  ];
 
-  if (isMain) {
-    return (
-      <MenuDotsDropdown
-        placement="bottomLeft"
-        trigger={['click']}
-        onOpenChange={onVisibleChange}
-        overlay={
-          <Menu style={{ width: 250 }} mode="vertical">
-            <Menu.Item
-              onClick={() => {
-                copyTextToClipboard(
-                  () =>
-                    new Promise((resolve, reject) => {
-                      const data = api.getDataAsCsv({
-                        columnKeys: api
-                          .getAllDisplayedColumns()
-                          .map(c => c.getColId())
-                          .filter(id => id !== colId),
-                        suppressQuotes: true,
-                        columnSeparator: '\t',
-                      });
-                      if (data) {
-                        resolve(data);
-                      } else {
-                        reject();
-                      }
-                    }),
-                );
-              }}
-              icon={<Icons.CopyOutlined iconSize="m" />}
-            >
-              {t('Copy the current data')}
-            </Menu.Item>
-            <Menu.Item
-              onClick={() => {
-                api.exportDataAsCsv({
-                  columnKeys: api
-                    .getAllDisplayedColumns()
-                    .map(c => c.getColId())
-                    .filter(id => id !== colId),
-                });
-              }}
-              icon={<Icons.DownloadOutlined iconSize="m" />}
-            >
-              {t('Download to CSV')}
-            </Menu.Item>
-            <Menu.Divider />
-            <Menu.Item
-              onClick={() => {
-                api.autoSizeAllColumns();
-              }}
-              icon={<Icons.ColumnWidthOutlined iconSize="m" />}
-            >
-              {t('Autosize all columns')}
-            </Menu.Item>
-            {unHideAction}
-            <Menu.Divider />
-            <Menu.Item
-              onClick={() => {
-                api.setColumnsVisible(invisibleColumns, true);
-                const columns = api.getColumns();
-                if (columns) {
-                  const pinnedColumns = columns.filter(
-                    c => c.getColId() !== PIVOT_COL_ID && c.isPinned(),
-                  );
-                  api.setColumnsPinned(pinnedColumns, null);
-                  api.moveColumns(columns, 0);
-                  const firstColumn = columns.find(
-                    c => c.getColId() !== PIVOT_COL_ID,
-                  );
-                  if (firstColumn) {
-                    api.ensureColumnVisible(firstColumn, 'start');
-                  }
-                }
-              }}
-              icon={<IconEmpty className="anticon" />}
-            >
-              {t('Reset columns')}
-            </Menu.Item>
-          </Menu>
+  mainMenuItems.push(unHideAction);
+
+  mainMenuItems.push(
+    {
+      type: 'divider',
+    },
+    {
+      key: 'resetColumns',
+      label: t('Reset columns'),
+      icon: <IconEmpty className="anticon" />,
+      onClick: () => {
+        api.setColumnsVisible(invisibleColumns, true);
+        const columns = api.getColumns();
+        if (columns) {
+          const pinnedColumns = columns.filter(
+            c => c.getColId() !== PIVOT_COL_ID && c.isPinned(),
+          );
+          api.setColumnsPinned(pinnedColumns, null);
+          api.moveColumns(columns, 0);
+          const firstColumn = columns.find(c => c.getColId() !== PIVOT_COL_ID);
+          if (firstColumn) {
+            api.ensureColumnVisible(firstColumn, 'start');
+          }
         }
-      />
-    );
+      },
+    },
+  );
+
+  const menuItems: MenuItem[] = [
+    {
+      key: 'copy',
+      label: t('Copy'),
+      icon: <Icons.CopyOutlined iconSize="m" />,
+      onClick: () => {
+        copyTextToClipboard(
+          () =>
+            new Promise((resolve, reject) => {
+              const data = api.getDataAsCsv({
+                columnKeys: [colId],
+                suppressQuotes: true,
+              });
+              if (data) {
+                resolve(data);
+              } else {
+                reject();

Review Comment:
   here as well



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

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


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

Reply via email to