michael-s-molina commented on code in PR #36644:
URL: https://github.com/apache/superset/pull/36644#discussion_r2675989191
##########
superset-frontend/src/SqlLab/components/SaveQuery/index.tsx:
##########
@@ -81,6 +80,7 @@ const SaveQuery = ({
saveQueryWarning,
database,
columns,
+ compactMode,
Review Comment:
```suggestion
```
##########
superset-frontend/src/SqlLab/components/AceEditorWrapper/index.tsx:
##########
@@ -147,10 +149,13 @@ const AceEditorWrapper = ({
onCursorPositionChange(cursor);
});
+ // setTimeout(() => {
const { row, column } = cursorPosition;
+ console.log('moving cursor to', { row, column });
editor.moveCursorToPosition({ row, column });
editor.focus();
editor.scrollToLine(row, true, true);
+ // }, 100);
Review Comment:
```suggestion
const { row, column } = cursorPosition;
editor.moveCursorToPosition({ row, column });
editor.focus();
editor.scrollToLine(row, true, true);
```
##########
superset-frontend/src/SqlLab/components/SqlEditor/index.tsx:
##########
@@ -778,93 +752,63 @@ const SqlEditor: FC<Props> = ({
const runMenuBtn = <Menu items={menuItems} />;
return (
- <StyledToolbar className="sql-toolbar" id="js-sql-toolbar">
- {hideActions ? (
- <Alert
- type="warning"
- message={t(
- 'The database that was used to generate this query could not be
found',
- )}
- description={t(
- 'Choose one of the available databases on the left panel.',
- )}
- closable={false}
+ <>
+ <RunQueryActionButton
+ queryEditorId={queryEditor.id}
+ queryState={latestQuery?.state}
+ runQuery={runQuery}
+ stopQuery={stopQuery}
+ overlayCreateAsMenu={showMenu ? runMenuBtn : null}
+ />
+ <span>
+ <QueryLimitSelect
+ queryEditorId={queryEditor.id}
+ maxRow={maxRow}
+ defaultQueryLimit={defaultQueryLimit}
/>
- ) : (
- <>
- <div className="leftItems">
- <span>
- <RunQueryActionButton
- allowAsync={database?.allow_run_async === true}
- queryEditorId={queryEditor.id}
- queryState={latestQuery?.state}
- runQuery={runQuery}
- stopQuery={stopQuery}
- overlayCreateAsMenu={showMenu ? runMenuBtn : null}
- />
- </span>
- {isFeatureEnabled(FeatureFlag.EstimateQueryCost) &&
- database?.allows_cost_estimate && (
- <span>
- <EstimateQueryCostButton
- getEstimate={getQueryCostEstimate}
- queryEditorId={queryEditor.id}
- tooltip={t('Estimate the cost before running a query')}
- />
- </span>
- )}
- <span>
- <QueryLimitSelect
- queryEditorId={queryEditor.id}
- maxRow={maxRow}
- defaultQueryLimit={defaultQueryLimit}
- />
- </span>
- {latestQuery && (
- <Timer
- startTime={latestQuery.startDttm}
- endTime={latestQuery.endDttm}
- status={STATE_TYPE_MAP[latestQuery.state]}
- isRunning={latestQuery.state === 'running'}
- />
- )}
- </div>
- <div className="rightItems">
- <span>
- <SaveQuery
- queryEditorId={queryEditor.id}
- columns={latestQuery?.results?.columns || []}
- onSave={onSaveQuery}
- onUpdate={(query, remoteId) =>
- dispatch(updateSavedQuery(query, remoteId))
- }
- saveQueryWarning={saveQueryWarning}
- database={database}
- />
- </span>
- <span>
- <ShareSqlLabQuery queryEditorId={queryEditor.id} />
- </span>
- <div>{primaryContributions}</div>
- <Dropdown
- popupRender={() => renderDropdown()}
- trigger={['click']}
- >
- <Button
- buttonSize="xsmall"
- showMarginRight={false}
- buttonStyle="link"
- >
- <Icons.EllipsisOutlined />
- </Button>
- </Dropdown>
- </div>
- </>
- )}
- </StyledToolbar>
+ </span>
+ <Divider type="vertical" />
+ {isFeatureEnabled(FeatureFlag.EstimateQueryCost) &&
+ database?.allows_cost_estimate && (
+ <span>
+ <EstimateQueryCostButton
+ getEstimate={getQueryCostEstimate}
+ queryEditorId={queryEditor.id}
+ tooltip={t('Estimate the cost before running a query')}
+ />
+ </span>
+ )}
+ <SaveQuery
+ queryEditorId={queryEditor.id}
+ columns={latestQuery?.results?.columns || []}
+ onSave={onSaveQuery}
+ onUpdate={(query, remoteId) =>
+ dispatch(updateSavedQuery(query, remoteId))
+ }
+ saveQueryWarning={saveQueryWarning}
+ database={database}
+ compactMode
Review Comment:
```suggestion
```
##########
superset-frontend/src/SqlLab/components/SaveQuery/index.tsx:
##########
@@ -209,7 +199,8 @@ const SaveQuery = ({
{shouldShowSaveButton && (
<SaveDatasetActionButton
setShowSave={setShowSave}
- overlayMenu={canExploreDatabase ? overlayMenu : null}
+ onSaveAsExplore={canExploreDatabase ? onSaveAsExplore : undefined}
+ compactMode={compactMode}
Review Comment:
```suggestion
```
##########
superset-frontend/src/components/ViewListExtension/index.tsx:
##########
@@ -16,18 +16,19 @@
* specific language governing permissions and limitations
* under the License.
*/
+import ExtensionsManager from 'src/extensions/ExtensionsManager';
+import { useExtensionsContext } from 'src/extensions/ExtensionsContext';
-/**
- * Jest configuration for @storybook/test-runner
- *
- * This extends the default test-runner config with custom timeouts
- * to handle slow story rendering in CI environments.
- */
-const { getJestConfig } = require('@storybook/test-runner');
-const testRunnerConfig = getJestConfig();
+export interface ViewExtensionProps {
+ viewId: string;
+}
-module.exports = {
- ...testRunnerConfig,
- // Increase timeout from default 15s to 60s for CI environments
- testTimeout: 60000,
+const ViewExtension = ({ viewId }: ViewExtensionProps) => {
+ const contributions =
+ ExtensionsManager.getInstance().getViewContributions(viewId) || [];
+ const { getView } = useExtensionsContext();
+
+ return <>{contributions.map(contribution => getView(contribution.id))}</>;
Review Comment:
Great suggestion. @justinpark
##########
superset-frontend/src/components/MenuListExtension/index.tsx:
##########
@@ -0,0 +1,147 @@
+/**
+ * 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 { useMemo } from 'react';
+import { css, useTheme } from '@apache-superset/core/ui';
+import { Button, Dropdown } from '@superset-ui/core/components';
+import { Menu, MenuItemType } from '@superset-ui/core/components/Menu';
+import { Icons } from '@superset-ui/core/components/Icons';
+import { commands } from 'src/core';
+import ExtensionsManager from 'src/extensions/ExtensionsManager';
+
+export type MenuExtensionProps = {
+ viewId: string;
+} & (
+ | {
+ primary: boolean;
+ secondary?: never;
+ children?: React.ReactNode;
+ defaultItems?: never;
+ compactMode?: boolean;
+ }
+ | {
+ primary?: never;
+ secondary: boolean;
+ children?: never;
+ defaultItems?: MenuItemType[];
+ compactMode?: never;
+ }
+);
+
+const MenuExtension = ({
+ viewId,
+ primary,
+ secondary,
+ defaultItems,
+ children,
+ compactMode,
+}: MenuExtensionProps) => {
+ const theme = useTheme();
+ const iconColor = theme.colorPrimary;
+ const contributions =
+ ExtensionsManager.getInstance().getMenuContributions(viewId);
+
+ const actions = primary ? contributions?.primary : contributions?.secondary;
+ const primaryActions = useMemo(
+ () =>
+ primary
+ ? (actions || []).map(contribution => {
+ const command =
+ ExtensionsManager.getInstance().getCommandContribution(
+ contribution.command,
+ )!;
+ // @ts-ignore
+ const Icon = Icons[command?.icon as IconNameType];
+
+ return (
+ <Button
+ key={contribution.view}
+ onClick={() => commands.executeCommand(command.command)}
+ tooltip={command?.description}
+ icon={<Icon iconSize="m" iconColor={iconColor} />}
Review Comment:
@justinpark This safeguard is important but not using `any`.
##########
superset-frontend/src/components/MenuListExtension/index.tsx:
##########
@@ -0,0 +1,147 @@
+/**
+ * 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 { useMemo } from 'react';
+import { css, useTheme } from '@apache-superset/core/ui';
+import { Button, Dropdown } from '@superset-ui/core/components';
+import { Menu, MenuItemType } from '@superset-ui/core/components/Menu';
+import { Icons } from '@superset-ui/core/components/Icons';
+import { commands } from 'src/core';
+import ExtensionsManager from 'src/extensions/ExtensionsManager';
+
+export type MenuExtensionProps = {
+ viewId: string;
+} & (
+ | {
+ primary: boolean;
+ secondary?: never;
+ children?: React.ReactNode;
+ defaultItems?: never;
+ compactMode?: boolean;
+ }
+ | {
+ primary?: never;
+ secondary: boolean;
+ children?: never;
+ defaultItems?: MenuItemType[];
+ compactMode?: never;
+ }
+);
+
+const MenuExtension = ({
+ viewId,
+ primary,
+ secondary,
+ defaultItems,
+ children,
+ compactMode,
+}: MenuExtensionProps) => {
+ const theme = useTheme();
+ const iconColor = theme.colorPrimary;
+ const contributions =
+ ExtensionsManager.getInstance().getMenuContributions(viewId);
+
+ const actions = primary ? contributions?.primary : contributions?.secondary;
+ const primaryActions = useMemo(
+ () =>
+ primary
+ ? (actions || []).map(contribution => {
+ const command =
+ ExtensionsManager.getInstance().getCommandContribution(
+ contribution.command,
+ )!;
Review Comment:
@justinpark These types of safeguards are important as we might have errors
in metadata definition and we don't have full validation yet.
##########
superset-frontend/src/SqlLab/components/SaveQuery/index.tsx:
##########
@@ -53,6 +52,7 @@ interface SaveQueryProps {
onUpdate: (arg0: QueryPayload, id: string) => void;
saveQueryWarning: string | null;
database: Partial<DatabaseObject> | undefined;
+ compactMode?: boolean;
Review Comment:
```suggestion
```
##########
superset-frontend/src/SqlLab/components/RunQueryActionButton/index.tsx:
##########
@@ -62,32 +63,27 @@ const buildTextAndIcon = (
};
const onClick = (
- shouldShowStopButton: boolean,
- allowAsync: boolean,
- runQuery: (c?: boolean) => void = () => undefined,
+ isStopAction: boolean,
+ runQuery: () => void = () => undefined,
stopQuery = () => {},
logAction: (name: string, payload: Record<string, any>) => void,
): void => {
- const eventName = shouldShowStopButton
+ const eventName = isStopAction
? LOG_ACTIONS_SQLLAB_STOP_QUERY
: LOG_ACTIONS_SQLLAB_RUN_QUERY;
logAction(eventName, { shortcut: false });
- if (shouldShowStopButton) return stopQuery();
- if (allowAsync) {
- return runQuery(true);
- }
- return runQuery(false);
+ if (isStopAction) return stopQuery();
+ runQuery();
};
-const StyledButton = styled.span`
+const StyledButton = styled.span<{ compact?: boolean }>`
Review Comment:
```suggestion
const StyledButton = styled.span`
```
##########
superset-frontend/src/components/MenuListExtension/index.tsx:
##########
@@ -0,0 +1,147 @@
+/**
+ * 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 { useMemo } from 'react';
+import { css, useTheme } from '@apache-superset/core/ui';
+import { Button, Dropdown } from '@superset-ui/core/components';
+import { Menu, MenuItemType } from '@superset-ui/core/components/Menu';
+import { Icons } from '@superset-ui/core/components/Icons';
+import { commands } from 'src/core';
+import ExtensionsManager from 'src/extensions/ExtensionsManager';
+
+export type MenuExtensionProps = {
+ viewId: string;
+} & (
+ | {
+ primary: boolean;
+ secondary?: never;
+ children?: React.ReactNode;
+ defaultItems?: never;
+ compactMode?: boolean;
+ }
+ | {
+ primary?: never;
+ secondary: boolean;
+ children?: never;
+ defaultItems?: MenuItemType[];
+ compactMode?: never;
+ }
+);
+
+const MenuExtension = ({
+ viewId,
+ primary,
+ secondary,
+ defaultItems,
+ children,
+ compactMode,
+}: MenuExtensionProps) => {
+ const theme = useTheme();
+ const iconColor = theme.colorPrimary;
+ const contributions =
+ ExtensionsManager.getInstance().getMenuContributions(viewId);
+
+ const actions = primary ? contributions?.primary : contributions?.secondary;
+ const primaryActions = useMemo(
+ () =>
+ primary
+ ? (actions || []).map(contribution => {
+ const command =
+ ExtensionsManager.getInstance().getCommandContribution(
+ contribution.command,
+ )!;
+ // @ts-ignore
+ const Icon = Icons[command?.icon as IconNameType];
+
+ return (
+ <Button
+ key={contribution.view}
+ onClick={() => commands.executeCommand(command.command)}
+ tooltip={command?.description}
+ icon={<Icon iconSize="m" iconColor={iconColor} />}
+ buttonSize="small"
+ >
+ {!compactMode ? command?.title : undefined}
+ </Button>
+ );
+ })
+ : [],
+ [actions, primary, iconColor, compactMode],
+ );
+ const secondaryActions = useMemo(
+ () =>
+ secondary
+ ? (actions || [])
+ .map(contribution => {
+ const command =
+ ExtensionsManager.getInstance().getCommandContribution(
+ contribution.command,
+ )!;
+ return {
+ key: command.command,
+ label: command.title,
+ title: command.description,
+ onClick: () => commands.executeCommand(command.command),
+ } as MenuItemType;
+ })
+ .concat(...(defaultItems || []))
Review Comment:
Good suggestion.
##########
superset-frontend/src/components/MenuListExtension/index.tsx:
##########
@@ -0,0 +1,147 @@
+/**
+ * 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 { useMemo } from 'react';
+import { css, useTheme } from '@apache-superset/core/ui';
+import { Button, Dropdown } from '@superset-ui/core/components';
+import { Menu, MenuItemType } from '@superset-ui/core/components/Menu';
+import { Icons } from '@superset-ui/core/components/Icons';
+import { commands } from 'src/core';
+import ExtensionsManager from 'src/extensions/ExtensionsManager';
+
+export type MenuExtensionProps = {
+ viewId: string;
+} & (
+ | {
+ primary: boolean;
+ secondary?: never;
+ children?: React.ReactNode;
+ defaultItems?: never;
+ compactMode?: boolean;
+ }
+ | {
+ primary?: never;
+ secondary: boolean;
+ children?: never;
+ defaultItems?: MenuItemType[];
+ compactMode?: never;
+ }
+);
+
+const MenuExtension = ({
+ viewId,
+ primary,
+ secondary,
+ defaultItems,
+ children,
+ compactMode,
+}: MenuExtensionProps) => {
+ const theme = useTheme();
+ const iconColor = theme.colorPrimary;
+ const contributions =
+ ExtensionsManager.getInstance().getMenuContributions(viewId);
+
+ const actions = primary ? contributions?.primary : contributions?.secondary;
+ const primaryActions = useMemo(
+ () =>
+ primary
+ ? (actions || []).map(contribution => {
+ const command =
+ ExtensionsManager.getInstance().getCommandContribution(
+ contribution.command,
+ )!;
+ // @ts-ignore
+ const Icon = Icons[command?.icon as IconNameType];
+
+ return (
+ <Button
+ key={contribution.view}
+ onClick={() => commands.executeCommand(command.command)}
+ tooltip={command?.description}
+ icon={<Icon iconSize="m" iconColor={iconColor} />}
+ buttonSize="small"
+ >
+ {!compactMode ? command?.title : undefined}
+ </Button>
+ );
+ })
+ : [],
+ [actions, primary, iconColor, compactMode],
+ );
+ const secondaryActions = useMemo(
+ () =>
+ secondary
+ ? (actions || [])
+ .map(contribution => {
+ const command =
+ ExtensionsManager.getInstance().getCommandContribution(
+ contribution.command,
+ )!;
Review Comment:
Same here
##########
superset-frontend/src/SqlLab/components/SqlEditorTopBar/index.tsx:
##########
@@ -0,0 +1,62 @@
+/**
+ * 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 { Divider, Flex } from '@superset-ui/core/components';
+import { styled } from '@apache-superset/core/ui';
+import { ViewContribution } from 'src/SqlLab/contributions';
+import MenuListExtension, {
+ type MenuListExtensionProps,
+} from 'src/components/MenuListExtension';
+
+const StyledFlex = styled(Flex)`
+ margin-bottom: ${({ theme }) => theme.sizeUnit * 2}px;
+
+ & .ant-divider {
+ margin: ${({ theme }) => theme.sizeUnit * 2}px 0;
+ height: ${({ theme }) => theme.sizeUnit * 6}px;
+ }
+`;
+export interface SqlEditorTopBarProps {
+ queryEditorId: string;
+ defaultPrimaryActions: React.ReactNode;
+ defaultSecondaryActions: MenuListExtensionProps['defaultItems'];
+}
+
+const SqlEditorTopBar = ({
+ defaultPrimaryActions,
+ defaultSecondaryActions,
+}: SqlEditorTopBarProps) => (
+ <StyledFlex justify="space-between" gap="small" id="js-sql-toolbar">
+ <Flex flex={1} gap="small" align="center">
+ <Flex gap="small" align="center">
+ <MenuListExtension viewId={ViewContribution.Editor} primary
compactMode>
Review Comment:
```suggestion
<MenuListExtension viewId={ViewContribution.Editor} primary>
```
--
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]
