diff --git a/UPDATING.md b/UPDATING.md index 0573224b7..dc95a9249 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 resulting in the cache being busted. - [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 d6a1f2097..fb9944500 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 131e9731c..447188f4b 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 1fca2f95e..4113e114a 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(, { useRedux: true }); + render(, { 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(, { 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(, { 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(, { 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(, { useRedux: true }); diff --git a/superset-frontend/src/dashboard/components/SliceHeader/index.tsx b/superset-frontend/src/dashboard/components/SliceHeader/index.tsx index af9d509e2..d4b516669 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 = ({ }) => { const dispatch = useDispatch(); const uiConfig = useUiConfig(); - const [headerTooltip, setHeaderTooltip] = useState(null); + const [headerTooltip, setHeaderTooltip] = useState(null); const headerRef = useRef(null); // TODO: change to indicator field after it will be implemented const crossFilterValue = useSelector( @@ -110,11 +118,7 @@ const SliceHeader: FC = ({ 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 e7415f517..ba748fc9c 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; + key: Key; + domEvent: MouseEvent; }) { switch (key) { case MENU_KEYS.FORCE_REFRESH: @@ -306,9 +308,11 @@ class SliceHeaderControls extends React.PureComponent< {this.props.supersetCanExplore && ( this.props.onExploreChart(domEvent)} > - {t('Edit chart')} + + {t('Edit chart')} + )} diff --git a/superset-frontend/src/dashboard/components/gridComponents/Chart.jsx b/superset-frontend/src/dashboard/components/gridComponents/Chart.jsx index fc3f6d39b..0c2abbfb0 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 6ace923c0..a3851a73b 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(); + const wrapper = shallow( + , + ); 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 09cd34738..759ffb81a 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( - - - {' '} - - , - ); + render(, { + 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 a117d9082..72e89075b 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( - - - + + + + + , { 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 86cf613b6..84591e5a8 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( - - - + + + + + , { 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 000000000..a34ad6172 --- /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 ( + <> +
{firstLine}
+
{secondLine}
+ + ); +}; diff --git a/superset/config.py b/superset/config.py index e962cee81..f30ce37f5 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,