From 4d42f8933500793f7fa7dcc503035019a528e752 Mon Sep 17 00:00:00 2001 From: Jesse Yang Date: Thu, 10 Sep 2020 18:09:30 -0700 Subject: [PATCH 01/11] feat: load AnnotationLayer and mathjs on demand --- .../explore/visualizations/line.test.ts | 8 +++ .../src/components/AsyncEsmComponent.tsx | 68 +++++++++++++++++++ .../controls/AnnotationLayerControl.jsx | 12 +++- superset-frontend/webpack.config.js | 3 +- 4 files changed, 87 insertions(+), 4 deletions(-) create mode 100644 superset-frontend/src/components/AsyncEsmComponent.tsx diff --git a/superset-frontend/cypress-base/cypress/integration/explore/visualizations/line.test.ts b/superset-frontend/cypress-base/cypress/integration/explore/visualizations/line.test.ts index 34f80f2b4916f..e3a95749365c1 100644 --- a/superset-frontend/cypress-base/cypress/integration/explore/visualizations/line.test.ts +++ b/superset-frontend/cypress-base/cypress/integration/explore/visualizations/line.test.ts @@ -33,6 +33,13 @@ describe('Visualization > Line', () => { cy.get('.alert-warning').contains(`"Metrics" cannot be empty`); }); + it('should load mathjs on demand', () => { + cy.get('script[src*="mathjs"]').should('have.length', 0); + cy.contains('Add Annotation Layer').scrollIntoView().click(); + cy.get('script[src*="mathjs"]').should('have.length', 1); + cy.contains('Layer Configuration'); + }); + it('should not show validator error when metric added', () => { const formData = { ...LINE_CHART_DEFAULTS, metrics: [] }; cy.visitChartByParams(JSON.stringify(formData)); @@ -68,6 +75,7 @@ describe('Visualization > Line', () => { const formData = { ...LINE_CHART_DEFAULTS, metrics: [NUM_METRIC] }; cy.visitChartByParams(JSON.stringify(formData)); cy.verifySliceSuccess({ waitAlias: '@getJson', chartSelector: 'svg' }); + cy.get('script[src*="mathjs"]').should('have.length', 1); }); it('should work with groupby', () => { diff --git a/superset-frontend/src/components/AsyncEsmComponent.tsx b/superset-frontend/src/components/AsyncEsmComponent.tsx new file mode 100644 index 0000000000000..caa983d6054e3 --- /dev/null +++ b/superset-frontend/src/components/AsyncEsmComponent.tsx @@ -0,0 +1,68 @@ +/** + * 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, { useEffect, useState } from 'react'; + +/** + * Create an asynchronously imported component, will re-render once import is + * complete. + */ +export default function AsyncEsmComponent

( + /** + * A promise generator that returns the React component to render. + */ + loadComponent: + | Promise> + | (() => Promise>), + /** + * Placeholder while still importing. + */ + placeholder?: React.ComponentType

, +) { + let component: React.ComponentType

; + let promise: Promise; + + // load component on initialization + if (loadComponent instanceof Promise) { + promise = loadComponent; + promise.then(loadedComponent => { + component = loadedComponent; + }); + } + + return function AsyncComponent(props: P) { + const [loaded, setLoaded] = useState(component !== undefined); + useEffect(() => { + if (!loaded) { + if (!promise && typeof loadComponent === 'function') { + promise = loadComponent(); + // save to cache cache + promise.then(loadedComponent => { + component = loadedComponent; + }); + } + // update state to trigger a re-render + promise.then(() => { + setLoaded(true); + }); + } + }); + const Component = component || placeholder; + return Component ? : null; + }; +} diff --git a/superset-frontend/src/explore/components/controls/AnnotationLayerControl.jsx b/superset-frontend/src/explore/components/controls/AnnotationLayerControl.jsx index f6f49f204eae3..1e5bacb9c94e0 100644 --- a/superset-frontend/src/explore/components/controls/AnnotationLayerControl.jsx +++ b/superset-frontend/src/explore/components/controls/AnnotationLayerControl.jsx @@ -27,10 +27,15 @@ import { import { connect } from 'react-redux'; import { t, withTheme } from '@superset-ui/core'; import { InfoTooltipWithTrigger } from '@superset-ui/chart-controls'; -import { getChartKey } from '../../exploreUtils'; -import { runAnnotationQuery } from '../../../chart/chartAction'; +import AsyncEsmComponent from 'src/components/AsyncEsmComponent'; +import { getChartKey } from 'src/explore/exploreUtils'; +import { runAnnotationQuery } from 'src/chart/chartAction'; -import AnnotationLayer from './AnnotationLayer'; +const AnnotationLayer = AsyncEsmComponent( + () => import('./AnnotationLayer').then(module => module.default), + // size of overlay inner content + () =>

, +); const propTypes = { colorScheme: PropTypes.string.isRequired, @@ -111,6 +116,7 @@ class AnnotationLayerControl extends React.PureComponent { > Date: Thu, 10 Sep 2020 20:12:10 -0700 Subject: [PATCH 02/11] Make brace async for dashboard page Load markdown async Add a test case for markdown edit --- ...ashboard.helper.js => dashboard.helper.ts} | 20 ++++++ .../integration/dashboard/edit_mode.test.js | 18 ++--- .../integration/dashboard/markdown.test.ts | 43 ++++++++++++ .../src/components/AsyncEsmComponent.tsx | 63 +++++++++++------ .../src/dashboard/components/CssEditor.jsx | 13 ++-- .../src/dashboard/components/Header.jsx | 22 +++--- .../components/gridComponents/Markdown.jsx | 21 ++++-- .../src/datasource/DatasourceEditor.jsx | 28 ++++---- .../src/datasource/DatasourceModal.tsx | 8 ++- .../controls/AnnotationLayerControl.jsx | 2 +- .../components/controls/DatasourceControl.jsx | 8 ++- .../components/controls/SpatialControl.jsx | 4 +- .../src/explore/components/controls/index.js | 33 +++++---- .../src/explore/components/controls/index.tsx | 70 +++++++++++++++++++ .../visualizations/FilterBox/FilterBox.jsx | 20 +++--- superset-frontend/webpack.config.js | 8 ++- 16 files changed, 275 insertions(+), 106 deletions(-) rename superset-frontend/cypress-base/cypress/integration/dashboard/{dashboard.helper.js => dashboard.helper.ts} (62%) create mode 100644 superset-frontend/cypress-base/cypress/integration/dashboard/markdown.test.ts create mode 100644 superset-frontend/src/explore/components/controls/index.tsx diff --git a/superset-frontend/cypress-base/cypress/integration/dashboard/dashboard.helper.js b/superset-frontend/cypress-base/cypress/integration/dashboard/dashboard.helper.ts similarity index 62% rename from superset-frontend/cypress-base/cypress/integration/dashboard/dashboard.helper.js rename to superset-frontend/cypress-base/cypress/integration/dashboard/dashboard.helper.ts index 250eb5e15c0a2..810b7f5a25024 100644 --- a/superset-frontend/cypress-base/cypress/integration/dashboard/dashboard.helper.js +++ b/superset-frontend/cypress-base/cypress/integration/dashboard/dashboard.helper.ts @@ -21,3 +21,23 @@ export const TABBED_DASHBOARD = '/superset/dashboard/tabbed_dash/'; export const CHECK_DASHBOARD_FAVORITE_ENDPOINT = '/superset/favstar/Dashboard/*/count'; + +export function drag(selector: string, content: string | number | RegExp) { + // drag-n-drop + const dataTransfer = { data: {} }; + return { + to(target: string | Cypress.Chainable) { + cy.get('.dragdroppable') + .contains(selector, content) + .trigger('mousedown', { which: 1 }) + .trigger('dragstart', { dataTransfer }) + .trigger('drag', {}); + + (typeof target === 'string' ? cy.get(target) : target) + .trigger('dragover', { dataTransfer }) + .trigger('drop', { dataTransfer }) + .trigger('dragend', { dataTransfer }) + .trigger('mouseup', { which: 1 }); + }, + }; +} diff --git a/superset-frontend/cypress-base/cypress/integration/dashboard/edit_mode.test.js b/superset-frontend/cypress-base/cypress/integration/dashboard/edit_mode.test.js index 1db86b5cda794..6202eb9f853e6 100644 --- a/superset-frontend/cypress-base/cypress/integration/dashboard/edit_mode.test.js +++ b/superset-frontend/cypress-base/cypress/integration/dashboard/edit_mode.test.js @@ -16,7 +16,7 @@ * specific language governing permissions and limitations * under the License. */ -import { WORLD_HEALTH_DASHBOARD } from './dashboard.helper'; +import { WORLD_HEALTH_DASHBOARD, drag } from './dashboard.helper'; describe('Dashboard edit mode', () => { beforeEach(() => { @@ -45,19 +45,9 @@ describe('Dashboard edit mode', () => { .find('.chart-card-container') .contains('Box plot'); - // drag-n-drop - const dataTransfer = { data: {} }; - cy.get('.dragdroppable') - .contains('Box plot') - .trigger('mousedown', { which: 1 }) - .trigger('dragstart', { dataTransfer }) - .trigger('drag', {}); - cy.get('.grid-content div.grid-row.background--transparent') - .last() - .trigger('dragover', { dataTransfer }) - .trigger('drop', { dataTransfer }) - .trigger('dragend', { dataTransfer }) - .trigger('mouseup', { which: 1 }); + drag('.chart-card', 'Box plot').to( + '.grid-row.background--transparent:last', + ); // add back to dashboard cy.get('.grid-container .box_plot').should('be.exist'); diff --git a/superset-frontend/cypress-base/cypress/integration/dashboard/markdown.test.ts b/superset-frontend/cypress-base/cypress/integration/dashboard/markdown.test.ts new file mode 100644 index 0000000000000..febd92fe84b16 --- /dev/null +++ b/superset-frontend/cypress-base/cypress/integration/dashboard/markdown.test.ts @@ -0,0 +1,43 @@ +/** + * 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 { TABBED_DASHBOARD, drag } from './dashboard.helper'; + +describe('Dashboard edit markdown', () => { + beforeEach(() => { + cy.server(); + cy.login(); + cy.visit(TABBED_DASHBOARD); + }); + + it('should load brace on demand', () => { + cy.get('script[src*="brace"]').should('have.length', 0); + cy.get('.dashboard-header [data-test=pencil]').click(); + cy.get('script[src*="brace"]').should('have.length', 0); + drag('.new-component', 'Markdown').to( + '.grid-row.background--transparent:first', + ); + cy.get('script[src*="brace"]').should('have.length', 1); + cy.contains('h3', '✨Markdown').click(); + cy.get('.ace_content').contains( + 'Click here to edit [markdown](https://bit.ly/1dQOfRK)', + ); + cy.get('.grid-row.background--transparent:first').click('right'); + cy.get('.ace_content').should('not.exist'); + }); +}); diff --git a/superset-frontend/src/components/AsyncEsmComponent.tsx b/superset-frontend/src/components/AsyncEsmComponent.tsx index caa983d6054e3..601b09563843e 100644 --- a/superset-frontend/src/components/AsyncEsmComponent.tsx +++ b/superset-frontend/src/components/AsyncEsmComponent.tsx @@ -18,51 +18,68 @@ */ import React, { useEffect, useState } from 'react'; +type PlaceholderProps = { + width?: string | number; + height?: string | number; +} & { + [key: string]: any; +}; + +function DefaultPlaceholder({ width, height }: PlaceholderProps) { + // `|| null` is for in case of width=0 or height=0. + return (width && height &&
) || null; +} + /** - * Create an asynchronously imported component, will re-render once import is - * complete. + * Asynchronously import an ES module as a React component, render a placeholder + * first (if provided) and re-render once import is complete. */ -export default function AsyncEsmComponent

( +export default function AsyncEsmComponent< + P = PlaceholderProps, + M = React.ComponentType

| { default: React.ComponentType

} +>( /** * A promise generator that returns the React component to render. */ - loadComponent: - | Promise> - | (() => Promise>), + loadComponent: Promise | (() => Promise), /** * Placeholder while still importing. */ - placeholder?: React.ComponentType

, + placeholder: React.ComponentType

| null = DefaultPlaceholder, ) { let component: React.ComponentType

; - let promise: Promise; + let promise: Promise | undefined; - // load component on initialization - if (loadComponent instanceof Promise) { - promise = loadComponent; - promise.then(loadedComponent => { - component = loadedComponent; + /** + * Safely wait for promise, make sure the loader function only execute once. + */ + function waitForPromise() { + if (!promise) { + // load component on initialization + promise = + loadComponent instanceof Promise ? loadComponent : loadComponent(); + } + promise.then(result => { + component = ((result as { default?: React.ComponentType

}).default || + result) as React.ComponentType

; }); + return promise; } - return function AsyncComponent(props: P) { + function AsyncComponent(props: P) { const [loaded, setLoaded] = useState(component !== undefined); useEffect(() => { if (!loaded) { - if (!promise && typeof loadComponent === 'function') { - promise = loadComponent(); - // save to cache cache - promise.then(loadedComponent => { - component = loadedComponent; - }); - } // update state to trigger a re-render - promise.then(() => { + waitForPromise().then(() => { setLoaded(true); }); } }); const Component = component || placeholder; return Component ? : null; - }; + } + AsyncComponent.load = waitForPromise; + + return AsyncComponent; } diff --git a/superset-frontend/src/dashboard/components/CssEditor.jsx b/superset-frontend/src/dashboard/components/CssEditor.jsx index a412f6dfd6223..19e544581e971 100644 --- a/superset-frontend/src/dashboard/components/CssEditor.jsx +++ b/superset-frontend/src/dashboard/components/CssEditor.jsx @@ -19,12 +19,15 @@ import React from 'react'; import PropTypes from 'prop-types'; import Select from 'src/components/Select'; -import AceEditor from 'react-ace'; -import 'brace/mode/css'; -import 'brace/theme/github'; import { t } from '@superset-ui/core'; +import AsyncEsmComponent from 'src/components/AsyncEsmComponent'; +import ModalTrigger from 'src/components/ModalTrigger'; -import ModalTrigger from '../../components/ModalTrigger'; +const AceEditor = AsyncEsmComponent(async () => { + const ace = await import('react-ace'); + await Promise.all([import('brace/mode/css'), import('brace/theme/github')]); + return ace.default; +}); const propTypes = { initialCss: PropTypes.string, @@ -90,7 +93,7 @@ class CssEditor extends React.PureComponent { import('./PropertiesModal')); + const propTypes = { addSuccessToast: PropTypes.func.isRequired, addDangerToast: PropTypes.func.isRequired, diff --git a/superset-frontend/src/dashboard/components/gridComponents/Markdown.jsx b/superset-frontend/src/dashboard/components/gridComponents/Markdown.jsx index 8da74f30e8f08..9fdfb5fcbec6a 100644 --- a/superset-frontend/src/dashboard/components/gridComponents/Markdown.jsx +++ b/superset-frontend/src/dashboard/components/gridComponents/Markdown.jsx @@ -20,10 +20,9 @@ import React from 'react'; import PropTypes from 'prop-types'; import ReactMarkdown from 'react-markdown'; import cx from 'classnames'; -import AceEditor from 'react-ace'; -import 'brace/mode/markdown'; -import 'brace/theme/textmate'; import { t } from '@superset-ui/core'; +import { Logger, LOG_ACTIONS_RENDER_CHART } from 'src/logger/LogUtils'; +import AsyncEsmComponent from 'src/components/AsyncEsmComponent'; import DeleteComponentButton from '../DeleteComponentButton'; import DragDroppable from '../dnd/DragDroppable'; @@ -37,7 +36,17 @@ import { GRID_MIN_ROW_UNITS, GRID_BASE_UNIT, } from '../../util/constants'; -import { Logger, LOG_ACTIONS_RENDER_CHART } from '../../../logger/LogUtils'; + +async function loadAceEditor() { + const ace = await import('react-ace'); + await Promise.all([ + import('brace/mode/markdown'), + import('brace/theme/textmate'), + ]); + return ace.default; +} + +const AceEditor = AsyncEsmComponent(loadAceEditor); const propTypes = { id: PropTypes.string.isRequired, @@ -166,6 +175,10 @@ class Markdown extends React.PureComponent { ) { this.state.editor.resize(true); } + // pre-load AceEditor when entering edit mode + if (this.props.editMode) { + loadAceEditor(); + } } componentDidCatch() { diff --git a/superset-frontend/src/datasource/DatasourceEditor.jsx b/superset-frontend/src/datasource/DatasourceEditor.jsx index ce85ef725ec82..5efc4328097b0 100644 --- a/superset-frontend/src/datasource/DatasourceEditor.jsx +++ b/superset-frontend/src/datasource/DatasourceEditor.jsx @@ -27,20 +27,22 @@ import Button from 'src/components/Button'; import Loading from 'src/components/Loading'; import TableSelector from 'src/components/TableSelector'; import CertifiedIconWithTooltip from 'src/components/CertifiedIconWithTooltip'; +import EditableTitle from 'src/components/EditableTitle'; -import getClientErrorObject from '../utils/getClientErrorObject'; -import CheckboxControl from '../explore/components/controls/CheckboxControl'; -import TextControl from '../explore/components/controls/TextControl'; -import SelectControl from '../explore/components/controls/SelectControl'; -import TextAreaControl from '../explore/components/controls/TextAreaControl'; -import SelectAsyncControl from '../explore/components/controls/SelectAsyncControl'; -import SpatialControl from '../explore/components/controls/SpatialControl'; -import CollectionTable from '../CRUD/CollectionTable'; -import EditableTitle from '../components/EditableTitle'; -import Fieldset from '../CRUD/Fieldset'; -import Field from '../CRUD/Field'; - -import withToasts from '../messageToasts/enhancers/withToasts'; +import getClientErrorObject from 'src/utils/getClientErrorObject'; + +import CheckboxControl from 'src/explore/components/controls/CheckboxControl'; +import TextControl from 'src/explore/components/controls/TextControl'; +import SelectControl from 'src/explore/components/controls/SelectControl'; +import TextAreaControl from 'src/explore/components/controls/TextAreaControl'; +import SelectAsyncControl from 'src/explore/components/controls/SelectAsyncControl'; +import SpatialControl from 'src/explore/components/controls/SpatialControl'; + +import CollectionTable from 'src/CRUD/CollectionTable'; +import Fieldset from 'src/CRUD/Fieldset'; +import Field from 'src/CRUD/Field'; + +import withToasts from 'src/messageToasts/enhancers/withToasts'; const DatasourceContainer = styled.div` .tab-content { diff --git a/superset-frontend/src/datasource/DatasourceModal.tsx b/superset-frontend/src/datasource/DatasourceModal.tsx index 4f60028c4f842..c139360e0f5e9 100644 --- a/superset-frontend/src/datasource/DatasourceModal.tsx +++ b/superset-frontend/src/datasource/DatasourceModal.tsx @@ -22,10 +22,12 @@ import Button from 'src/components/Button'; // @ts-ignore import Dialog from 'react-bootstrap-dialog'; import { t, SupersetClient } from '@superset-ui/core'; +import AsyncEsmComponent from 'src/components/AsyncEsmComponent'; -import getClientErrorObject from '../utils/getClientErrorObject'; -import DatasourceEditor from './DatasourceEditor'; -import withToasts from '../messageToasts/enhancers/withToasts'; +import getClientErrorObject from 'src/utils/getClientErrorObject'; +import withToasts from 'src/messageToasts/enhancers/withToasts'; + +const DatasourceEditor = AsyncEsmComponent(() => import('./DatasourceEditor')); interface DatasourceModalProps { addSuccessToast: (msg: string) => void; diff --git a/superset-frontend/src/explore/components/controls/AnnotationLayerControl.jsx b/superset-frontend/src/explore/components/controls/AnnotationLayerControl.jsx index 1e5bacb9c94e0..f8266373c9c26 100644 --- a/superset-frontend/src/explore/components/controls/AnnotationLayerControl.jsx +++ b/superset-frontend/src/explore/components/controls/AnnotationLayerControl.jsx @@ -32,7 +32,7 @@ import { getChartKey } from 'src/explore/exploreUtils'; import { runAnnotationQuery } from 'src/chart/chartAction'; const AnnotationLayer = AsyncEsmComponent( - () => import('./AnnotationLayer').then(module => module.default), + () => import('./AnnotationLayer'), // size of overlay inner content () =>

, ); diff --git a/superset-frontend/src/explore/components/controls/DatasourceControl.jsx b/superset-frontend/src/explore/components/controls/DatasourceControl.jsx index c06d493b7429a..04f3c031cfa80 100644 --- a/superset-frontend/src/explore/components/controls/DatasourceControl.jsx +++ b/superset-frontend/src/explore/components/controls/DatasourceControl.jsx @@ -32,10 +32,12 @@ import { t } from '@superset-ui/core'; import { ColumnOption, MetricOption } from '@superset-ui/chart-controls'; import Label from 'src/components/Label'; +import TooltipWrapper from 'src/components/TooltipWrapper'; + +import DatasourceModal from 'src/datasource/DatasourceModal'; +import ChangeDatasourceModal from 'src/datasource/ChangeDatasourceModal'; + import ControlHeader from '../ControlHeader'; -import DatasourceModal from '../../../datasource/DatasourceModal'; -import ChangeDatasourceModal from '../../../datasource/ChangeDatasourceModal'; -import TooltipWrapper from '../../../components/TooltipWrapper'; import './DatasourceControl.less'; const propTypes = { diff --git a/superset-frontend/src/explore/components/controls/SpatialControl.jsx b/superset-frontend/src/explore/components/controls/SpatialControl.jsx index 9cbfa45c6f51b..9553200aab5bb 100644 --- a/superset-frontend/src/explore/components/controls/SpatialControl.jsx +++ b/superset-frontend/src/explore/components/controls/SpatialControl.jsx @@ -23,10 +23,10 @@ import Button from 'src/components/Button'; import { t } from '@superset-ui/core'; import Label from 'src/components/Label'; +import PopoverSection from 'src/components/PopoverSection'; +import Checkbox from 'src/components/Checkbox'; import ControlHeader from '../ControlHeader'; import SelectControl from './SelectControl'; -import PopoverSection from '../../../components/PopoverSection'; -import Checkbox from '../../../components/Checkbox'; const spatialTypes = { latlong: 'latlong', diff --git a/superset-frontend/src/explore/components/controls/index.js b/superset-frontend/src/explore/components/controls/index.js index aa084500f006e..df37da0a956d3 100644 --- a/superset-frontend/src/explore/components/controls/index.js +++ b/superset-frontend/src/explore/components/controls/index.js @@ -16,6 +16,7 @@ * specific language governing permissions and limitations * under the License. */ +import AsyncEsmComponent from 'src/components/AsyncEsmComponent'; import AnnotationLayerControl from './AnnotationLayerControl'; import BoundsControl from './BoundsControl'; import CheckboxControl from './CheckboxControl'; @@ -25,19 +26,15 @@ import ColorPickerControl from './ColorPickerControl'; import ColorSchemeControl from './ColorSchemeControl'; import DatasourceControl from './DatasourceControl'; import DateFilterControl from './DateFilterControl'; -import FixedOrMetricControl from './FixedOrMetricControl'; import HiddenControl from './HiddenControl'; import SelectAsyncControl from './SelectAsyncControl'; import SelectControl from './SelectControl'; import SliderControl from './SliderControl'; import SpatialControl from './SpatialControl'; -import TextAreaControl from './TextAreaControl'; import TextControl from './TextControl'; import TimeSeriesColumnControl from './TimeSeriesColumnControl'; import ViewportControl from './ViewportControl'; import VizTypeControl from './VizTypeControl'; -import MetricsControl from './MetricsControl'; -import AdhocFilterControl from './AdhocFilterControl'; import FilterBoxItemControl from './FilterBoxItemControl'; import withVerification from './withVerification'; @@ -51,34 +48,36 @@ const controlMap = { ColorSchemeControl, DatasourceControl, DateFilterControl, - FixedOrMetricControl, + FixedOrMetricControl: AsyncEsmComponent(() => + import('./FixedOrMetricControl'), + ), HiddenControl, SelectAsyncControl, SelectControl, SliderControl, SpatialControl, - TextAreaControl, + TextAreaControl: AsyncEsmComponent(() => import('./TextAreaControl')), TextControl, TimeSeriesColumnControl, ViewportControl, VizTypeControl, - MetricsControl, - AdhocFilterControl, + MetricsControl: AsyncEsmComponent(() => import('./MetricsControl')), + AdhocFilterControl: AsyncEsmComponent(() => import('./AdhocFilterControl')), FilterBoxItemControl, - MetricsControlVerifiedOptions: withVerification( - MetricsControl, - 'metric_name', - 'savedMetrics', - ), SelectControlVerifiedOptions: withVerification( SelectControl, 'column_name', 'options', ), - AdhocFilterControlVerifiedOptions: withVerification( - AdhocFilterControl, - 'column_name', - 'columns', + MetricsControlVerifiedOptions: AsyncEsmComponent(() => + import('./MetricsControl').then(module => { + return withVerification(module.default, 'metric_name', 'savedMetrics'); + }), + ), + AdhocFilterControlVerifiedOptions: AsyncEsmComponent(() => + import('./AdhocFilterControl').then(module => { + return withVerification(module.default, 'column_name', 'columns'); + }), ), }; export default controlMap; diff --git a/superset-frontend/src/explore/components/controls/index.tsx b/superset-frontend/src/explore/components/controls/index.tsx new file mode 100644 index 0000000000000..c56e7e80c77f4 --- /dev/null +++ b/superset-frontend/src/explore/components/controls/index.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 AsyncEsmComponent from 'src/components/AsyncEsmComponent'; +import HiddenControl from './HiddenControl'; +import withVerification from './withVerification'; + +const asyncComponent = (importer: Parameters[0]) => + AsyncEsmComponent(importer, null); + +const controlMap = { + HiddenControl, + AdhocFilterControl: asyncComponent(() => import('./AdhocFilterControl')), + AnnotationLayerControl: asyncComponent( + () => import('./AnnotationLayerControl'), + ), + BoundsControl: asyncComponent(() => import('./BoundsControl')), + CheckboxControl: asyncComponent(() => import('./CheckboxControl')), + CollectionControl: asyncComponent(() => import('./CollectionControl')), + ColorMapControl: asyncComponent(() => import('./ColorMapControl')), + ColorPickerControl: asyncComponent(() => import('./ColorPickerControl')), + ColorSchemeControl: asyncComponent(() => import('./ColorSchemeControl')), + DatasourceControl: asyncComponent(() => import('./DatasourceControl')), + DateFilterControl: asyncComponent(() => import('./DateFilterControl')), + FilterBoxItemControl: asyncComponent(() => import('./FilterBoxItemControl')), + FixedOrMetricControl: asyncComponent(() => import('./FixedOrMetricControl')), + SelectAsyncControl: asyncComponent(() => import('./SelectAsyncControl')), + SelectControl: asyncComponent(() => import('./SelectControl')), + SliderControl: asyncComponent(() => import('./SliderControl')), + SpatialControl: asyncComponent(() => import('./SpatialControl')), + TextAreaControl: asyncComponent(() => import('./TextAreaControl')), + TextControl: asyncComponent(() => import('./TextControl')), + TimeSeriesColumnControl: asyncComponent( + () => import('./TimeSeriesColumnControl'), + ), + ViewportControl: asyncComponent(() => import('./ViewportControl')), + VizTypeControl: asyncComponent(() => import('./VizTypeControl')), + MetricsControl: asyncComponent(() => import('./MetricsControl')), + SelectControlVerifiedOptions: asyncComponent(async () => { + const { default: SelectControl } = await import('./SelectControl'); + return withVerification(SelectControl, 'column_name', 'options'); + }), + MetricsControlVerifiedOptions: asyncComponent(async () => { + const { default: MetricsControl } = await import('./MetricsControl'); + return withVerification(MetricsControl, 'metric_name', 'savedMetrics'); + }), + AdhocFilterControlVerifiedOptions: asyncComponent(async () => { + const { default: AdhocFilterControl } = await import( + './AdhocFilterControl' + ); + return withVerification(AdhocFilterControl, 'column_name', 'columns'); + }), +}; + +export default controlMap; diff --git a/superset-frontend/src/visualizations/FilterBox/FilterBox.jsx b/superset-frontend/src/visualizations/FilterBox/FilterBox.jsx index 51dfc417c1cc4..311b7171999c7 100644 --- a/superset-frontend/src/visualizations/FilterBox/FilterBox.jsx +++ b/superset-frontend/src/visualizations/FilterBox/FilterBox.jsx @@ -26,20 +26,20 @@ import { t, styled, SupersetClient } from '@superset-ui/core'; import FormLabel from 'src/components/FormLabel'; -import DateFilterControl from '../../explore/components/controls/DateFilterControl'; -import ControlRow from '../../explore/components/ControlRow'; -import Control from '../../explore/components/Control'; -import controls from '../../explore/controls'; -import { getExploreUrl } from '../../explore/exploreUtils'; -import OnPasteSelect from '../../components/Select/OnPasteSelect'; -import { getDashboardFilterKey } from '../../dashboard/util/getDashboardFilterKey'; -import { getFilterColorMap } from '../../dashboard/util/dashboardFiltersColorMap'; +import DateFilterControl from 'src/explore/components/controls/DateFilterControl'; +import ControlRow from 'src/explore/components/ControlRow'; +import Control from 'src/explore/components/Control'; +import controls from 'src/explore/controls'; +import { getExploreUrl } from 'src/explore/exploreUtils'; +import OnPasteSelect from 'src/components/Select/OnPasteSelect'; +import { getDashboardFilterKey } from 'src/dashboard/util/getDashboardFilterKey'; +import { getFilterColorMap } from 'src/dashboard/util/dashboardFiltersColorMap'; import { FILTER_CONFIG_ATTRIBUTES, FILTER_OPTIONS_LIMIT, TIME_FILTER_LABELS, -} from '../../explore/constants'; -import FilterBadgeIcon from '../../components/FilterBadgeIcon'; +} from 'src/explore/constants'; +import FilterBadgeIcon from 'src/components/FilterBadgeIcon'; import './FilterBox.less'; diff --git a/superset-frontend/webpack.config.js b/superset-frontend/webpack.config.js index e3a549a880a56..0a338bf1efcc8 100644 --- a/superset-frontend/webpack.config.js +++ b/superset-frontend/webpack.config.js @@ -55,9 +55,12 @@ const output = { if (isDevMode) { output.filename = '[name].[hash:8].entry.js'; output.chunkFilename = '[name].[hash:8].chunk.js'; -} else { +} else if (nameChunks) { output.filename = '[name].[chunkhash].entry.js'; output.chunkFilename = '[name].[chunkhash].chunk.js'; +} else { + output.filename = '[chunkhash].entry.js'; + output.chunkFilename = '[chunkhash].chunk.js'; } const plugins = [ @@ -199,6 +202,7 @@ const config = { sideEffects: true, splitChunks: { chunks: 'all', + minSize: 20000, name: nameChunks, automaticNameDelimiter: '-', minChunks: 2, @@ -214,6 +218,8 @@ const config = { 'react', 'react-dom', 'prop-types', + 'react-prop-types', + 'prop-types-extra', 'redux', 'react-redux', 'react-hot-loader', From c8b2f669e21c898133bf1b21ba58aae32ab0bc05 Mon Sep 17 00:00:00 2001 From: Jesse Yang Date: Fri, 11 Sep 2020 01:51:20 -0700 Subject: [PATCH 03/11] Make it work for datasource editor --- .../src/CRUD/CollectionTable.tsx | 6 +-- .../src/components/AsyncAceEditor.jsx | 50 +++++++++++++++++ .../src/components/AsyncEsmComponent.tsx | 18 +++++-- .../src/dashboard/components/CssEditor.jsx | 2 +- .../src/datasource/DatasourceModal.tsx | 11 +++- .../AdhocFilterEditPopoverSqlTabContent.jsx | 54 ++++++++----------- .../components/AdhocMetricEditPopover.jsx | 35 ++++-------- .../components/controls/TextAreaControl.jsx | 36 +++++++++---- .../src/explore/components/controls/index.js | 33 ++++++------ 9 files changed, 152 insertions(+), 93 deletions(-) create mode 100644 superset-frontend/src/components/AsyncAceEditor.jsx diff --git a/superset-frontend/src/CRUD/CollectionTable.tsx b/superset-frontend/src/CRUD/CollectionTable.tsx index 661547fbe55f5..f1620f43e9563 100644 --- a/superset-frontend/src/CRUD/CollectionTable.tsx +++ b/superset-frontend/src/CRUD/CollectionTable.tsx @@ -175,10 +175,10 @@ export default class CRUDCollection extends React.PureComponent< ))} {extraButtons} {allowDeletes && !allowAddItem && ( - + )} {allowAddItem && ( - + @@ -237,7 +237,7 @@ export default class CRUDCollection extends React.PureComponent< )), ); if (allowAddItem) { - tds.push(); + tds.push(); } if (allowDeletes) { tds.push( diff --git a/superset-frontend/src/components/AsyncAceEditor.jsx b/superset-frontend/src/components/AsyncAceEditor.jsx new file mode 100644 index 0000000000000..74e39d2cbb3e5 --- /dev/null +++ b/superset-frontend/src/components/AsyncAceEditor.jsx @@ -0,0 +1,50 @@ +/** + * 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 AsyncEsmComponent from 'src/components/AsyncEsmComponent'; + +const AsyncAceEditor = AsyncEsmComponent(async () => { + const { default: ace } = await import('brace'); + const { default: ReactAceEditor } = await import('react-ace'); + + await Promise.all([ + import('brace/mode/sql'), + import('brace/theme/github'), + import('brace/ext/language_tools'), + ]); + + return React.forwardRef(function ExtendedAceEditor( + { keywords, mode, ...props }, + ref, + ) { + if (keywords && mode === 'sql') { + console.log(keywords); + const langTools = ace.acequire('ace/ext/language_tools'); + const completer = { + getCompletions: (aceEditor, session, pos, prefix, callback) => { + callback(null, keywords); + }, + }; + langTools.setCompleters([completer]); + } + return ; + }); +}); + +export default AsyncAceEditor; diff --git a/superset-frontend/src/components/AsyncEsmComponent.tsx b/superset-frontend/src/components/AsyncEsmComponent.tsx index 601b09563843e..936c2a6cdf2c1 100644 --- a/superset-frontend/src/components/AsyncEsmComponent.tsx +++ b/superset-frontend/src/components/AsyncEsmComponent.tsx @@ -16,7 +16,7 @@ * specific language governing permissions and limitations * under the License. */ -import React, { useEffect, useState } from 'react'; +import React, { useEffect, useState, RefObject } from 'react'; type PlaceholderProps = { width?: string | number; @@ -66,20 +66,28 @@ export default function AsyncEsmComponent< return promise; } - function AsyncComponent(props: P) { + function AsyncComponent(props: P, ref: RefObject>) { const [loaded, setLoaded] = useState(component !== undefined); useEffect(() => { + let isMounted = true; if (!loaded) { // update state to trigger a re-render waitForPromise().then(() => { - setLoaded(true); + if (isMounted) { + setLoaded(true); + } }); } + return () => { + isMounted = false; + }; }); const Component = component || placeholder; - return Component ? : null; + return Component ? ( + + ) : null; } AsyncComponent.load = waitForPromise; - return AsyncComponent; + return React.forwardRef(AsyncComponent); } diff --git a/superset-frontend/src/dashboard/components/CssEditor.jsx b/superset-frontend/src/dashboard/components/CssEditor.jsx index 19e544581e971..92112c246568a 100644 --- a/superset-frontend/src/dashboard/components/CssEditor.jsx +++ b/superset-frontend/src/dashboard/components/CssEditor.jsx @@ -20,8 +20,8 @@ import React from 'react'; import PropTypes from 'prop-types'; import Select from 'src/components/Select'; import { t } from '@superset-ui/core'; -import AsyncEsmComponent from 'src/components/AsyncEsmComponent'; import ModalTrigger from 'src/components/ModalTrigger'; +import AsyncEsmComponent from 'src/components/AsyncEsmComponent'; const AceEditor = AsyncEsmComponent(async () => { const ace = await import('react-ace'); diff --git a/superset-frontend/src/datasource/DatasourceModal.tsx b/superset-frontend/src/datasource/DatasourceModal.tsx index c139360e0f5e9..0bb85c744f5ca 100644 --- a/superset-frontend/src/datasource/DatasourceModal.tsx +++ b/superset-frontend/src/datasource/DatasourceModal.tsx @@ -26,8 +26,16 @@ import AsyncEsmComponent from 'src/components/AsyncEsmComponent'; import getClientErrorObject from 'src/utils/getClientErrorObject'; import withToasts from 'src/messageToasts/enhancers/withToasts'; +import Loading from 'src/components/Loading'; -const DatasourceEditor = AsyncEsmComponent(() => import('./DatasourceEditor')); +const DatasourceEditor = AsyncEsmComponent( + () => import('./DatasourceEditor'), + () => ( +
+ +
+ ), +); interface DatasourceModalProps { addSuccessToast: (msg: string) => void; @@ -152,6 +160,7 @@ const DatasourceModal: FunctionComponent = ({ {show && ( diff --git a/superset-frontend/src/explore/components/AdhocFilterEditPopoverSqlTabContent.jsx b/superset-frontend/src/explore/components/AdhocFilterEditPopoverSqlTabContent.jsx index 49d616bd465bf..ccce7cced31b0 100644 --- a/superset-frontend/src/explore/components/AdhocFilterEditPopoverSqlTabContent.jsx +++ b/superset-frontend/src/explore/components/AdhocFilterEditPopoverSqlTabContent.jsx @@ -18,16 +18,12 @@ */ import React from 'react'; import PropTypes from 'prop-types'; -import AceEditor from 'react-ace'; -import ace from 'brace'; -import 'brace/mode/sql'; -import 'brace/theme/github'; -import 'brace/ext/language_tools'; import { FormGroup } from 'react-bootstrap'; import Select from 'src/components/Select'; import { t } from '@superset-ui/core'; +import AceEditor from 'src/components/AsyncAceEditor'; +import sqlKeywords from 'src/SqlLab/utils/sqlKeywords'; -import sqlKeywords from '../../SqlLab/utils/sqlKeywords'; import AdhocFilter, { EXPRESSION_TYPES, CLAUSES } from '../AdhocFilter'; import adhocMetricType from '../propTypes/adhocMetricType'; import columnType from '../propTypes/columnType'; @@ -45,8 +41,6 @@ const propTypes = { height: PropTypes.number.isRequired, }; -const langTools = ace.acequire('ace/ext/language_tools'); - export default class AdhocFilterEditPopoverSqlTabContent extends React.Component { constructor(props) { super(props); @@ -63,32 +57,12 @@ export default class AdhocFilterEditPopoverSqlTabContent extends React.Component autosize: false, clearable: false, }; - - if (langTools) { - const words = sqlKeywords.concat( - this.props.options.map(option => { - if (option.column_name) { - return { - name: option.column_name, - value: option.column_name, - score: 50, - meta: 'option', - }; - } - return null; - }), - ); - const completer = { - getCompletions: (aceEditor, session, pos, prefix, callback) => { - callback(null, words); - }, - }; - langTools.setCompleters([completer]); - } } componentDidUpdate() { - this.aceEditorRef.editor.resize(); + if (this.aceEditorRef) { + this.aceEditorRef.editor.resize(); + } } onSqlExpressionClauseChange(clause) { @@ -116,7 +90,7 @@ export default class AdhocFilterEditPopoverSqlTabContent extends React.Component } render() { - const { adhocFilter, height } = this.props; + const { adhocFilter, height, options } = this.props; const clauseSelectProps = { placeholder: t('choose WHERE or HAVING...'), @@ -124,6 +98,21 @@ export default class AdhocFilterEditPopoverSqlTabContent extends React.Component value: adhocFilter.clause, onChange: this.onSqlExpressionClauseChange, }; + const keywords = sqlKeywords.concat( + options + .map(option => { + if (option.column_name) { + return { + name: option.column_name, + value: option.column_name, + score: 50, + meta: 'option', + }; + } + return null; + }) + .filter(Boolean), + ); return ( @@ -142,6 +131,7 @@ export default class AdhocFilterEditPopoverSqlTabContent extends React.Component ({ - name: column.column_name, - value: column.column_name, - score: 50, - meta: 'column', - })), - ); - const completer = { - getCompletions: (aceEditor, session, pos, prefix, callback) => { - callback(null, words); - }, - }; - langTools.setCompleters([completer]); - } document.addEventListener('mouseup', this.onMouseUp); } @@ -199,6 +177,14 @@ export default class AdhocMetricEditPopover extends React.Component { } = this.props; const { adhocMetric } = this.state; + const keywords = sqlKeywords.concat( + columns.map(column => ({ + name: column.column_name, + value: column.column_name, + score: 50, + meta: 'column', + })), + ); const columnSelectProps = { placeholder: t('%s column(s)', columns.length), @@ -281,6 +267,7 @@ export default class AdhocMetricEditPopover extends React.Component { { + const { default: ReactAceEditor } = await import('react-ace'); + await Promise.all([ + import('brace/mode/sql'), + import('brace/mode/json'), + import('brace/mode/html'), + import('brace/mode/markdown'), + import('brace/mode/javascript'), + import('brace/theme/textmate'), + ]); + return ReactAceEditor; +}); + const propTypes = { name: PropTypes.string, onChange: PropTypes.func, @@ -65,6 +72,12 @@ const defaultProps = { }; export default class TextAreaControl extends React.Component { + constructor() { + super(); + this.onAceChangeDebounce = debounce(value => { + this.onAceChange(value); + }, 300); + } onControlChange(event) { this.props.onChange(event.target.value); } @@ -75,18 +88,19 @@ export default class TextAreaControl extends React.Component { renderEditor(inModal = false) { const value = this.props.value || ''; + const minLines = inModal ? 40 : this.props.minLines || 12; if (this.props.language) { return ( diff --git a/superset-frontend/src/explore/components/controls/index.js b/superset-frontend/src/explore/components/controls/index.js index df37da0a956d3..aa084500f006e 100644 --- a/superset-frontend/src/explore/components/controls/index.js +++ b/superset-frontend/src/explore/components/controls/index.js @@ -16,7 +16,6 @@ * specific language governing permissions and limitations * under the License. */ -import AsyncEsmComponent from 'src/components/AsyncEsmComponent'; import AnnotationLayerControl from './AnnotationLayerControl'; import BoundsControl from './BoundsControl'; import CheckboxControl from './CheckboxControl'; @@ -26,15 +25,19 @@ import ColorPickerControl from './ColorPickerControl'; import ColorSchemeControl from './ColorSchemeControl'; import DatasourceControl from './DatasourceControl'; import DateFilterControl from './DateFilterControl'; +import FixedOrMetricControl from './FixedOrMetricControl'; import HiddenControl from './HiddenControl'; import SelectAsyncControl from './SelectAsyncControl'; import SelectControl from './SelectControl'; import SliderControl from './SliderControl'; import SpatialControl from './SpatialControl'; +import TextAreaControl from './TextAreaControl'; import TextControl from './TextControl'; import TimeSeriesColumnControl from './TimeSeriesColumnControl'; import ViewportControl from './ViewportControl'; import VizTypeControl from './VizTypeControl'; +import MetricsControl from './MetricsControl'; +import AdhocFilterControl from './AdhocFilterControl'; import FilterBoxItemControl from './FilterBoxItemControl'; import withVerification from './withVerification'; @@ -48,36 +51,34 @@ const controlMap = { ColorSchemeControl, DatasourceControl, DateFilterControl, - FixedOrMetricControl: AsyncEsmComponent(() => - import('./FixedOrMetricControl'), - ), + FixedOrMetricControl, HiddenControl, SelectAsyncControl, SelectControl, SliderControl, SpatialControl, - TextAreaControl: AsyncEsmComponent(() => import('./TextAreaControl')), + TextAreaControl, TextControl, TimeSeriesColumnControl, ViewportControl, VizTypeControl, - MetricsControl: AsyncEsmComponent(() => import('./MetricsControl')), - AdhocFilterControl: AsyncEsmComponent(() => import('./AdhocFilterControl')), + MetricsControl, + AdhocFilterControl, FilterBoxItemControl, + MetricsControlVerifiedOptions: withVerification( + MetricsControl, + 'metric_name', + 'savedMetrics', + ), SelectControlVerifiedOptions: withVerification( SelectControl, 'column_name', 'options', ), - MetricsControlVerifiedOptions: AsyncEsmComponent(() => - import('./MetricsControl').then(module => { - return withVerification(module.default, 'metric_name', 'savedMetrics'); - }), - ), - AdhocFilterControlVerifiedOptions: AsyncEsmComponent(() => - import('./AdhocFilterControl').then(module => { - return withVerification(module.default, 'column_name', 'columns'); - }), + AdhocFilterControlVerifiedOptions: withVerification( + AdhocFilterControl, + 'column_name', + 'columns', ), }; export default controlMap; From 7b4b958925ed6d02e7b3b8bb47d346ae10341334 Mon Sep 17 00:00:00 2001 From: Jesse Yang Date: Fri, 11 Sep 2020 09:52:31 -0700 Subject: [PATCH 04/11] Better preload and placeholder --- .../src/components/AsyncAceEditor.jsx | 1 - .../src/components/AsyncEsmComponent.tsx | 98 +++++++++++++------ .../src/dashboard/components/Header.jsx | 4 +- .../dashboard/components/PropertiesModal.jsx | 7 +- .../src/datasource/DatasourceModal.tsx | 11 +-- .../controls/AnnotationLayerControl.jsx | 7 +- .../src/explore/components/controls/index.tsx | 70 ------------- superset-frontend/webpack.config.js | 3 +- 8 files changed, 84 insertions(+), 117 deletions(-) delete mode 100644 superset-frontend/src/explore/components/controls/index.tsx diff --git a/superset-frontend/src/components/AsyncAceEditor.jsx b/superset-frontend/src/components/AsyncAceEditor.jsx index 74e39d2cbb3e5..953f9d1070364 100644 --- a/superset-frontend/src/components/AsyncAceEditor.jsx +++ b/superset-frontend/src/components/AsyncAceEditor.jsx @@ -34,7 +34,6 @@ const AsyncAceEditor = AsyncEsmComponent(async () => { ref, ) { if (keywords && mode === 'sql') { - console.log(keywords); const langTools = ace.acequire('ace/ext/language_tools'); const completer = { getCompletions: (aceEditor, session, pos, prefix, callback) => { diff --git a/superset-frontend/src/components/AsyncEsmComponent.tsx b/superset-frontend/src/components/AsyncEsmComponent.tsx index 936c2a6cdf2c1..831be347f1e52 100644 --- a/superset-frontend/src/components/AsyncEsmComponent.tsx +++ b/superset-frontend/src/components/AsyncEsmComponent.tsx @@ -17,17 +17,32 @@ * under the License. */ import React, { useEffect, useState, RefObject } from 'react'; +import Loading from './Loading'; type PlaceholderProps = { + showLoadingForImport: boolean; width?: string | number; height?: string | number; } & { [key: string]: any; }; -function DefaultPlaceholder({ width, height }: PlaceholderProps) { - // `|| null` is for in case of width=0 or height=0. - return (width && height &&
) || null; +function DefaultPlaceholder({ + width, + height, + showLoadingForImport, +}: PlaceholderProps) { + return ( + // since `width` defaults to 100%, we can display the placeholder once + // height is specified. + (height && ( +
+ {showLoadingForImport && } +
+ )) || + // `|| null` is for in case of height=0. + null + ); } /** @@ -45,10 +60,14 @@ export default function AsyncEsmComponent< /** * Placeholder while still importing. */ - placeholder: React.ComponentType

| null = DefaultPlaceholder, + placeholder: React.ComponentType< + PlaceholderProps + > | null = DefaultPlaceholder, ) { let component: React.ComponentType

; let promise: Promise | undefined; + // component props + placeholder props + type FullProps = P & PlaceholderProps; /** * Safely wait for promise, make sure the loader function only execute once. @@ -59,35 +78,52 @@ export default function AsyncEsmComponent< promise = loadComponent instanceof Promise ? loadComponent : loadComponent(); } - promise.then(result => { - component = ((result as { default?: React.ComponentType

}).default || - result) as React.ComponentType

; - }); + if (!component) { + promise.then(result => { + component = ((result as { default?: React.ComponentType

}).default || + result) as React.ComponentType

; + }); + } return promise; } - function AsyncComponent(props: P, ref: RefObject>) { - const [loaded, setLoaded] = useState(component !== undefined); - useEffect(() => { - let isMounted = true; - if (!loaded) { - // update state to trigger a re-render - waitForPromise().then(() => { - if (isMounted) { - setLoaded(true); - } - }); - } - return () => { - isMounted = false; - }; - }); - const Component = component || placeholder; - return Component ? ( - - ) : null; - } - AsyncComponent.load = waitForPromise; + type AsyncComponent = React.ForwardRefExoticComponent< + React.PropsWithoutRef & + React.RefAttributes> + > & { + preload?: typeof waitForPromise; + }; + + const AsyncComponent: AsyncComponent = React.forwardRef( + function AsyncComponent( + props: FullProps, + ref: RefObject>, + ) { + const [loaded, setLoaded] = useState(component !== undefined); + useEffect(() => { + let isMounted = true; + if (!loaded) { + // update state to trigger a re-render + waitForPromise().then(() => { + if (isMounted) { + setLoaded(true); + } + }); + } + return () => { + isMounted = false; + }; + }); + const Component = component || placeholder; + return Component ? ( + + ) : null; + }, + ); + // preload the async component before rendering + AsyncComponent.preload = waitForPromise; - return React.forwardRef(AsyncComponent); + return AsyncComponent as AsyncComponent & { + preload: typeof waitForPromise; + }; } diff --git a/superset-frontend/src/dashboard/components/Header.jsx b/superset-frontend/src/dashboard/components/Header.jsx index 0561760970568..55f94aed938a3 100644 --- a/superset-frontend/src/dashboard/components/Header.jsx +++ b/superset-frontend/src/dashboard/components/Header.jsx @@ -31,7 +31,6 @@ import { import Icon from 'src/components/Icon'; import Button from 'src/components/Button'; -import AsyncEsmComponent from 'src/components/AsyncEsmComponent'; import EditableTitle from 'src/components/EditableTitle'; import FaveStar from 'src/components/FaveStar'; import { safeStringify } from 'src/utils/safeStringify'; @@ -39,6 +38,7 @@ import { safeStringify } from 'src/utils/safeStringify'; import HeaderActionsDropdown from './HeaderActionsDropdown'; import PublishedStatus from './PublishedStatus'; import UndoRedoKeylisteners from './UndoRedoKeylisteners'; +import PropertiesModal from './PropertiesModal'; import { chartPropShape } from '../util/propShapes'; import { @@ -49,8 +49,6 @@ import { import setPeriodicRunner from '../util/setPeriodicRunner'; import { options as PeriodicRefreshOptions } from './RefreshIntervalModal'; -const PropertiesModal = AsyncEsmComponent(() => import('./PropertiesModal')); - const propTypes = { addSuccessToast: PropTypes.func.isRequired, addDangerToast: PropTypes.func.isRequired, diff --git a/superset-frontend/src/dashboard/components/PropertiesModal.jsx b/superset-frontend/src/dashboard/components/PropertiesModal.jsx index fe4ed2e7eac69..fdf086914157e 100644 --- a/superset-frontend/src/dashboard/components/PropertiesModal.jsx +++ b/superset-frontend/src/dashboard/components/PropertiesModal.jsx @@ -22,16 +22,19 @@ import { Row, Col, Modal, FormControl } from 'react-bootstrap'; import Button from 'src/components/Button'; import Dialog from 'react-bootstrap-dialog'; import { AsyncSelect } from 'src/components/Select'; -import AceEditor from 'react-ace'; import rison from 'rison'; import { t, SupersetClient } from '@superset-ui/core'; import FormLabel from 'src/components/FormLabel'; +import AsyncEsmComponent from 'src/components/AsyncEsmComponent'; + import ColorSchemeControlWrapper from 'src/dashboard/components/ColorSchemeControlWrapper'; import getClientErrorObject from '../../utils/getClientErrorObject'; import withToasts from '../../messageToasts/enhancers/withToasts'; import '../stylesheets/buttons.less'; +const AceEditor = AsyncEsmComponent(() => import('react-ace')); + const propTypes = { dashboardId: PropTypes.number.isRequired, show: PropTypes.bool.isRequired, @@ -79,6 +82,7 @@ class PropertiesModal extends React.PureComponent { componentDidMount() { this.fetchDashboardDetails(); + AceEditor.preload(); } onColorSchemeChange(value) { @@ -305,6 +309,7 @@ class PropertiesModal extends React.PureComponent { {t('JSON Metadata')} import('./DatasourceEditor'), - () => ( -

- -
- ), -); +const DatasourceEditor = AsyncEsmComponent(() => import('./DatasourceEditor')); interface DatasourceModalProps { addSuccessToast: (msg: string) => void; @@ -160,6 +152,7 @@ const DatasourceModal: FunctionComponent = ({ {show && ( import('./AnnotationLayer'), // size of overlay inner content - () =>
, + () =>
, ); const propTypes = { @@ -66,6 +66,11 @@ class AnnotationLayerControl extends React.PureComponent { this.removeAnnotationLayer = this.removeAnnotationLayer.bind(this); } + componentDidMount() { + // preload the AnotationLayer component and dependent libraries i.e. mathjs + AnnotationLayer.preload(); + } + UNSAFE_componentWillReceiveProps(nextProps) { const { name, annotationError, validationErrors, value } = nextProps; if (Object.keys(annotationError).length && !validationErrors.length) { diff --git a/superset-frontend/src/explore/components/controls/index.tsx b/superset-frontend/src/explore/components/controls/index.tsx deleted file mode 100644 index c56e7e80c77f4..0000000000000 --- a/superset-frontend/src/explore/components/controls/index.tsx +++ /dev/null @@ -1,70 +0,0 @@ -/** - * 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 AsyncEsmComponent from 'src/components/AsyncEsmComponent'; -import HiddenControl from './HiddenControl'; -import withVerification from './withVerification'; - -const asyncComponent = (importer: Parameters[0]) => - AsyncEsmComponent(importer, null); - -const controlMap = { - HiddenControl, - AdhocFilterControl: asyncComponent(() => import('./AdhocFilterControl')), - AnnotationLayerControl: asyncComponent( - () => import('./AnnotationLayerControl'), - ), - BoundsControl: asyncComponent(() => import('./BoundsControl')), - CheckboxControl: asyncComponent(() => import('./CheckboxControl')), - CollectionControl: asyncComponent(() => import('./CollectionControl')), - ColorMapControl: asyncComponent(() => import('./ColorMapControl')), - ColorPickerControl: asyncComponent(() => import('./ColorPickerControl')), - ColorSchemeControl: asyncComponent(() => import('./ColorSchemeControl')), - DatasourceControl: asyncComponent(() => import('./DatasourceControl')), - DateFilterControl: asyncComponent(() => import('./DateFilterControl')), - FilterBoxItemControl: asyncComponent(() => import('./FilterBoxItemControl')), - FixedOrMetricControl: asyncComponent(() => import('./FixedOrMetricControl')), - SelectAsyncControl: asyncComponent(() => import('./SelectAsyncControl')), - SelectControl: asyncComponent(() => import('./SelectControl')), - SliderControl: asyncComponent(() => import('./SliderControl')), - SpatialControl: asyncComponent(() => import('./SpatialControl')), - TextAreaControl: asyncComponent(() => import('./TextAreaControl')), - TextControl: asyncComponent(() => import('./TextControl')), - TimeSeriesColumnControl: asyncComponent( - () => import('./TimeSeriesColumnControl'), - ), - ViewportControl: asyncComponent(() => import('./ViewportControl')), - VizTypeControl: asyncComponent(() => import('./VizTypeControl')), - MetricsControl: asyncComponent(() => import('./MetricsControl')), - SelectControlVerifiedOptions: asyncComponent(async () => { - const { default: SelectControl } = await import('./SelectControl'); - return withVerification(SelectControl, 'column_name', 'options'); - }), - MetricsControlVerifiedOptions: asyncComponent(async () => { - const { default: MetricsControl } = await import('./MetricsControl'); - return withVerification(MetricsControl, 'metric_name', 'savedMetrics'); - }), - AdhocFilterControlVerifiedOptions: asyncComponent(async () => { - const { default: AdhocFilterControl } = await import( - './AdhocFilterControl' - ); - return withVerification(AdhocFilterControl, 'column_name', 'columns'); - }), -}; - -export default controlMap; diff --git a/superset-frontend/webpack.config.js b/superset-frontend/webpack.config.js index 0a338bf1efcc8..dd4cff3bae77b 100644 --- a/superset-frontend/webpack.config.js +++ b/superset-frontend/webpack.config.js @@ -202,7 +202,8 @@ const config = { sideEffects: true, splitChunks: { chunks: 'all', - minSize: 20000, + // increase minSize for devMode to 1000kb because of sourcemap + minSize: isDevMode ? 1000000 : 20000, name: nameChunks, automaticNameDelimiter: '-', minChunks: 2, From 6ce91615ee84db201e5ec5802fb7d73be0eb6868 Mon Sep 17 00:00:00 2001 From: Jesse Yang Date: Fri, 11 Sep 2020 13:15:05 -0700 Subject: [PATCH 05/11] Introduce a consolidated AsyncAceEditor component --- .../integration/dashboard/dashboard.helper.ts | 6 +- .../integration/dashboard/markdown.test.ts | 29 ++- .../integration/explore/AdhocFilters.test.ts | 26 ++- .../explore/visualizations/line.test.ts | 5 +- .../gridComponents/Markdown_spec.jsx | 14 +- .../explore/components/TextArea_spec.jsx | 4 +- .../SqlLab/components/AceEditorWrapper.tsx | 33 ++-- .../components/TemplateParamsEditor.jsx | 14 +- .../src/components/AsyncAceEditor.jsx | 49 ------ .../src/components/AsyncAceEditor.tsx | 166 ++++++++++++++++++ .../src/components/AsyncEsmComponent.tsx | 13 +- .../src/dashboard/components/CssEditor.jsx | 16 +- .../dashboard/components/PropertiesModal.jsx | 10 +- .../components/gridComponents/Markdown.jsx | 19 +- .../AdhocFilterEditPopoverSqlTabContent.jsx | 6 +- .../components/AdhocMetricEditPopover.jsx | 13 +- .../components/controls/TextAreaControl.jsx | 24 +-- superset-frontend/src/types/brace.d.ts | 28 +++ superset-frontend/webpack.config.js | 8 +- 19 files changed, 312 insertions(+), 171 deletions(-) delete mode 100644 superset-frontend/src/components/AsyncAceEditor.jsx create mode 100644 superset-frontend/src/components/AsyncAceEditor.tsx create mode 100644 superset-frontend/src/types/brace.d.ts diff --git a/superset-frontend/cypress-base/cypress/integration/dashboard/dashboard.helper.ts b/superset-frontend/cypress-base/cypress/integration/dashboard/dashboard.helper.ts index 810b7f5a25024..10a42b7a194a4 100644 --- a/superset-frontend/cypress-base/cypress/integration/dashboard/dashboard.helper.ts +++ b/superset-frontend/cypress-base/cypress/integration/dashboard/dashboard.helper.ts @@ -22,8 +22,12 @@ export const TABBED_DASHBOARD = '/superset/dashboard/tabbed_dash/'; export const CHECK_DASHBOARD_FAVORITE_ENDPOINT = '/superset/favstar/Dashboard/*/count'; +/** + * Drag an element and drop it to another element. + * Usage: + * drag(source).to(target); + */ export function drag(selector: string, content: string | number | RegExp) { - // drag-n-drop const dataTransfer = { data: {} }; return { to(target: string | Cypress.Chainable) { diff --git a/superset-frontend/cypress-base/cypress/integration/dashboard/markdown.test.ts b/superset-frontend/cypress-base/cypress/integration/dashboard/markdown.test.ts index febd92fe84b16..a5f470fa52dd4 100644 --- a/superset-frontend/cypress-base/cypress/integration/dashboard/markdown.test.ts +++ b/superset-frontend/cypress-base/cypress/integration/dashboard/markdown.test.ts @@ -25,18 +25,39 @@ describe('Dashboard edit markdown', () => { cy.visit(TABBED_DASHBOARD); }); - it('should load brace on demand', () => { - cy.get('script[src*="brace"]').should('have.length', 0); + it('should load AceEditor on demand', () => { + let numScripts = 0; + cy.get('script').then(nodes => { + numScripts = nodes.length; + }); cy.get('.dashboard-header [data-test=pencil]').click(); - cy.get('script[src*="brace"]').should('have.length', 0); + cy.get('script').then(nodes => { + // load 5 new script chunks for css editor + expect(nodes.length).to.greaterThan(numScripts); + numScripts = nodes.length; + }); + + // add new markdown component drag('.new-component', 'Markdown').to( '.grid-row.background--transparent:first', ); - cy.get('script[src*="brace"]').should('have.length', 1); + cy.get('script').then(nodes => { + // load more scripts for markdown editor + expect(nodes.length).to.greaterThan(numScripts); + numScripts = nodes.length; + }); + cy.contains('h3', '✨Markdown').click(); cy.get('.ace_content').contains( 'Click here to edit [markdown](https://bit.ly/1dQOfRK)', ); + + // entering edit mode does not add new scripts + // (though scripts may still be removed by others) + cy.get('script').then(nodes => { + expect(nodes.length).to.most(numScripts); + }); + cy.get('.grid-row.background--transparent:first').click('right'); cy.get('.ace_content').should('not.exist'); }); diff --git a/superset-frontend/cypress-base/cypress/integration/explore/AdhocFilters.test.ts b/superset-frontend/cypress-base/cypress/integration/explore/AdhocFilters.test.ts index c51da29acd849..0745546e5b438 100644 --- a/superset-frontend/cypress-base/cypress/integration/explore/AdhocFilters.test.ts +++ b/superset-frontend/cypress-base/cypress/integration/explore/AdhocFilters.test.ts @@ -25,14 +25,31 @@ describe('AdhocFilters', () => { cy.route('GET', '/superset/filter/table/*/name').as('filterValues'); }); - it('Set simple adhoc filter', () => { - cy.visitChartByName('Num Births Trend'); + it('Should not load mathjs', () => { + cy.visitChartByName('Boys'); cy.verifySliceSuccess({ waitAlias: '@postJson' }); + cy.get('script[src*="mathjs"]').should('have.length', 0); + }); + + let numScripts = 0; + + it('Should load AceEditor scripts on demand', () => { + cy.get('script').then(nodes => { + numScripts = nodes.length; + }); cy.get('[data-test=adhoc_filters]').within(() => { - cy.get('.Select__control').click(); + cy.get('.Select__control').scrollIntoView().click(); cy.get('input[type=text]').focus().type('name{enter}'); }); + + cy.get('script').then(nodes => { + // should load new script chunks for SQL editor + expect(nodes.length).to.greaterThan(numScripts); + }); + }); + + it('Set simple adhoc filter', () => { cy.get('#filter-edit-popover').within(() => { cy.get('[data-test=adhoc-filter-simple-value]').within(() => { cy.get('.Select__control').click(); @@ -40,7 +57,6 @@ describe('AdhocFilters', () => { }); cy.get('button').contains('Save').click(); }); - cy.get('button[data-test="run-query-button"]').click(); cy.verifySliceSuccess({ waitAlias: '@postJson', @@ -53,7 +69,7 @@ describe('AdhocFilters', () => { cy.verifySliceSuccess({ waitAlias: '@postJson' }); cy.get('[data-test=adhoc_filters]').within(() => { - cy.get('.Select__control').click(); + cy.get('.Select__control').scrollIntoView().click(); cy.get('input[type=text]').focus().type('name{enter}'); }); diff --git a/superset-frontend/cypress-base/cypress/integration/explore/visualizations/line.test.ts b/superset-frontend/cypress-base/cypress/integration/explore/visualizations/line.test.ts index e3a95749365c1..9467c71f5a4e0 100644 --- a/superset-frontend/cypress-base/cypress/integration/explore/visualizations/line.test.ts +++ b/superset-frontend/cypress-base/cypress/integration/explore/visualizations/line.test.ts @@ -33,9 +33,10 @@ describe('Visualization > Line', () => { cy.get('.alert-warning').contains(`"Metrics" cannot be empty`); }); - it('should load mathjs on demand', () => { - cy.get('script[src*="mathjs"]').should('have.length', 0); + it('should preload mathjs', () => { + cy.get('script[src*="mathjs"]').should('have.length', 1); cy.contains('Add Annotation Layer').scrollIntoView().click(); + // should not load additional mathjs cy.get('script[src*="mathjs"]').should('have.length', 1); cy.contains('Layer Configuration'); }); diff --git a/superset-frontend/spec/javascripts/dashboard/components/gridComponents/Markdown_spec.jsx b/superset-frontend/spec/javascripts/dashboard/components/gridComponents/Markdown_spec.jsx index e73c48f68372e..ad82b52f82c9a 100644 --- a/superset-frontend/spec/javascripts/dashboard/components/gridComponents/Markdown_spec.jsx +++ b/superset-frontend/spec/javascripts/dashboard/components/gridComponents/Markdown_spec.jsx @@ -20,9 +20,9 @@ import { Provider } from 'react-redux'; import React from 'react'; import { mount } from 'enzyme'; import sinon from 'sinon'; -import AceEditor from 'react-ace'; import ReactMarkdown from 'react-markdown'; +import { MarkdownEditor } from 'src/components/AsyncAceEditor'; import Markdown from 'src/dashboard/components/gridComponents/Markdown'; import MarkdownModeDropdown from 'src/dashboard/components/menu/MarkdownModeDropdown'; import DeleteComponentButton from 'src/dashboard/components/DeleteComponentButton'; @@ -105,23 +105,23 @@ describe('Markdown', () => { it('should render an Markdown when NOT focused', () => { const wrapper = setup(); - expect(wrapper.find(AceEditor)).not.toExist(); + expect(wrapper.find(MarkdownEditor)).not.toExist(); expect(wrapper.find(ReactMarkdown)).toExist(); }); it('should render an AceEditor when focused and editMode=true and editorMode=edit', () => { const wrapper = setup({ editMode: true }); - expect(wrapper.find(AceEditor)).not.toExist(); + expect(wrapper.find(MarkdownEditor)).not.toExist(); expect(wrapper.find(ReactMarkdown)).toExist(); wrapper.find(WithPopoverMenu).simulate('click'); // focus + edit - expect(wrapper.find(AceEditor)).toExist(); + expect(wrapper.find(MarkdownEditor)).toExist(); expect(wrapper.find(ReactMarkdown)).not.toExist(); }); it('should render a ReactMarkdown when focused and editMode=true and editorMode=preview', () => { const wrapper = setup({ editMode: true }); wrapper.find(WithPopoverMenu).simulate('click'); // focus + edit - expect(wrapper.find(AceEditor)).toExist(); + expect(wrapper.find(MarkdownEditor)).toExist(); expect(wrapper.find(ReactMarkdown)).not.toExist(); // we can't call setState on Markdown bc it's not the root component, so call @@ -131,7 +131,7 @@ describe('Markdown', () => { wrapper.update(); expect(wrapper.find(ReactMarkdown)).toExist(); - expect(wrapper.find(AceEditor)).not.toExist(); + expect(wrapper.find(MarkdownEditor)).not.toExist(); }); it('should call updateComponents when editMode changes from edit => preview, and there are markdownSource changes', () => { @@ -148,7 +148,7 @@ describe('Markdown', () => { dropdown.prop('onChange')('edit'); // because we can't call setState on Markdown, change it through the editor // then go back to preview mode to invoke updateComponents - const editor = wrapper.find(AceEditor); + const editor = wrapper.find(MarkdownEditor); editor.prop('onChange')('new markdown!'); dropdown.prop('onChange')('preview'); expect(updateComponents.callCount).toBe(1); diff --git a/superset-frontend/spec/javascripts/explore/components/TextArea_spec.jsx b/superset-frontend/spec/javascripts/explore/components/TextArea_spec.jsx index 4c656b5e1af74..57c34b24559f2 100644 --- a/superset-frontend/spec/javascripts/explore/components/TextArea_spec.jsx +++ b/superset-frontend/spec/javascripts/explore/components/TextArea_spec.jsx @@ -21,7 +21,7 @@ import React from 'react'; import { FormControl } from 'react-bootstrap'; import sinon from 'sinon'; import { shallow } from 'enzyme'; -import AceEditor from 'react-ace'; +import { TextAreaEditor } from 'src/components/AsyncAceEditor'; import TextAreaControl from 'src/explore/components/controls/TextAreaControl'; @@ -52,6 +52,6 @@ describe('SelectControl', () => { props.language = 'markdown'; wrapper = shallow(); expect(wrapper.find(FormControl)).not.toExist(); - expect(wrapper.find(AceEditor)).toExist(); + expect(wrapper.find(TextAreaEditor)).toExist(); }); }); diff --git a/superset-frontend/src/SqlLab/components/AceEditorWrapper.tsx b/superset-frontend/src/SqlLab/components/AceEditorWrapper.tsx index a05ef3cdfae77..09f2d8a871fdc 100644 --- a/superset-frontend/src/SqlLab/components/AceEditorWrapper.tsx +++ b/superset-frontend/src/SqlLab/components/AceEditorWrapper.tsx @@ -17,12 +17,8 @@ * under the License. */ import React from 'react'; -import AceEditor from 'react-ace'; -import 'brace/mode/sql'; -import 'brace/theme/github'; -import 'brace/ext/language_tools'; -import ace from 'brace'; import { areArraysShallowEqual } from 'src/reduxUtils'; +import { supersetColors } from 'src/components/styles'; import sqlKeywords from 'src/SqlLab/utils/sqlKeywords'; import { SCHEMA_AUTOCOMPLETE_SCORE, @@ -30,8 +26,10 @@ import { COLUMN_AUTOCOMPLETE_SCORE, SQL_FUNCTIONS_AUTOCOMPLETE_SCORE, } from 'src/SqlLab/constants'; - -const langTools = ace.acequire('ace/ext/language_tools'); +import { + AceCompleterKeyword, + SQLEditor as AceSQLEditor, +} from 'src/components/AsyncAceEditor'; type HotKey = { key: string; @@ -61,7 +59,7 @@ interface Props { interface State { sql: string; selectedText: string; - words: any[]; + words: AceCompleterKeyword[]; } class AceEditorWrapper extends React.PureComponent { @@ -235,14 +233,7 @@ class AceEditorWrapper extends React.PureComponent { .concat(functionWords) .concat(sqlKeywords); - this.setState({ words }, () => { - const completer = { - getCompletions: this.getCompletions.bind(this), - }; - if (langTools) { - langTools.setCompleters([completer]); - } - }); + this.setState({ words }); } getAceAnnotations() { @@ -262,9 +253,8 @@ class AceEditorWrapper extends React.PureComponent { render() { return ( - { enableLiveAutocompletion={this.props.autocomplete} value={this.state.sql} annotations={this.getAceAnnotations()} + // TODO: migrate this color to supersetTheme + style={{ + background: supersetColors.grayBg, + border: `1px solid ${supersetColors.grayLight}`, + }} /> ); } diff --git a/superset-frontend/src/SqlLab/components/TemplateParamsEditor.jsx b/superset-frontend/src/SqlLab/components/TemplateParamsEditor.jsx index 75b317fe5f743..a15f5638b514d 100644 --- a/superset-frontend/src/SqlLab/components/TemplateParamsEditor.jsx +++ b/superset-frontend/src/SqlLab/components/TemplateParamsEditor.jsx @@ -19,18 +19,12 @@ import React from 'react'; import PropTypes from 'prop-types'; import { Badge } from 'react-bootstrap'; -import AceEditor from 'react-ace'; -import 'brace/mode/sql'; -import 'brace/mode/json'; -import 'brace/mode/html'; -import 'brace/mode/markdown'; -import 'brace/theme/textmate'; - import { t } from '@superset-ui/core'; import { InfoTooltipWithTrigger } from '@superset-ui/chart-controls'; import Button from 'src/components/Button'; -import ModalTrigger from '../../components/ModalTrigger'; +import ModalTrigger from 'src/components/ModalTrigger'; +import { ConfigEditor } from 'src/components/AsyncAceEditor'; const propTypes = { onChange: PropTypes.func, @@ -104,9 +98,9 @@ export default class TemplateParamsEditor extends React.Component { return (
{this.renderDoc()} - { - const { default: ace } = await import('brace'); - const { default: ReactAceEditor } = await import('react-ace'); - - await Promise.all([ - import('brace/mode/sql'), - import('brace/theme/github'), - import('brace/ext/language_tools'), - ]); - - return React.forwardRef(function ExtendedAceEditor( - { keywords, mode, ...props }, - ref, - ) { - if (keywords && mode === 'sql') { - const langTools = ace.acequire('ace/ext/language_tools'); - const completer = { - getCompletions: (aceEditor, session, pos, prefix, callback) => { - callback(null, keywords); - }, - }; - langTools.setCompleters([completer]); - } - return ; - }); -}); - -export default AsyncAceEditor; diff --git a/superset-frontend/src/components/AsyncAceEditor.tsx b/superset-frontend/src/components/AsyncAceEditor.tsx new file mode 100644 index 0000000000000..3feea01b1b91a --- /dev/null +++ b/superset-frontend/src/components/AsyncAceEditor.tsx @@ -0,0 +1,166 @@ +/** + * 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 { + Editor, + IEditSession, + Position, + TextMode as OrigTextMode, +} from 'brace'; +import AceEditor, { AceEditorProps } from 'react-ace'; +import AsyncEsmComponent from 'src/components/AsyncEsmComponent'; + +type TextMode = OrigTextMode & { $id: string }; + +/** + * Async loaders to import brace modules. Must manually create call `import(...)` + * promises because webpack can only analyze asycn imports statically. + */ +const aceModuleLoaders = { + 'mode/sql': () => import('brace/mode/sql'), + 'mode/markdown': () => import('brace/mode/markdown'), + 'mode/css': () => import('brace/mode/css'), + 'mode/json': () => import('brace/mode/json'), + 'mode/yaml': () => import('brace/mode/yaml'), + 'mode/html': () => import('brace/mode/html'), + 'mode/javascript': () => import('brace/mode/javascript'), + 'theme/textmate': () => import('brace/theme/textmate'), + 'theme/github': () => import('brace/theme/github'), + 'ext/language_tools': () => import('brace/ext/language_tools'), +}; +export type AceModule = keyof typeof aceModuleLoaders; + +export interface AceCompleterKeywordData { + name: string; + value: string; + score: number; + meta: string; +} +export interface AceCompleterKeyword extends AceCompleterKeywordData { + completer?: { + insertMatch: (editor: Editor, data: AceCompleterKeywordData) => void; + }; +} + +export type AsyncAceEditorProps = AceEditorProps & { + keywords?: AceCompleterKeyword[]; +}; + +export type AceEditorMode = 'sql'; +export type AceEditorTheme = 'textmate' | 'github'; +export type AsyncAceEditorOptions = { + defaultMode?: AceEditorMode; + defaultTheme?: AceEditorTheme; + defaultTabSize?: number; +}; + +/** + * Get an async AceEditor with automatical loading of specified ace modules. + */ +export default function AsyncAceEditor( + aceModules: AceModule[], + { defaultMode, defaultTheme, defaultTabSize = 2 }: 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]())); + + const inferredMode = + defaultMode || + aceModules.find(x => x.startsWith('mode/'))?.replace('mode/', ''); + const inferredTheme = + defaultTheme || + aceModules.find(x => x.startsWith('theme/'))?.replace('theme/', ''); + + return React.forwardRef( + function ExtendedAceEditor( + { + keywords, + mode = inferredMode, + theme = inferredTheme, + tabSize = defaultTabSize, + ...props + }, + ref, + ) { + if (keywords) { + const langTools = ace.acequire('ace/ext/language_tools'); + const completer = { + getCompletions: ( + editor: AceEditor, + session: IEditSession, + pos: Position, + prefix: string, + callback: (error: null, wordList: object[]) => void, + ) => { + if ((session.getMode() as TextMode).$id === `ace/mode/${mode}`) { + callback(null, keywords); + } + }, + }; + langTools.setCompleters([completer]); + } + return ( + + ); + }, + ); + }); +} + +export const SQLEditor = AsyncAceEditor([ + 'mode/sql', + 'theme/github', + 'ext/language_tools', +]); + +export const MarkdownEditor = AsyncAceEditor([ + 'mode/markdown', + 'theme/textmate', +]); + +export const TextAreaEditor = AsyncAceEditor([ + 'mode/markdown', + 'mode/sql', + 'mode/json', + 'mode/html', + 'mode/javascript', + 'theme/textmate', +]); + +export const CssEditor = AsyncAceEditor(['mode/css', 'theme/github']); + +export const JsonEditor = AsyncAceEditor(['mode/json', 'theme/github']); + +/** + * JSON or Yaml config editor. + */ +export const ConfigEditor = AsyncAceEditor([ + 'mode/json', + 'mode/yaml', + 'theme/github', +]); diff --git a/superset-frontend/src/components/AsyncEsmComponent.tsx b/superset-frontend/src/components/AsyncEsmComponent.tsx index 831be347f1e52..ffef99a4a30c9 100644 --- a/superset-frontend/src/components/AsyncEsmComponent.tsx +++ b/superset-frontend/src/components/AsyncEsmComponent.tsx @@ -16,13 +16,14 @@ * specific language governing permissions and limitations * under the License. */ -import React, { useEffect, useState, RefObject } from 'react'; +import React, { CSSProperties, useEffect, useState, RefObject } from 'react'; import Loading from './Loading'; type PlaceholderProps = { showLoadingForImport: boolean; width?: string | number; height?: string | number; + style?: CSSProperties; } & { [key: string]: any; }; @@ -31,12 +32,13 @@ function DefaultPlaceholder({ width, height, showLoadingForImport, + style, }: PlaceholderProps) { return ( // since `width` defaults to 100%, we can display the placeholder once // height is specified. (height && ( -
+
{showLoadingForImport && }
)) || @@ -64,10 +66,10 @@ export default function AsyncEsmComponent< PlaceholderProps > | null = DefaultPlaceholder, ) { - let component: React.ComponentType

; - let promise: Promise | undefined; // component props + placeholder props type FullProps = P & PlaceholderProps; + let promise: Promise | undefined; + let component: React.ComponentType; /** * Safely wait for promise, make sure the loader function only execute once. @@ -81,7 +83,7 @@ export default function AsyncEsmComponent< if (!component) { promise.then(result => { component = ((result as { default?: React.ComponentType

}).default || - result) as React.ComponentType

; + result) as React.ComponentType; }); } return promise; @@ -116,6 +118,7 @@ export default function AsyncEsmComponent< }); const Component = component || placeholder; return Component ? ( + // placeholder does not get the ref ) : null; }, diff --git a/superset-frontend/src/dashboard/components/CssEditor.jsx b/superset-frontend/src/dashboard/components/CssEditor.jsx index 92112c246568a..69f2500233d09 100644 --- a/superset-frontend/src/dashboard/components/CssEditor.jsx +++ b/superset-frontend/src/dashboard/components/CssEditor.jsx @@ -21,13 +21,7 @@ import PropTypes from 'prop-types'; import Select from 'src/components/Select'; import { t } from '@superset-ui/core'; import ModalTrigger from 'src/components/ModalTrigger'; -import AsyncEsmComponent from 'src/components/AsyncEsmComponent'; - -const AceEditor = AsyncEsmComponent(async () => { - const ace = await import('react-ace'); - await Promise.all([import('brace/mode/css'), import('brace/theme/github')]); - return ace.default; -}); +import { CssEditor as AceCssEditor } from 'src/components/AsyncAceEditor'; const propTypes = { initialCss: PropTypes.string, @@ -52,6 +46,10 @@ class CssEditor extends React.PureComponent { this.changeCssTemplate = this.changeCssTemplate.bind(this); } + componentDidMount() { + AceCssEditor.preload(); + } + changeCss(css) { this.setState({ css }, () => { this.props.onChange(css); @@ -90,9 +88,7 @@ class CssEditor extends React.PureComponent {

{t('Live CSS Editor')}
- import('react-ace')); - const propTypes = { dashboardId: PropTypes.number.isRequired, show: PropTypes.bool.isRequired, @@ -82,7 +80,7 @@ class PropertiesModal extends React.PureComponent { componentDidMount() { this.fetchDashboardDetails(); - AceEditor.preload(); + JsonEditor.preload(); } onColorSchemeChange(value) { @@ -308,14 +306,12 @@ class PropertiesModal extends React.PureComponent { {t('JSON Metadata')} - - this.aceEditorRef.editor.resize(), 0); + setTimeout(() => { + if (this.aceEditorRef) { + this.aceEditorRef.editor.resize(); + } + }, 0); } renderColumnOption(option) { @@ -265,11 +269,10 @@ export default class AdhocMetricEditPopover extends React.Component { > {this.props.datasourceType !== 'druid' ? ( - { - const { default: ReactAceEditor } = await import('react-ace'); - await Promise.all([ - import('brace/mode/sql'), - import('brace/mode/json'), - import('brace/mode/html'), - import('brace/mode/markdown'), - import('brace/mode/javascript'), - import('brace/theme/textmate'), - ]); - return ReactAceEditor; -}); +import ControlHeader from '../ControlHeader'; const propTypes = { name: PropTypes.string, @@ -91,9 +78,8 @@ export default class TextAreaControl extends React.Component { const minLines = inModal ? 40 : this.props.minLines || 12; if (this.props.language) { return ( - Date: Fri, 11 Sep 2020 15:43:57 -0700 Subject: [PATCH 06/11] Try to fix Cypress tests --- .../cypress/integration/explore/AdhocFilters.test.ts | 6 +++--- .../cypress/integration/explore/control.test.ts | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/superset-frontend/cypress-base/cypress/integration/explore/AdhocFilters.test.ts b/superset-frontend/cypress-base/cypress/integration/explore/AdhocFilters.test.ts index 0745546e5b438..a48763acfe01f 100644 --- a/superset-frontend/cypress-base/cypress/integration/explore/AdhocFilters.test.ts +++ b/superset-frontend/cypress-base/cypress/integration/explore/AdhocFilters.test.ts @@ -25,15 +25,15 @@ describe('AdhocFilters', () => { cy.route('GET', '/superset/filter/table/*/name').as('filterValues'); }); - it('Should not load mathjs', () => { - cy.visitChartByName('Boys'); + it('Should not load mathjs when not needed', () => { + cy.visitChartByName('Boys'); // a table chart cy.verifySliceSuccess({ waitAlias: '@postJson' }); cy.get('script[src*="mathjs"]').should('have.length', 0); }); let numScripts = 0; - it('Should load AceEditor scripts on demand', () => { + it('Should load AceEditor scripts when needed', () => { cy.get('script').then(nodes => { numScripts = nodes.length; }); diff --git a/superset-frontend/cypress-base/cypress/integration/explore/control.test.ts b/superset-frontend/cypress-base/cypress/integration/explore/control.test.ts index 7a08611497354..a0ee5361b0045 100644 --- a/superset-frontend/cypress-base/cypress/integration/explore/control.test.ts +++ b/superset-frontend/cypress-base/cypress/integration/explore/control.test.ts @@ -37,7 +37,7 @@ describe('Datasource control', () => { cy.get('#datasource_menu').click(); cy.get('a').contains('Edit Datasource').click(); // create new metric - cy.get('button').contains('Add Item').click(); + cy.get('table button').contains('Add Item', { timeout: 10000 }).click(); cy.get('input[value=""]').click(); cy.get('input[value=""]') .focus() From aed8b1344dceddc1159db0973b7f6c0ba7eafe52 Mon Sep 17 00:00:00 2001 From: Jesse Yang Date: Fri, 11 Sep 2020 15:58:57 -0700 Subject: [PATCH 07/11] Split bootstrap-slider to vendors, too --- superset-frontend/webpack.config.js | 1 + 1 file changed, 1 insertion(+) diff --git a/superset-frontend/webpack.config.js b/superset-frontend/webpack.config.js index 99b2a8b7659c7..b112a8c300af9 100644 --- a/superset-frontend/webpack.config.js +++ b/superset-frontend/webpack.config.js @@ -236,6 +236,7 @@ const config = { 'antd', '@ant-design.*', '.*bootstrap', + 'react-bootstrap-slider', 'moment', 'jquery', 'core-js.*', From 6decb3350a444b24b4f7770e0636bc71c42fab02 Mon Sep 17 00:00:00 2001 From: Jesse Yang Date: Fri, 11 Sep 2020 17:53:40 -0700 Subject: [PATCH 08/11] Create custom placeholder for AceEditor in SQL Lab --- .../SqlLab/components/AceEditorWrapper.tsx | 72 ++++++---------- .../src/components/AsyncAceEditor.tsx | 83 +++++++++++++++---- .../src/components/AsyncEsmComponent.tsx | 14 ++-- 3 files changed, 99 insertions(+), 70 deletions(-) diff --git a/superset-frontend/src/SqlLab/components/AceEditorWrapper.tsx b/superset-frontend/src/SqlLab/components/AceEditorWrapper.tsx index 09f2d8a871fdc..95c6eb8cfcfb1 100644 --- a/superset-frontend/src/SqlLab/components/AceEditorWrapper.tsx +++ b/superset-frontend/src/SqlLab/components/AceEditorWrapper.tsx @@ -18,7 +18,6 @@ */ import React from 'react'; import { areArraysShallowEqual } from 'src/reduxUtils'; -import { supersetColors } from 'src/components/styles'; import sqlKeywords from 'src/SqlLab/utils/sqlKeywords'; import { SCHEMA_AUTOCOMPLETE_SCORE, @@ -27,8 +26,9 @@ import { SQL_FUNCTIONS_AUTOCOMPLETE_SCORE, } from 'src/SqlLab/constants'; import { + Editor, AceCompleterKeyword, - SQLEditor as AceSQLEditor, + FullSQLEditor as AceEditor, } from 'src/components/AsyncAceEditor'; type HotKey = { @@ -149,43 +149,6 @@ class AceEditorWrapper extends React.PureComponent { this.props.onChange(text); } - getCompletions( - aceEditor: any, - session: any, - pos: any, - prefix: string, - callback: (p0: any, p1: any[]) => void, - ) { - // If the prefix starts with a number, don't try to autocomplete with a - // table name or schema or anything else - if (!Number.isNaN(parseInt(prefix, 10))) { - return; - } - const completer = { - insertMatch: (editor: any, data: any) => { - if (data.meta === 'table') { - this.props.actions.addTable( - this.props.queryEditor, - data.value, - this.props.queryEditor.schema, - ); - } - editor.completer.insertMatch({ - value: `${data.caption}${ - ['function', 'schema'].includes(data.meta) ? '' : ' ' - }`, - }); - }, - }; - // Mutate instead of object spread here for performance - const words = this.state.words.map(word => { - /* eslint-disable-next-line no-param-reassign */ - word.completer = completer; - return word; - }); - callback(null, words); - } - setAutoCompleter(props: Props) { // Loading schema, table and column names as auto-completable words const schemas = props.schemas || []; @@ -227,11 +190,33 @@ class AceEditorWrapper extends React.PureComponent { meta: 'function', })); + const completer = { + insertMatch: (editor: Editor, data: any) => { + if (data.meta === 'table') { + this.props.actions.addTable( + this.props.queryEditor, + data.value, + this.props.queryEditor.schema, + ); + } + // executing https://github.com/thlorenz/brace/blob/3a00c5d59777f9d826841178e1eb36694177f5e6/ext/language_tools.js#L1448 + editor.completer.insertMatch( + `${data.caption}${ + ['function', 'schema'].includes(data.meta) ? '' : ' ' + }`, + ); + }, + }; + const words = schemaWords .concat(tableWords) .concat(columnWords) .concat(functionWords) - .concat(sqlKeywords); + .concat(sqlKeywords) + .map(word => ({ + ...word, + completer, + })); this.setState({ words }); } @@ -253,7 +238,7 @@ class AceEditorWrapper extends React.PureComponent { render() { return ( - { enableLiveAutocompletion={this.props.autocomplete} value={this.state.sql} annotations={this.getAceAnnotations()} - // TODO: migrate this color to supersetTheme - style={{ - background: supersetColors.grayBg, - border: `1px solid ${supersetColors.grayLight}`, - }} /> ); } diff --git a/superset-frontend/src/components/AsyncAceEditor.tsx b/superset-frontend/src/components/AsyncAceEditor.tsx index 3feea01b1b91a..0d394d9f3d492 100644 --- a/superset-frontend/src/components/AsyncAceEditor.tsx +++ b/superset-frontend/src/components/AsyncAceEditor.tsx @@ -18,15 +18,40 @@ */ import React from 'react'; import { - Editor, + Editor as OrigEditor, IEditSession, Position, TextMode as OrigTextMode, } from 'brace'; import AceEditor, { AceEditorProps } from 'react-ace'; -import AsyncEsmComponent from 'src/components/AsyncEsmComponent'; +import AsyncEsmComponent, { + PlaceholderProps, +} from 'src/components/AsyncEsmComponent'; -type TextMode = OrigTextMode & { $id: string }; +export interface AceCompleterKeywordData { + name: string; + value: string; + score: number; + meta: string; +} + +export type TextMode = OrigTextMode & { $id: string }; + +export interface AceCompleter { + insertMatch: ( + data?: Editor | { value: string } | string, + options?: AceCompleterKeywordData, + ) => void; +} + +export type Editor = OrigEditor & { + completer: AceCompleter; + completers: AceCompleter[]; +}; + +export interface AceCompleterKeyword extends AceCompleterKeywordData { + completer?: AceCompleter; +} /** * Async loaders to import brace modules. Must manually create call `import(...)` @@ -44,19 +69,8 @@ const aceModuleLoaders = { 'theme/github': () => import('brace/theme/github'), 'ext/language_tools': () => import('brace/ext/language_tools'), }; -export type AceModule = keyof typeof aceModuleLoaders; -export interface AceCompleterKeywordData { - name: string; - value: string; - score: number; - meta: string; -} -export interface AceCompleterKeyword extends AceCompleterKeywordData { - completer?: { - insertMatch: (editor: Editor, data: AceCompleterKeywordData) => void; - }; -} +export type AceModule = keyof typeof aceModuleLoaders; export type AsyncAceEditorProps = AceEditorProps & { keywords?: AceCompleterKeyword[]; @@ -68,6 +82,9 @@ export type AsyncAceEditorOptions = { defaultMode?: AceEditorMode; defaultTheme?: AceEditorTheme; defaultTabSize?: number; + placeholder?: React.ComponentType< + PlaceholderProps & Partial + > | null; }; /** @@ -75,7 +92,12 @@ export type AsyncAceEditorOptions = { */ export default function AsyncAceEditor( aceModules: AceModule[], - { defaultMode, defaultTheme, defaultTabSize = 2 }: AsyncAceEditorOptions = {}, + { + defaultMode, + defaultTheme, + defaultTabSize = 2, + placeholder, + }: AsyncAceEditorOptions = {}, ) { return AsyncEsmComponent(async () => { const { default: ace } = await import('brace'); @@ -111,6 +133,10 @@ export default function AsyncAceEditor( prefix: string, callback: (error: null, wordList: object[]) => void, ) => { + // If the prefix starts with a number, don't try to autocomplete + if (!Number.isNaN(parseInt(prefix, 10))) { + return; + } if ((session.getMode() as TextMode).$id === `ace/mode/${mode}`) { callback(null, keywords); } @@ -129,7 +155,7 @@ export default function AsyncAceEditor( ); }, ); - }); + }, placeholder); } export const SQLEditor = AsyncAceEditor([ @@ -138,6 +164,29 @@ export const SQLEditor = AsyncAceEditor([ 'ext/language_tools', ]); +export const FullSQLEditor = AsyncAceEditor( + ['mode/sql', 'theme/github', 'ext/language_tools'], + { + // a custom placeholder in SQL lab for less jumpy re-renders + placeholder: () => { + const gutterBackground = '#e8e8e8'; // from ace-github theme + return ( +
+
+ {/* make it possible to resize the placeholder */} +
+
+ ); + }, + }, +); + export const MarkdownEditor = AsyncAceEditor([ 'mode/markdown', 'theme/textmate', diff --git a/superset-frontend/src/components/AsyncEsmComponent.tsx b/superset-frontend/src/components/AsyncEsmComponent.tsx index ffef99a4a30c9..a502926f31d22 100644 --- a/superset-frontend/src/components/AsyncEsmComponent.tsx +++ b/superset-frontend/src/components/AsyncEsmComponent.tsx @@ -19,11 +19,11 @@ import React, { CSSProperties, useEffect, useState, RefObject } from 'react'; import Loading from './Loading'; -type PlaceholderProps = { - showLoadingForImport: boolean; +export type PlaceholderProps = { + showLoadingForImport?: boolean; width?: string | number; height?: string | number; - style?: CSSProperties; + placeholderStyle?: CSSProperties; } & { [key: string]: any; }; @@ -31,14 +31,14 @@ type PlaceholderProps = { function DefaultPlaceholder({ width, height, - showLoadingForImport, - style, + showLoadingForImport = false, + placeholderStyle: style, }: PlaceholderProps) { return ( // since `width` defaults to 100%, we can display the placeholder once // height is specified. (height && ( -
+
{showLoadingForImport && }
)) || @@ -63,7 +63,7 @@ export default function AsyncEsmComponent< * Placeholder while still importing. */ placeholder: React.ComponentType< - PlaceholderProps + PlaceholderProps & Partial

> | null = DefaultPlaceholder, ) { // component props + placeholder props From 3c801d40e9715853bd86bd27458847a2425f4a45 Mon Sep 17 00:00:00 2001 From: Jesse Yang Date: Tue, 15 Sep 2020 10:18:33 -0700 Subject: [PATCH 09/11] Add test for vizType control --- .../integration/explore/control.test.ts | 59 ++++++++++++++++--- 1 file changed, 52 insertions(+), 7 deletions(-) diff --git a/superset-frontend/cypress-base/cypress/integration/explore/control.test.ts b/superset-frontend/cypress-base/cypress/integration/explore/control.test.ts index a0ee5361b0045..647b31f7065b9 100644 --- a/superset-frontend/cypress-base/cypress/integration/explore/control.test.ts +++ b/superset-frontend/cypress-base/cypress/integration/explore/control.test.ts @@ -32,10 +32,23 @@ describe('Datasource control', () => { }); it('should allow edit datasource', () => { + let numScripts = 0; + cy.visitChartByName('Num Births Trend'); cy.verifySliceSuccess({ waitAlias: '@postJson' }); cy.get('#datasource_menu').click(); + + cy.get('script').then(nodes => { + numScripts = nodes.length; + }); + cy.get('a').contains('Edit Datasource').click(); + + // should load additional scripts for the modal + cy.get('script').then(nodes => { + expect(nodes.length).to.greaterThan(numScripts); + }); + // create new metric cy.get('table button').contains('Add Item', { timeout: 10000 }).click(); cy.get('input[value=""]').click(); @@ -65,19 +78,33 @@ describe('Datasource control', () => { }); }); -describe('Groupby control', () => { - it('Set groupby', () => { - cy.server(); +describe('VizType control', () => { + beforeEach(() => { cy.login(); + cy.server(); cy.route('GET', '/superset/explore_json/**').as('getJson'); cy.route('POST', '/superset/explore_json/**').as('postJson'); - cy.visitChartByName('Num Births Trend'); + }); + + it('Can change vizType', () => { + cy.visitChartByName('Daily Totals'); cy.verifySliceSuccess({ waitAlias: '@postJson' }); - cy.get('[data-test=groupby]').within(() => { - cy.get('.Select__control').click(); - cy.get('input[type=text]').type('state{enter}'); + let numScripts = 0; + cy.get('script').then(nodes => { + numScripts = nodes.length; }); + + cy.get('.Control .label').contains('Table').click(); + + cy.get('[role="button"]').contains('Line Chart').click(); + + // should load mathjs for line chart + cy.get('script[src*="mathjs"]').should('have.length', 1); + cy.get('script').then(nodes => { + expect(nodes.length).to.greaterThan(numScripts); + }); + cy.get('button[data-test="run-query-button"]').click(); cy.verifySliceSuccess({ waitAlias: '@postJson', chartSelector: 'svg' }); }); @@ -118,3 +145,21 @@ describe('Time range filter', () => { cy.get('#filter-popover').should('not.exist'); }); }); + +describe('Groupby control', () => { + it('Set groupby', () => { + cy.server(); + cy.login(); + cy.route('GET', '/superset/explore_json/**').as('getJson'); + cy.route('POST', '/superset/explore_json/**').as('postJson'); + cy.visitChartByName('Num Births Trend'); + cy.verifySliceSuccess({ waitAlias: '@postJson' }); + + cy.get('[data-test=groupby]').within(() => { + cy.get('.Select__control').click(); + cy.get('input[type=text]').type('state{enter}'); + }); + cy.get('button[data-test="run-query-button"]').click(); + cy.verifySliceSuccess({ waitAlias: '@postJson', chartSelector: 'svg' }); + }); +}); From 0d1a9ae0c2eb930ff2fc0679b4ac93c19735dbbb Mon Sep 17 00:00:00 2001 From: Jesse Yang Date: Tue, 15 Sep 2020 12:12:43 -0700 Subject: [PATCH 10/11] Fix a typo --- superset-frontend/webpack.config.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superset-frontend/webpack.config.js b/superset-frontend/webpack.config.js index b112a8c300af9..081b98fdaa9f8 100644 --- a/superset-frontend/webpack.config.js +++ b/superset-frontend/webpack.config.js @@ -242,7 +242,7 @@ const config = { 'core-js.*', '@emotion.*', 'd3', - 'd3-(array|color|scale||interpolateformat|selection|collection|time|time-format)', + 'd3-(array|color|scale|interpolate|format|selection|collection|time|time-format)', ].join('|')})/`, ), }, From 4477fdd5a1ecdbc4eeaa8ceb8575d729bd2960c8 Mon Sep 17 00:00:00 2001 From: Jesse Yang Date: Tue, 15 Sep 2020 13:41:33 -0700 Subject: [PATCH 11/11] Try to fix AdhocFilters test --- .../integration/explore/AdhocFilters.test.ts | 22 ++++++++++--------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/superset-frontend/cypress-base/cypress/integration/explore/AdhocFilters.test.ts b/superset-frontend/cypress-base/cypress/integration/explore/AdhocFilters.test.ts index a48763acfe01f..d1969f341d862 100644 --- a/superset-frontend/cypress-base/cypress/integration/explore/AdhocFilters.test.ts +++ b/superset-frontend/cypress-base/cypress/integration/explore/AdhocFilters.test.ts @@ -68,19 +68,21 @@ describe('AdhocFilters', () => { cy.visitChartByName('Num Births Trend'); cy.verifySliceSuccess({ waitAlias: '@postJson' }); - cy.get('[data-test=adhoc_filters]').within(() => { - cy.get('.Select__control').scrollIntoView().click(); - cy.get('input[type=text]').focus().type('name{enter}'); - }); + cy.get('[data-test=adhoc_filters] .Select__control') + .scrollIntoView() + .click(); + cy.get('[data-test=adhoc_filters] input[type=text]') + .focus() + .type('name{enter}'); cy.wait('@filterValues'); - cy.get('#filter-edit-popover').within(() => { - cy.get('#adhoc-filter-edit-tabs-tab-SQL').click(); - cy.get('.ace_content').click(); - cy.get('.ace_text-input').type("'Amy' OR name = 'Bob'"); - cy.get('button').contains('Save').click(); - }); + cy.get('#filter-edit-popover #adhoc-filter-edit-tabs-tab-SQL').click(); + cy.get('#filter-edit-popover .ace_content').click(); + cy.get('#filter-edit-popover .ace_text-input').type( + "'Amy' OR name = 'Bob'", + ); + cy.get('#filter-edit-popover button').contains('Save').click(); cy.get('button[data-test="run-query-button"]').click(); cy.verifySliceSuccess({