From 60dcd651f44b7e1aa1b030e0cd5c64334a346e60 Mon Sep 17 00:00:00 2001 From: Kamil Gabryjelski Date: Sun, 3 Apr 2022 13:26:56 +0200 Subject: [PATCH] feat(explore): SQL popover in datasource panel (#19308) * feat(explore): SQL popover in datasource panel * Fix acequire not defined * Rebase and fix tests * Disable highlighting gutter * Use ace-build acequire instead of brace --- superset-frontend/package-lock.json | 23 +++--- superset-frontend/package.json | 1 + .../superset-ui-chart-controls/package.json | 7 +- .../src/components/ColumnOption.tsx | 14 +--- .../src/components/MetricOption.tsx | 10 +-- .../src/components/SQLPopover.tsx | 70 +++++++++++++++++++ .../test/components/ColumnOption.test.tsx | 12 ++-- .../test/components/MetricOption.test.tsx | 11 +-- .../TemplateParamsEditor.test.tsx | 3 - .../components/TemplateParamsEditor/index.tsx | 1 - .../src/components/AsyncAceEditor/index.tsx | 6 +- 11 files changed, 110 insertions(+), 48 deletions(-) create mode 100644 superset-frontend/packages/superset-ui-chart-controls/src/components/SQLPopover.tsx diff --git a/superset-frontend/package-lock.json b/superset-frontend/package-lock.json index de2d1a60f81f4..2e845ffab4f53 100644 --- a/superset-frontend/package-lock.json +++ b/superset-frontend/package-lock.json @@ -51,6 +51,7 @@ "@superset-ui/switchboard": "file:./packages/superset-ui-switchboard", "@vx/responsive": "^0.0.195", "abortcontroller-polyfill": "^1.1.9", + "ace-builds": "^1.4.14", "antd": "^4.9.4", "array-move": "^2.2.1", "babel-plugin-typescript-to-proptypes": "^2.0.0", @@ -24403,9 +24404,9 @@ } }, "node_modules/ace-builds": { - "version": "1.4.13", - "resolved": "https://registry.npmjs.org/ace-builds/-/ace-builds-1.4.13.tgz", - "integrity": "sha512-SOLzdaQkY6ecPKYRDDg+MY1WoGgXA34cIvYJNNoBMGGUswHmlauU2Hy0UL96vW0Fs/LgFbMUjD+6vqzWTldIYQ==" + "version": "1.4.14", + "resolved": "https://registry.npmjs.org/ace-builds/-/ace-builds-1.4.14.tgz", + "integrity": "sha512-NBOQlm9+7RBqRqZwimpgquaLeTJFayqb9UEPtTkpC3TkkwDnlsT/TwsCC0svjt9kEZ6G9mH5AEOHSz6Q/HrzQQ==" }, "node_modules/acorn": { "version": "7.1.1", @@ -58467,6 +58468,8 @@ "license": "Apache-2.0", "dependencies": { "@react-icons/all-files": "^4.1.0", + "@types/enzyme": "^3.10.5", + "@types/react": "*", "lodash": "^4.17.15", "prop-types": "^15.7.2" }, @@ -58479,10 +58482,11 @@ "@testing-library/react": "^11.2.0", "@testing-library/react-hooks": "^5.0.3", "@testing-library/user-event": "^12.7.0", - "@types/enzyme": "^3.10.5", - "@types/react": "*", + "ace-builds": "^1.4.14", "antd": "^4.9.4", + "brace": "^0.11.1", "react": "^16.13.1", + "react-ace": "^9.4.4", "react-dom": "^16.13.1" } }, @@ -59324,6 +59328,7 @@ "prop-types": "^15.6.2" }, "peerDependencies": { + "@emotion/react": "^11.4.1", "@superset-ui/chart-controls": "*", "@superset-ui/core": "*", "react": "^16.13.1" @@ -76177,6 +76182,8 @@ "version": "file:packages/superset-ui-chart-controls", "requires": { "@react-icons/all-files": "^4.1.0", + "@types/enzyme": "^3.10.5", + "@types/react": "*", "lodash": "^4.17.15", "prop-types": "^15.7.2" } @@ -79091,9 +79098,9 @@ } }, "ace-builds": { - "version": "1.4.13", - "resolved": "https://registry.npmjs.org/ace-builds/-/ace-builds-1.4.13.tgz", - "integrity": "sha512-SOLzdaQkY6ecPKYRDDg+MY1WoGgXA34cIvYJNNoBMGGUswHmlauU2Hy0UL96vW0Fs/LgFbMUjD+6vqzWTldIYQ==" + "version": "1.4.14", + "resolved": "https://registry.npmjs.org/ace-builds/-/ace-builds-1.4.14.tgz", + "integrity": "sha512-NBOQlm9+7RBqRqZwimpgquaLeTJFayqb9UEPtTkpC3TkkwDnlsT/TwsCC0svjt9kEZ6G9mH5AEOHSz6Q/HrzQQ==" }, "acorn": { "version": "7.1.1", diff --git a/superset-frontend/package.json b/superset-frontend/package.json index f122d09464390..edf122a24992c 100644 --- a/superset-frontend/package.json +++ b/superset-frontend/package.json @@ -111,6 +111,7 @@ "@superset-ui/switchboard": "file:./packages/superset-ui-switchboard", "@vx/responsive": "^0.0.195", "abortcontroller-polyfill": "^1.1.9", + "ace-builds": "^1.4.14", "antd": "^4.9.4", "array-move": "^2.2.1", "babel-plugin-typescript-to-proptypes": "^2.0.0", diff --git a/superset-frontend/packages/superset-ui-chart-controls/package.json b/superset-frontend/packages/superset-ui-chart-controls/package.json index bdb6be4daf846..1890a5e38a08b 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/package.json +++ b/superset-frontend/packages/superset-ui-chart-controls/package.json @@ -24,6 +24,8 @@ ], "dependencies": { "@react-icons/all-files": "^4.1.0", + "@types/enzyme": "^3.10.5", + "@types/react": "*", "lodash": "^4.17.15", "prop-types": "^15.7.2" }, @@ -36,10 +38,11 @@ "@testing-library/react": "^11.2.0", "@testing-library/react-hooks": "^5.0.3", "@testing-library/user-event": "^12.7.0", - "@types/enzyme": "^3.10.5", - "@types/react": "*", + "ace-builds": "^1.4.14", "antd": "^4.9.4", + "brace": "^0.11.1", "react": "^16.13.1", + "react-ace": "^9.4.4", "react-dom": "^16.13.1" }, "publishConfig": { diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/components/ColumnOption.tsx b/superset-frontend/packages/superset-ui-chart-controls/src/components/ColumnOption.tsx index dd7775ec4dd06..fce2e8ff2ad07 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/src/components/ColumnOption.tsx +++ b/superset-frontend/packages/superset-ui-chart-controls/src/components/ColumnOption.tsx @@ -20,10 +20,10 @@ import React, { useState, ReactNode, useLayoutEffect } from 'react'; import { css, styled, SupersetTheme } from '@superset-ui/core'; import { Tooltip } from './Tooltip'; import { ColumnTypeLabel } from './ColumnTypeLabel/ColumnTypeLabel'; -import InfoTooltipWithTrigger from './InfoTooltipWithTrigger'; import CertifiedIconWithTooltip from './CertifiedIconWithTooltip'; import { ColumnMeta } from '../types'; import { getColumnLabelText, getColumnTooltipNode } from './labelUtils'; +import { SQLPopover } from './SQLPopover'; export type ColumnOptionProps = { column: ColumnMeta; @@ -69,17 +69,7 @@ export function ColumnOption({ {getColumnLabelText(column)} - - {hasExpression && ( - - )} - + {hasExpression && } {column.is_certified && ( - {showFormula && ( - + {showFormula && metric.expression && ( + )} {metric.is_certified && ( css` + color: ${theme.colors.grayscale.base}; + font-size: ${theme.typography.sizes.s}px; + & svg { + margin-left: ${theme.gridUnit}px; + margin-right: ${theme.gridUnit}px; + } + `} +`; + +export const SQLPopover = (props: PopoverProps & { sqlExpression: string }) => { + const theme = useTheme(); + return ( + + } + placement="bottomLeft" + arrowPointAtCenter + title={t('SQL expression')} + {...props} + > + + + ); +}; diff --git a/superset-frontend/packages/superset-ui-chart-controls/test/components/ColumnOption.test.tsx b/superset-frontend/packages/superset-ui-chart-controls/test/components/ColumnOption.test.tsx index cc0106e9d650c..b1fb4b26535bf 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/test/components/ColumnOption.test.tsx +++ b/superset-frontend/packages/superset-ui-chart-controls/test/components/ColumnOption.test.tsx @@ -20,12 +20,8 @@ import React from 'react'; import { shallow, ShallowWrapper } from 'enzyme'; import { GenericDataType } from '@superset-ui/core'; -import { - ColumnOption, - ColumnOptionProps, - ColumnTypeLabel, - InfoTooltipWithTrigger, -} from '../../src'; +import { ColumnOption, ColumnOptionProps, ColumnTypeLabel } from '../../src'; +import { SQLPopover } from '../../src/components/SQLPopover'; describe('ColumnOption', () => { const defaultProps: ColumnOptionProps = { @@ -53,8 +49,8 @@ describe('ColumnOption', () => { expect(lbl).toHaveLength(1); expect(lbl.first().text()).toBe('Foo'); }); - it('shows 1 InfoTooltipWithTrigger', () => { - expect(wrapper.find(InfoTooltipWithTrigger)).toHaveLength(1); + it('shows SQL Popover trigger', () => { + expect(wrapper.find(SQLPopover)).toHaveLength(1); }); it('shows a label with column_name when no verbose_name', () => { delete props.column.verbose_name; diff --git a/superset-frontend/packages/superset-ui-chart-controls/test/components/MetricOption.test.tsx b/superset-frontend/packages/superset-ui-chart-controls/test/components/MetricOption.test.tsx index e71882bd3ee14..59ba64c7bfe6f 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/test/components/MetricOption.test.tsx +++ b/superset-frontend/packages/superset-ui-chart-controls/test/components/MetricOption.test.tsx @@ -51,18 +51,21 @@ describe('MetricOption', () => { expect(lbl).toHaveLength(1); expect(lbl.first().text()).toBe('Foo'); }); - it('shows 2 InfoTooltipWithTrigger', () => { - expect(wrapper.find('InfoTooltipWithTrigger')).toHaveLength(2); + it('shows a InfoTooltipWithTrigger', () => { + expect(wrapper.find('InfoTooltipWithTrigger')).toHaveLength(1); + }); + it('shows SQL Popover trigger', () => { + expect(wrapper.find('SQLPopover')).toHaveLength(1); }); it('shows a label with metric_name when no verbose_name', () => { props.metric.verbose_name = ''; wrapper = shallow(factory(props)); expect(wrapper.find('.option-label').first().text()).toBe('foo'); }); - it('shows only 1 InfoTooltipWithTrigger when no warning', () => { + it('doesnt show InfoTooltipWithTrigger when no warning', () => { props.metric.warning_text = ''; wrapper = shallow(factory(props)); - expect(wrapper.find('InfoTooltipWithTrigger')).toHaveLength(1); + expect(wrapper.find('InfoTooltipWithTrigger')).toHaveLength(0); }); it('sets target="_blank" when openInNewWindow is true', () => { props.url = 'https://github.com/apache/incubator-superset'; diff --git a/superset-frontend/src/SqlLab/components/TemplateParamsEditor/TemplateParamsEditor.test.tsx b/superset-frontend/src/SqlLab/components/TemplateParamsEditor/TemplateParamsEditor.test.tsx index e663704ba21b8..bc04030d28c8e 100644 --- a/superset-frontend/src/SqlLab/components/TemplateParamsEditor/TemplateParamsEditor.test.tsx +++ b/superset-frontend/src/SqlLab/components/TemplateParamsEditor/TemplateParamsEditor.test.tsx @@ -24,7 +24,6 @@ import { getByText, waitFor, } from 'spec/helpers/testing-library'; -import brace from 'brace'; import { ThemeProvider, supersetTheme } from '@superset-ui/core'; import TemplateParamsEditor from 'src/SqlLab/components/TemplateParamsEditor'; @@ -48,8 +47,6 @@ describe('TemplateParamsEditor', () => { { wrapper: ThemeWrapper }, ); fireEvent.click(getByText(container, 'Parameters')); - const spy = jest.spyOn(brace, 'acequire'); - spy.mockReturnValue({ setCompleters: () => 'foo' }); await waitFor(() => { expect(baseElement.querySelector('#ace-editor')).toBeInTheDocument(); }); diff --git a/superset-frontend/src/SqlLab/components/TemplateParamsEditor/index.tsx b/superset-frontend/src/SqlLab/components/TemplateParamsEditor/index.tsx index 4bedbfcecce31..62d0a7209de1c 100644 --- a/superset-frontend/src/SqlLab/components/TemplateParamsEditor/index.tsx +++ b/superset-frontend/src/SqlLab/components/TemplateParamsEditor/index.tsx @@ -74,7 +74,6 @@ function TemplateParamsEditor({ syntax.

import('brace/mode/sql'), @@ -101,7 +102,6 @@ export default function AsyncAceEditor( }: AsyncAceEditorOptions = {}, ) { return AsyncEsmComponent(async () => { - const { default: ace } = await import('brace'); const { default: ReactAceEditor } = await import('react-ace'); await Promise.all(aceModules.map(x => aceModuleLoaders[x]())); @@ -126,7 +126,7 @@ export default function AsyncAceEditor( ref, ) { if (keywords) { - const langTools = ace.acequire('ace/ext/language_tools'); + const langTools = acequire('ace/ext/language_tools'); const completer = { getCompletions: ( editor: AceEditor,