This is an automated email from the ASF dual-hosted git repository.
kgabryje pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new de4f7db57e feat(dashboard): Transition to Explore with React Router
(#20606)
de4f7db57e is described below
commit de4f7db57ec33c497be9c880fde534a1f026241f
Author: Kamil Gabryjelski <[email protected]>
AuthorDate: Thu Jul 7 11:52:03 2022 +0200
feat(dashboard): Transition to Explore with React Router (#20606)
* feat(dashboard): Use react-router for transition to Explore + cmd click
to open in new tab
* Update tooltip
* Add a feature flag
* Update edit chart onclick event
* Fix lint
* Fix tests
* Change feature flag name
* Add tooltip to Edit chart
---
UPDATING.md | 1 +
.../superset-ui-core/src/utils/featureFlags.ts | 1 +
.../src/components/EditableTitle/index.tsx | 4 +-
.../components/SliceHeader/SliceHeader.test.tsx | 34 +++++++++++++++--
.../src/dashboard/components/SliceHeader/index.tsx | 18 +++++----
.../components/SliceHeaderControls/index.tsx | 16 +++++---
.../dashboard/components/gridComponents/Chart.jsx | 26 +++++++++++--
.../components/gridComponents/Chart.test.jsx | 9 ++++-
.../components/gridComponents/ChartHolder.test.tsx | 24 +++++-------
.../components/gridComponents/Column.test.jsx | 9 +++--
.../components/gridComponents/Row.test.jsx | 9 +++--
.../src/dashboard/util/getSliceHeaderTooltip.tsx | 44 ++++++++++++++++++++++
superset/config.py | 1 +
13 files changed, 150 insertions(+), 46 deletions(-)
diff --git a/UPDATING.md b/UPDATING.md
index 0573224b71..dc95a92496 100644
--- a/UPDATING.md
+++ b/UPDATING.md
@@ -28,6 +28,7 @@ assists people when migrating to a new version.
- [18936](https://github.com/apache/superset/pull/18936): Removes legacy
SIP-15 interim logic/flags—specifically the `SIP_15_ENABLED`,
`SIP_15_GRACE_PERIOD_END`, `SIP_15_DEFAULT_TIME_RANGE_ENDPOINTS`, and
`SIP_15_TOAST_MESSAGE` flags. Time range endpoints are no longer configurable
and strictly adhere to the `[start, end)` paradigm, i.e., inclusive of the
start and exclusive of the end. Additionally this change removes the now
obsolete `time_range_endpoints` from the form-data and result [...]
- [19570](https://github.com/apache/superset/pull/19570): makes
[sqloxide](https://pypi.org/project/sqloxide/) optional so the SIP-68 migration
can be run on aarch64. If the migration is taking too long installing sqloxide
manually should improve the performance.
- [20170](https://github.com/apache/superset/pull/20170): Introduced a new
endpoint for getting datasets samples.
+- [20606](https://github.com/apache/superset/pull/20606): When user clicks on
chart title or "Edit chart" button in Dashboard page, Explore opens in the same
tab. Clicking while holding cmd/ctrl opens Explore in a new tab. To bring back
the old behaviour (always opening Explore in a new tab), flip feature flag
`DASHBOARD_EDIT_CHART_IN_NEW_TAB` to `True`.
### Breaking Changes
diff --git
a/superset-frontend/packages/superset-ui-core/src/utils/featureFlags.ts
b/superset-frontend/packages/superset-ui-core/src/utils/featureFlags.ts
index d6a1f2097f..fb99445004 100644
--- a/superset-frontend/packages/superset-ui-core/src/utils/featureFlags.ts
+++ b/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_NEW_TAB = 'DASHBOARD_EDIT_CHART_IN_NEW_TAB',
}
export type ScheduleQueriesProps = {
JSONSCHEMA: {
diff --git a/superset-frontend/src/components/EditableTitle/index.tsx
b/superset-frontend/src/components/EditableTitle/index.tsx
index 131e9731c0..447188f4b9 100644
--- a/superset-frontend/src/components/EditableTitle/index.tsx
+++ b/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)`
diff --git
a/superset-frontend/src/dashboard/components/SliceHeader/SliceHeader.test.tsx
b/superset-frontend/src/dashboard/components/SliceHeader/SliceHeader.test.tsx
index 1fca2f95e1..4113e114ad 100644
---
a/superset-frontend/src/dashboard/components/SliceHeader/SliceHeader.test.tsx
+++
b/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();
});
@@ -270,15 +270,41 @@ test('Should render click to edit prompt and run
onExploreChart on click', async
render(<SliceHeader {...props} />, { useRedux: true });
userEvent.hover(screen.getByText('Vaccine Candidates per Phase'));
expect(
- await screen.findByText(
- 'Click to edit Vaccine Candidates per Phase in a new tab',
- ),
+ 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();
userEvent.click(screen.getByText('Vaccine Candidates per Phase'));
expect(props.onExploreChart).toHaveBeenCalled();
});
+test('Display cmd button in tooltip if running on MacOS', async () => {
+ 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('Display correct tooltip when DASHBOARD_EDIT_CHART_IN_NEW_TAB is
enabled', async () => {
+ window.featureFlags.DASHBOARD_EDIT_CHART_IN_NEW_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 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 });
diff --git a/superset-frontend/src/dashboard/components/SliceHeader/index.tsx
b/superset-frontend/src/dashboard/components/SliceHeader/index.tsx
index af9d509e2f..d4b5166691 100644
--- a/superset-frontend/src/dashboard/components/SliceHeader/index.tsx
+++ b/superset-frontend/src/dashboard/components/SliceHeader/index.tsx
@@ -16,7 +16,14 @@
* specific language governing permissions and limitations
* under the License.
*/
-import React, { FC, useEffect, useMemo, useRef, useState } from 'react';
+import React, {
+ FC,
+ ReactNode,
+ useEffect,
+ useMemo,
+ useRef,
+ useState,
+} from 'react';
import { styled, t } from '@superset-ui/core';
import { useUiConfig } from 'src/components/UiConfigContext';
import { Tooltip } from 'src/components/Tooltip';
@@ -29,6 +36,7 @@ import FiltersBadge from
'src/dashboard/components/FiltersBadge';
import Icons from 'src/components/Icons';
import { RootState } from 'src/dashboard/types';
import FilterIndicator from
'src/dashboard/components/FiltersBadge/FilterIndicator';
+import { getSliceHeaderTooltip } from
'src/dashboard/util/getSliceHeaderTooltip';
import { clearDataMask } from 'src/dataMask/actions';
type SliceHeaderProps = SliceHeaderControlsProps & {
@@ -89,7 +97,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 +118,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 ||
diff --git
a/superset-frontend/src/dashboard/components/SliceHeaderControls/index.tsx
b/superset-frontend/src/dashboard/components/SliceHeaderControls/index.tsx
index e7415f517c..ba748fc9cb 100644
--- a/superset-frontend/src/dashboard/components/SliceHeaderControls/index.tsx
+++ b/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,
@@ -32,6 +32,8 @@ import ShareMenuItems from
'src/dashboard/components/menu/ShareMenuItems';
import downloadAsImage from 'src/utils/downloadAsImage';
import { FeatureFlag, isFeatureEnabled } from 'src/featureFlags';
import CrossFilterScopingModal from
'src/dashboard/components/CrossFilterScopingModal/CrossFilterScopingModal';
+import { getSliceHeaderTooltip } from
'src/dashboard/util/getSliceHeaderTooltip';
+import { Tooltip } from 'src/components/Tooltip';
import Icons from 'src/components/Icons';
import ModalTrigger from 'src/components/ModalTrigger';
import Button from 'src/components/Button';
@@ -106,7 +108,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 +172,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,9 +308,11 @@ 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')}
+ <Tooltip
title={getSliceHeaderTooltip(this.props.slice.slice_name)}>
+ {t('Edit chart')}
+ </Tooltip>
</Menu.Item>
)}
diff --git
a/superset-frontend/src/dashboard/components/gridComponents/Chart.jsx
b/superset-frontend/src/dashboard/components/gridComponents/Chart.jsx
index fc3f6d39b3..0c2abbfb0c 100644
--- a/superset-frontend/src/dashboard/components/gridComponents/Chart.jsx
+++ b/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_NEW_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);
diff --git
a/superset-frontend/src/dashboard/components/gridComponents/Chart.test.jsx
b/superset-frontend/src/dashboard/components/gridComponents/Chart.test.jsx
index 6ace923c0f..a3851a73b3 100644
--- a/superset-frontend/src/dashboard/components/gridComponents/Chart.test.jsx
+++ b/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();
diff --git
a/superset-frontend/src/dashboard/components/gridComponents/ChartHolder.test.tsx
b/superset-frontend/src/dashboard/components/gridComponents/ChartHolder.test.tsx
index 09cd34738e..759ffb81a9 100644
---
a/superset-frontend/src/dashboard/components/gridComponents/ChartHolder.test.tsx
+++
b/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();
diff --git
a/superset-frontend/src/dashboard/components/gridComponents/Column.test.jsx
b/superset-frontend/src/dashboard/components/gridComponents/Column.test.jsx
index a117d90820..72e89075b5 100644
--- a/superset-frontend/src/dashboard/components/gridComponents/Column.test.jsx
+++ b/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,
diff --git
a/superset-frontend/src/dashboard/components/gridComponents/Row.test.jsx
b/superset-frontend/src/dashboard/components/gridComponents/Row.test.jsx
index 86cf613b6f..84591e5a88 100644
--- a/superset-frontend/src/dashboard/components/gridComponents/Row.test.jsx
+++ b/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,
diff --git a/superset-frontend/src/dashboard/util/getSliceHeaderTooltip.tsx
b/superset-frontend/src/dashboard/util/getSliceHeaderTooltip.tsx
new file mode 100644
index 0000000000..a34ad61722
--- /dev/null
+++ b/superset-frontend/src/dashboard/util/getSliceHeaderTooltip.tsx
@@ -0,0 +1,44 @@
+/**
+ * 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 { FeatureFlag, isFeatureEnabled, t } from '@superset-ui/core';
+import { detectOS } from 'src/utils/common';
+
+export const getSliceHeaderTooltip = (sliceName: string | undefined) => {
+ if (isFeatureEnabled(FeatureFlag.DASHBOARD_EDIT_CHART_IN_NEW_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>
+ </>
+ );
+};
diff --git a/superset/config.py b/superset/config.py
index e962cee813..f30ce37f57 100644
--- a/superset/config.py
+++ b/superset/config.py
@@ -429,6 +429,7 @@ DEFAULT_FEATURE_FLAGS: Dict[str, bool] = {
"GENERIC_CHART_AXES": False,
"ALLOW_ADHOC_SUBQUERY": False,
"USE_ANALAGOUS_COLORS": True,
+ "DASHBOARD_EDIT_CHART_IN_NEW_TAB": False,
# 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,
