korbit-ai[bot] commented on code in PR #32587:
URL: https://github.com/apache/superset/pull/32587#discussion_r2148818131
##########
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:
### Empty Promise Rejection <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
Empty reject call in Promise without providing error information.
###### Why this matters
When the Promise is rejected, no error information is passed, making it
impossible to understand why the operation failed during debugging.
###### Suggested change ∙ *Feature Preview*
```typescript
if (data) {
resolve(data);
} else {
reject(new Error('Failed to get CSV data'));
}
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/e01df21e-8495-41c4-a439-9eb07ad7bb29/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/e01df21e-8495-41c4-a439-9eb07ad7bb29?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/e01df21e-8495-41c4-a439-9eb07ad7bb29?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/e01df21e-8495-41c4-a439-9eb07ad7bb29?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/e01df21e-8495-41c4-a439-9eb07ad7bb29)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:29bde402-d275-4fdd-a94e-20c6e8fcf393 -->
[](29bde402-d275-4fdd-a94e-20c6e8fcf393)
##########
superset-frontend/src/SqlLab/components/SaveQuery/index.tsx:
##########
@@ -111,16 +111,18 @@ const SaveQuery = ({
database?.allows_virtual_table_explore !== undefined;
const overlayMenu = (
- <Menu>
- <Menu.Item
- onClick={() => {
- logAction(LOG_ACTIONS_SQLLAB_CREATE_CHART, {});
- setShowSaveDatasetModal(true);
- }}
- >
- {t('Save dataset')}
- </Menu.Item>
- </Menu>
+ <Menu
+ items={[
+ {
+ label: t('Save dataset'),
+ key: 'save-dataset',
+ onClick: () => {
+ logAction(LOG_ACTIONS_SQLLAB_CREATE_CHART, {});
+ setShowSaveDatasetModal(true);
+ },
+ },
+ ]}
+ />
Review Comment:
### Misleading Action Logging <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The logAction is logging 'CREATE_CHART' when saving a dataset, which is
misleading and doesn't match the actual action being performed.
###### Why this matters
Inconsistent documentation through logging makes it harder to track and
debug user actions accurately in the system.
###### Suggested change ∙ *Feature Preview*
Change the log action to be more descriptive of the actual operation:
```typescript
logAction(LOG_ACTIONS_SQLLAB_SAVE_DATASET, {});
```
And ensure LOG_ACTIONS_SQLLAB_SAVE_DATASET is added to the imports from
LogUtils.
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/bf180797-6c9b-47a9-8e12-0f1cfc91bccb/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/bf180797-6c9b-47a9-8e12-0f1cfc91bccb?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/bf180797-6c9b-47a9-8e12-0f1cfc91bccb?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/bf180797-6c9b-47a9-8e12-0f1cfc91bccb?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/bf180797-6c9b-47a9-8e12-0f1cfc91bccb)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:be421efe-2f21-4f12-83e5-e97553f99927 -->
[](be421efe-2f21-4f12-83e5-e97553f99927)
--
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
