feat(dashboard): Transition to Explore with React Router

superset-frontend/packages/superset-ui-core/src/utils/featureFlags.ts

@@ -55,6 +55,7 @@ export enum FeatureFlag {
   UX_BETA = 'UX_BETA',
   GENERIC_CHART_AXES = 'GENERIC_CHART_AXES',
   USE_ANALAGOUS_COLORS = 'USE_ANALAGOUS_COLORS',
+  DASHBOARD_EDIT_CHART_IN_TAB = 'DASHBOARD_EDIT_CHART_IN_TAB',
 }
 export type ScheduleQueriesProps = {
   JSONSCHEMA: {

superset-frontend/src/components/EditableTitle/index.tsx

@@ -16,7 +16,7 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import React, { useEffect, useState, useRef } from 'react';
+import React, { MouseEvent, useEffect, useState, useRef } from 'react';
 import cx from 'classnames';
 import { css, styled, t } from '@superset-ui/core';
 import { Tooltip } from 'src/components/Tooltip';
@@ -37,7 +37,7 @@ export interface EditableTitleProps {
   placeholder?: string;
   certifiedBy?: string;
   certificationDetails?: string;
-  onClickTitle?: () => void;
+  onClickTitle?: (event: MouseEvent) => void;
 }
 
 const StyledCertifiedBadge = styled(CertifiedBadge)`

superset-frontend/src/dashboard/components/SliceHeader/SliceHeader.test.tsx

@@ -164,7 +164,7 @@ const createProps = (overrides: any = {}) => ({
 
 test('Should render', () => {
   const props = createProps();
-  render(<SliceHeader {...props} />, { useRedux: true });
+  render(<SliceHeader {...props} />, { useRedux: true, useRouter: true });
   expect(screen.getByTestId('slice-header')).toBeInTheDocument();
 });
 
@@ -279,6 +279,33 @@ test('Should render click to edit prompt and run onExploreChart on click', async
   expect(props.onExploreChart).toHaveBeenCalled();
 });
 
+test('Display correct tooltip when DASHBOARD_EDIT_CHART_IN_TAB is enabled', async () => {
+  window.featureFlags.DASHBOARD_EDIT_CHART_IN_TAB = true;
+  const props = createProps();
+  render(<SliceHeader {...props} />, { useRedux: true });
+  userEvent.hover(screen.getByText('Vaccine Candidates per Phase'));
+  expect(
+    await screen.findByText('Click to edit Vaccine Candidates per Phase.'),
+  ).toBeInTheDocument();
+  expect(
+    await screen.findByText('Use ctrl + click to open in a new tab.'),
+  ).toBeInTheDocument();
+});
+
+test('Display cmd button in tooltip if running on MacOS', async () => {
+  window.featureFlags.DASHBOARD_EDIT_CHART_IN_TAB = true;
+  jest.spyOn(window.navigator, 'appVersion', 'get').mockReturnValue('Mac');
+  const props = createProps();
+  render(<SliceHeader {...props} />, { useRedux: true });
+  userEvent.hover(screen.getByText('Vaccine Candidates per Phase'));
+  expect(
+    await screen.findByText('Click to edit Vaccine Candidates per Phase.'),
+  ).toBeInTheDocument();
+  expect(
+    await screen.findByText('Use ⌘ + click to open in a new tab.'),
+  ).toBeInTheDocument();
+});
+
 test('Should not render click to edit prompt and run onExploreChart on click if supersetCanExplore=false', () => {
   const props = createProps({ supersetCanExplore: false });
   render(<SliceHeader {...props} />, { useRedux: true });

superset-frontend/src/dashboard/components/SliceHeader/index.tsx

@@ -16,8 +16,15 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import React, { FC, useEffect, useMemo, useRef, useState } from 'react';
-import { styled, t } from '@superset-ui/core';
+import React, {
+  FC,
+  ReactNode,
+  useEffect,
+  useMemo,
+  useRef,
+  useState,
+} from 'react';
+import { FeatureFlag, isFeatureEnabled, styled, t } from '@superset-ui/core';
 import { useUiConfig } from 'src/components/UiConfigContext';
 import { Tooltip } from 'src/components/Tooltip';
 import { useDispatch, useSelector } from 'react-redux';
@@ -30,6 +37,7 @@ import Icons from 'src/components/Icons';
 import { RootState } from 'src/dashboard/types';
 import FilterIndicator from 'src/dashboard/components/FiltersBadge/FilterIndicator';
 import { clearDataMask } from 'src/dataMask/actions';
+import { detectOS } from 'src/utils/common';
 
 type SliceHeaderProps = SliceHeaderControlsProps & {
   innerRef?: string;
@@ -54,6 +62,28 @@ const CrossFilterIcon = styled(Icons.CursorTarget)`
   width: 22px;
 `;
 
+export const getSliceHeaderTooltip = (sliceName: string | undefined) => {
+  if (!isFeatureEnabled(FeatureFlag.DASHBOARD_EDIT_CHART_IN_TAB)) {
+    return sliceName
+      ? t('Click to edit %s in a new tab', sliceName)
+      : t('Click to edit chart.');
+  }
+  const isMac = detectOS() === 'MacOS';
+  const firstLine = sliceName
+    ? t('Click to edit %s.', sliceName)
+    : t('Click to edit chart.');
+  const secondLine = t(
+    'Use %s to open in a new tab.',
+    isMac ? '⌘ + click' : 'ctrl + click',
+  );
+  return (
+    <>
+      <div>{firstLine}</div>
+      <div>{secondLine}</div>
+    </>
+  );
+};
+
 const SliceHeader: FC<SliceHeaderProps> = ({
   innerRef = null,
   forceRefresh = () => ({}),
@@ -89,7 +119,7 @@ const SliceHeader: FC<SliceHeaderProps> = ({
 }) => {
   const dispatch = useDispatch();
   const uiConfig = useUiConfig();
-  const [headerTooltip, setHeaderTooltip] = useState<string | null>(null);
+  const [headerTooltip, setHeaderTooltip] = useState<ReactNode | null>(null);
   const headerRef = useRef<HTMLDivElement>(null);
   // TODO: change to indicator field after it will be implemented
   const crossFilterValue = useSelector<RootState, any>(
@@ -110,11 +140,7 @@ const SliceHeader: FC<SliceHeaderProps> = ({
   useEffect(() => {
     const headerElement = headerRef.current;
     if (handleClickTitle) {
-      setHeaderTooltip(
-        sliceName
-          ? t('Click to edit %s in a new tab', sliceName)
-          : t('Click to edit chart in a new tab'),
-      );
+      setHeaderTooltip(getSliceHeaderTooltip(sliceName));
     } else if (
       headerElement &&
       (headerElement.scrollWidth > headerElement.offsetWidth ||

superset-frontend/src/dashboard/components/SliceHeaderControls/index.tsx

@@ -16,7 +16,7 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import React from 'react';
+import React, { MouseEvent, Key } from 'react';
 import moment from 'moment';
 import {
   Behavior,
@@ -106,7 +106,7 @@ export interface SliceHeaderControlsProps {
   isFullSize?: boolean;
   isDescriptionExpanded?: boolean;
   formData: QueryFormData;
-  onExploreChart: () => void;
+  onExploreChart: (event: MouseEvent) => void;
 
   forceRefresh: (sliceId: number, dashboardId: number) => void;
   logExploreChart?: (sliceId: number) => void;
@@ -170,8 +170,8 @@ class SliceHeaderControls extends React.PureComponent<
     key,
     domEvent,
   }: {
-    key: React.Key;
-    domEvent: React.MouseEvent<HTMLElement>;
+    key: Key;
+    domEvent: MouseEvent<HTMLElement>;
   }) {
     switch (key) {
       case MENU_KEYS.FORCE_REFRESH:
@@ -306,7 +306,7 @@ class SliceHeaderControls extends React.PureComponent<
         {this.props.supersetCanExplore && (
           <Menu.Item
             key={MENU_KEYS.EXPLORE_CHART}
-            onClick={this.props.onExploreChart}
+            onClick={({ domEvent }) => this.props.onExploreChart(domEvent)}
           >
             {t('Edit chart')}
           </Menu.Item>

superset-frontend/src/dashboard/components/gridComponents/Chart.jsx

@@ -19,8 +19,15 @@
 import cx from 'classnames';
 import React from 'react';
 import PropTypes from 'prop-types';
-import { styled, t, logging } from '@superset-ui/core';
+import {
+  styled,
+  t,
+  logging,
+  isFeatureEnabled,
+  FeatureFlag,
+} from '@superset-ui/core';
 import { isEqual } from 'lodash';
+import { withRouter } from 'react-router-dom';
 
 import { exportChart, mountExploreUrl } from 'src/explore/exploreUtils';
 import ChartContainer from 'src/components/Chart/ChartContainer';
@@ -120,7 +127,7 @@ const SliceContainer = styled.div`
   max-height: 100%;
 `;
 
-export default class Chart extends React.Component {
+class Chart extends React.Component {
   constructor(props) {
     super(props);
     this.state = {
@@ -270,7 +277,9 @@ export default class Chart extends React.Component {
     });
   };
 
-  onExploreChart = async () => {
+  onExploreChart = async clickEvent => {
+    const isOpenInNewTab =
+      clickEvent.shiftKey || clickEvent.ctrlKey || clickEvent.metaKey;
     try {
       const lastTabId = window.localStorage.getItem('last_tab_id');
       const nextTabId = lastTabId
@@ -287,7 +296,14 @@ export default class Chart extends React.Component {
         [URL_PARAMS.formDataKey.name]: key,
         [URL_PARAMS.sliceId.name]: this.props.slice.slice_id,
       });
-      window.open(url, '_blank', 'noreferrer');
+      if (
+        !isFeatureEnabled(FeatureFlag.DASHBOARD_EDIT_CHART_IN_TAB) ||
+        isOpenInNewTab
+      ) {
+        window.open(url, '_blank', 'noreferrer');
+      } else {
+        this.props.history.push(url);
+      }
     } catch (error) {
       logging.error(error);
       this.props.addDangerToast(t('An error occurred while opening Explore'));
@@ -496,3 +512,5 @@ export default class Chart extends React.Component {
 
 Chart.propTypes = propTypes;
 Chart.defaultProps = defaultProps;
+
+export default withRouter(Chart);

superset-frontend/src/dashboard/components/gridComponents/Chart.test.jsx

@@ -72,7 +72,9 @@ describe('Chart', () => {
   };
 
   function setup(overrideProps) {
-    const wrapper = shallow(<Chart {...props} {...overrideProps} />);
+    const wrapper = shallow(
+      <Chart.WrappedComponent {...props} {...overrideProps} />,
+    );
     return wrapper;
   }
 
@@ -94,7 +96,10 @@ describe('Chart', () => {
   });
 
   it('should calculate the description height if it has one and isExpanded=true', () => {
-    const spy = jest.spyOn(Chart.prototype, 'getDescriptionHeight');
+    const spy = jest.spyOn(
+      Chart.WrappedComponent.prototype,
+      'getDescriptionHeight',
+    );
     const wrapper = setup({ isExpanded: true });
 
     expect(wrapper.find('.slice_description')).toExist();

superset-frontend/src/dashboard/components/gridComponents/ChartHolder.test.tsx

@@ -18,15 +18,12 @@
  */
 
 import React from 'react';
+import mockState from 'spec/fixtures/mockState';
 import { sliceId as chartId } from 'spec/fixtures/mockChartQueries';
+import { screen, render } from 'spec/helpers/testing-library';
 import { nativeFiltersInfo } from 'src/dashboard/fixtures/mockNativeFilters';
 import newComponentFactory from 'src/dashboard/util/newComponentFactory';
-import { getMockStore } from 'spec/fixtures/mockStore';
 import { initialState } from 'src/SqlLab/fixtures';
-import { DndProvider } from 'react-dnd';
-import { HTML5Backend } from 'react-dnd-html5-backend';
-import { Provider } from 'react-redux';
-import { screen, render } from 'spec/helpers/testing-library';
 import { CHART_TYPE, ROW_TYPE } from '../../util/componentTypes';
 import { ChartHolder } from './index';
 
@@ -67,17 +64,14 @@ describe('ChartHolder', () => {
     fullSizeChartId: chartId,
     setFullSizeChartId: () => {},
   };
-  const mockStore = getMockStore({
-    ...initialState,
-  });
+
   const renderWrapper = () =>
-    render(
-      <Provider store={mockStore}>
-        <DndProvider backend={HTML5Backend}>
-          <ChartHolder {...defaultProps} />{' '}
-        </DndProvider>
-      </Provider>,
-    );
+    render(<ChartHolder {...defaultProps} />, {
+      useRouter: true,
+      useDnd: true,
+      useRedux: true,
+      initialState: { ...mockState, ...initialState },
+    });
 
   it('should render empty state', async () => {
     renderWrapper();

superset-frontend/src/dashboard/components/gridComponents/Column.test.jsx

@@ -20,6 +20,7 @@ import { Provider } from 'react-redux';
 import React from 'react';
 import { mount } from 'enzyme';
 import sinon from 'sinon';
+import { MemoryRouter } from 'react-router-dom';
 import { supersetTheme, ThemeProvider } from '@superset-ui/core';
 import { DndProvider } from 'react-dnd';
 import { HTML5Backend } from 'react-dnd-html5-backend';
@@ -71,9 +72,11 @@ describe('Column', () => {
     });
     const wrapper = mount(
       <Provider store={mockStore}>
-        <DndProvider backend={HTML5Backend}>
-          <Column {...props} {...overrideProps} />
-        </DndProvider>
+        <MemoryRouter>
+          <DndProvider backend={HTML5Backend}>
+            <Column {...props} {...overrideProps} />
+          </DndProvider>
+        </MemoryRouter>
       </Provider>,
       {
         wrappingComponent: ThemeProvider,

superset-frontend/src/dashboard/components/gridComponents/Row.test.jsx

@@ -22,6 +22,7 @@ import { mount } from 'enzyme';
 import sinon from 'sinon';
 import { DndProvider } from 'react-dnd';
 import { HTML5Backend } from 'react-dnd-html5-backend';
+import { MemoryRouter } from 'react-router-dom';
 
 import BackgroundStyleDropdown from 'src/dashboard/components/menu/BackgroundStyleDropdown';
 import DashboardComponent from 'src/dashboard/containers/DashboardComponent';
@@ -67,9 +68,11 @@ describe('Row', () => {
     });
     const wrapper = mount(
       <Provider store={mockStore}>
-        <DndProvider backend={HTML5Backend}>
-          <Row {...props} {...overrideProps} />
-        </DndProvider>
+        <MemoryRouter>
+          <DndProvider backend={HTML5Backend}>
+            <Row {...props} {...overrideProps} />
+          </DndProvider>
+        </MemoryRouter>
       </Provider>,
       {
         wrappingComponent: ThemeProvider,

superset/config.py

@@ -429,6 +429,7 @@ def _try_json_readsha(filepath: str, length: int) -> Optional[str]:
     "GENERIC_CHART_AXES": False,
     "ALLOW_ADHOC_SUBQUERY": False,
     "USE_ANALAGOUS_COLORS": True,
+    "DASHBOARD_EDIT_CHART_IN_TAB": True,
     # Apply RLS rules to SQL Lab queries. This requires parsing and manipulating the
     # query, and might break queries and/or allow users to bypass RLS. Use with care!
     "RLS_IN_SQLLAB": False,