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
apache · Apr 3, 2022 · 60dcd65 · 60dcd65
1 parent 90dbe8d
commit 60dcd65
Show file tree

Hide file tree

Showing 11 changed files with 110 additions and 48 deletions.
diff --git a/superset-frontend/package-lock.json b/superset-frontend/package-lock.json
diff --git 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
@@ -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
@@ -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)}
         </span>
       </Tooltip>
-
-      {hasExpression && (
-        <InfoTooltipWithTrigger
-          className="m-r-5 text-muted"
-          icon="question-circle-o"
-          tooltip={column.expression}
-          label={`expr-${column.column_name}`}
-          placement="top"
-        />
-      )}
-
+      {hasExpression && <SQLPopover sqlExpression={expression} />}
       {column.is_certified && (
         <CertifiedIconWithTooltip
           metricName={column.metric_name}

diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/components/MetricOption.tsx b/superset-frontend/packages/superset-ui-chart-controls/src/components/MetricOption.tsx
@@ -29,6 +29,7 @@ import { ColumnTypeLabel } from './ColumnTypeLabel/ColumnTypeLabel';
 import CertifiedIconWithTooltip from './CertifiedIconWithTooltip';
 import Tooltip from './Tooltip';
 import { getMetricTooltipNode } from './labelUtils';
+import { SQLPopover } from './SQLPopover';
 
 const FlexRowContainer = styled.div`
   align-items: center;
@@ -89,13 +90,8 @@ export function MetricOption({
           {link}
         </span>
       </Tooltip>
-      {showFormula && (
-        <InfoTooltipWithTrigger
-          className="text-muted m-r-5"
-          icon="question-circle-o"
-          tooltip={metric.expression}
-          label={`expr-${metric.metric_name}`}
-        />
+      {showFormula && metric.expression && (
+        <SQLPopover sqlExpression={metric.expression} />
       )}
       {metric.is_certified && (
         <CertifiedIconWithTooltip

diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/components/SQLPopover.tsx b/superset-frontend/packages/superset-ui-chart-controls/src/components/SQLPopover.tsx
@@ -0,0 +1,70 @@
+/**
+ * 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 React from 'react';
+import { Popover } from 'antd';
+import type { PopoverProps } from 'antd/lib/popover';
+import AceEditor from 'react-ace';
+import { CalculatorOutlined } from '@ant-design/icons';
+import { css, styled, useTheme, t } from '@superset-ui/core';
+import 'ace-builds/src-noconflict/mode-sql';
+
+const StyledCalculatorIcon = styled(CalculatorOutlined)`
+  ${({ theme }) => 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 (
+    <Popover
+      content={
+        <AceEditor
+          mode="sql"
+          value={props.sqlExpression}
+          editorProps={{ $blockScrolling: true }}
+          setOptions={{
+            highlightActiveLine: false,
+            highlightGutterLine: false,
+          }}
+          minLines={2}
+          maxLines={6}
+          readOnly
+          wrapEnabled
+          style={{
+            border: `1px solid ${theme.colors.grayscale.light2}`,
+            background: theme.colors.secondary.light5,
+            maxWidth: theme.gridUnit * 100,
+          }}
+        />
+      }
+      placement="bottomLeft"
+      arrowPointAtCenter
+      title={t('SQL expression')}
+      {...props}
+    >
+      <StyledCalculatorIcon />
+    </Popover>
+  );
+};
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
@@ -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
@@ -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
@@ -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
@@ -74,7 +74,6 @@ function TemplateParamsEditor({
         syntax.
       </p>
       <StyledConfigEditor
-        keywords={[]}
         mode={language}
         minLines={25}
         maxLines={50}

diff --git a/superset-frontend/src/components/AsyncAceEditor/index.tsx b/superset-frontend/src/components/AsyncAceEditor/index.tsx
@@ -24,6 +24,7 @@ import {
   TextMode as OrigTextMode,
 } from 'brace';
 import AceEditor, { IAceEditorProps } from 'react-ace';
+import { acequire } from 'ace-builds/src-noconflict/ace';
 import AsyncEsmComponent, {
   PlaceholderProps,
 } from 'src/components/AsyncEsmComponent';
@@ -55,7 +56,7 @@ export interface AceCompleterKeyword extends AceCompleterKeywordData {
 
 /**
  * Async loaders to import brace modules. Must manually create call `import(...)`
- * promises because webpack can only analyze asycn imports statically.
+ * promises because webpack can only analyze async imports statically.
  */
 const aceModuleLoaders = {
   'mode/sql': () => 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,