From 490bd16bab02a18692d20ef5af622eac7cd8895a Mon Sep 17 00:00:00 2001 From: Kamil Gabryjelski Date: Tue, 29 Mar 2022 14:06:24 +0200 Subject: [PATCH 1/5] feat(dashboard): Implement empty states in empty tabs --- .../src/components/EmptyState/index.tsx | 8 ++ .../DashboardBuilder/DashboardBuilder.tsx | 8 +- .../dashboard/components/DashboardGrid.jsx | 83 +++++++++++++++---- .../components/gridComponents/Tab.jsx | 66 ++++++++++++++- .../dashboard/containers/DashboardGrid.jsx | 6 +- 5 files changed, 147 insertions(+), 24 deletions(-) diff --git a/superset-frontend/src/components/EmptyState/index.tsx b/superset-frontend/src/components/EmptyState/index.tsx index d3e0c5701a1e..1d049fae9220 100644 --- a/superset-frontend/src/components/EmptyState/index.tsx +++ b/superset-frontend/src/components/EmptyState/index.tsx @@ -58,6 +58,14 @@ const EmptyStateContainer = styled.div` & .ant-empty-image svg { width: auto; } + + & a { + color: inherit; + text-decoration: underline; + &:hover { + color: ${theme.colors.grayscale.base}; + } + } `} `; diff --git a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx index 4f7ef563ce55..97649310cc93 100644 --- a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx +++ b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx @@ -148,6 +148,10 @@ const StyledContent = styled.div<{ ${({ fullSizeChartId }) => fullSizeChartId && `z-index: 101;`} `; +const DashboardContentWrapper = styled.div` + height: 100%; +`; + const StyledDashboardContent = styled.div<{ dashboardFiltersOpen: boolean; editMode: boolean; @@ -401,7 +405,7 @@ const DashboardBuilder: FC = () => { image="dashboard.svg" /> )} -
@@ -423,7 +427,7 @@ const DashboardBuilder: FC = () => { /> )} -
+ ); diff --git a/superset-frontend/src/dashboard/components/DashboardGrid.jsx b/superset-frontend/src/dashboard/components/DashboardGrid.jsx index 5a9d2ff81278..4be8d6bc05d0 100644 --- a/superset-frontend/src/dashboard/components/DashboardGrid.jsx +++ b/superset-frontend/src/dashboard/components/DashboardGrid.jsx @@ -24,6 +24,7 @@ import { componentShape } from '../util/propShapes'; import DashboardComponent from '../containers/DashboardComponent'; import DragDroppable from './dnd/DragDroppable'; import { GRID_GUTTER_SIZE, GRID_COLUMN_COUNT } from '../util/constants'; +import { TAB_TYPE } from '../util/componentTypes'; const propTypes = { depth: PropTypes.number.isRequired, @@ -137,9 +138,11 @@ class DashboardGrid extends React.PureComponent { gridComponent, handleComponentDrop, depth, - editMode, width, isComponentVisible, + editMode, + canEdit, + setEditMode, } = this.props; const columnPlusGutterWidth = (width + GRID_GUTTER_SIZE) / GRID_COLUMN_COUNT; @@ -147,26 +150,70 @@ class DashboardGrid extends React.PureComponent { const columnWidth = columnPlusGutterWidth - GRID_GUTTER_SIZE; const { isResizing, rowGuideTop } = this.state; + const shouldDisplayEmptyState = gridComponent?.children?.length === 0; + const shouldDisplayTopLevelTabEmptyState = + shouldDisplayEmptyState && gridComponent.type === TAB_TYPE; + + const dashboardEmptyState = editMode && ( + + + {t('Create a new chart')} + + } + buttonAction={() => { + window.open('/chart/add', '_blank', 'noopener noreferrer'); + }} + image="chart.svg" + /> + ); + + const topLevelTabEmptyState = editMode ? ( + + + {t('Create a new chart')} + + } + buttonAction={() => { + window.open('/chart/add', '_blank', 'noopener noreferrer'); + }} + image="chart.svg" + /> + ) : ( + { + setEditMode(true); + }) + } + image="chart.svg" + /> + ); + return width < 100 ? null : ( <> - {editMode && gridComponent?.children?.length === 0 && ( + {shouldDisplayEmptyState && ( - - - {t('Create a new chart')} - - } - buttonAction={() => { - window.location.assign('/chart/add'); - }} - image="chart.svg" - /> + {shouldDisplayTopLevelTabEmptyState + ? topLevelTabEmptyState + : dashboardEmptyState} )}
diff --git a/superset-frontend/src/dashboard/components/gridComponents/Tab.jsx b/superset-frontend/src/dashboard/components/gridComponents/Tab.jsx index cf051baefbb9..ba052534658f 100644 --- a/superset-frontend/src/dashboard/components/gridComponents/Tab.jsx +++ b/superset-frontend/src/dashboard/components/gridComponents/Tab.jsx @@ -18,8 +18,12 @@ */ import React from 'react'; import PropTypes from 'prop-types'; -import { styled } from '@superset-ui/core'; +import { bindActionCreators } from 'redux'; +import { connect } from 'react-redux'; +import { styled, t } from '@superset-ui/core'; +import { EmptyStateMedium } from 'src/components/EmptyState'; +import { setEditMode } from 'src/dashboard/actions/dashboardState'; import DashboardComponent from '../../containers/DashboardComponent'; import DragDroppable from '../dnd/DragDroppable'; import EditableTitle from '../../../components/EditableTitle'; @@ -85,7 +89,7 @@ const renderDraggableContentTop = dropProps =>
); -export default class Tab extends React.PureComponent { +class Tab extends React.PureComponent { constructor(props) { super(props); this.handleChangeText = this.handleChangeText.bind(this); @@ -143,8 +147,11 @@ export default class Tab extends React.PureComponent { onResizeStop, editMode, isComponentVisible, + canEdit, + setEditMode, } = this.props; + const shouldDisplayEmptyState = tabComponent.children.length === 0; return (
{/* Make top of tab droppable */} @@ -162,6 +169,44 @@ export default class Tab extends React.PureComponent { {renderDraggableContentTop} )} + {shouldDisplayEmptyState && ( + + {t('You can')}{' '} + + {t('create a new chart')} + {' '} + {t( + 'You can create a new chart or use existing ones from the panel on the right', + )} + + ) : ( + t('You can add the components in the edit mode') + )) + } + buttonText={canEdit && !editMode && t('Edit the dashboard')} + buttonAction={ + canEdit && + !editMode && + (() => { + setEditMode(true); + }) + } + image={'chart.svg'} + /> + )} {tabComponent.children.map((componentId, componentIndex) => ( Date: Tue, 29 Mar 2022 14:27:33 +0200 Subject: [PATCH 2/5] Change button to in text link --- .../src/components/EmptyState/index.tsx | 3 ++- .../components/gridComponents/Tab.jsx | 21 +++++++------------ 2 files changed, 10 insertions(+), 14 deletions(-) diff --git a/superset-frontend/src/components/EmptyState/index.tsx b/superset-frontend/src/components/EmptyState/index.tsx index 1d049fae9220..02c1d7c4a295 100644 --- a/superset-frontend/src/components/EmptyState/index.tsx +++ b/superset-frontend/src/components/EmptyState/index.tsx @@ -59,7 +59,8 @@ const EmptyStateContainer = styled.div` width: auto; } - & a { + & a, + & span[role='button'] { color: inherit; text-decoration: underline; &:hover { diff --git a/superset-frontend/src/dashboard/components/gridComponents/Tab.jsx b/superset-frontend/src/dashboard/components/gridComponents/Tab.jsx index ba052534658f..55dea7a67b0b 100644 --- a/superset-frontend/src/dashboard/components/gridComponents/Tab.jsx +++ b/superset-frontend/src/dashboard/components/gridComponents/Tab.jsx @@ -182,28 +182,23 @@ class Tab extends React.PureComponent { {t('You can')}{' '} {t('create a new chart')} {' '} - {t( - 'You can create a new chart or use existing ones from the panel on the right', - )} + {t('or use existing ones from the panel on the right')} ) : ( - t('You can add the components in the edit mode') + + {t('You can add the components in the')}{' '} + setEditMode(true)}> + {t('edit mode')} + + )) } - buttonText={canEdit && !editMode && t('Edit the dashboard')} - buttonAction={ - canEdit && - !editMode && - (() => { - setEditMode(true); - }) - } image={'chart.svg'} /> )} From ca33bebfe54e6950ffaae7c0cde2e60fb5c1f53e Mon Sep 17 00:00:00 2001 From: Kamil Gabryjelski Date: Tue, 29 Mar 2022 14:35:14 +0200 Subject: [PATCH 3/5] Add edit dashboard button to dashboard empty state --- .../components/DashboardBuilder/DashboardBuilder.tsx | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx index 97649310cc93..a19059e9c43e 100644 --- a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx +++ b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx @@ -34,7 +34,10 @@ import getDirectPathToTabIndex from 'src/dashboard/util/getDirectPathToTabIndex' import { URL_PARAMS } from 'src/constants'; import { getUrlParam } from 'src/utils/urlUtils'; import { DashboardLayout, RootState } from 'src/dashboard/types'; -import { setDirectPathToChild } from 'src/dashboard/actions/dashboardState'; +import { + setDirectPathToChild, + setEditMode, +} from 'src/dashboard/actions/dashboardState'; import { useElementOnScreen } from 'src/hooks/useElementOnScreen'; import { FeatureFlag, isFeatureEnabled } from 'src/featureFlags'; import { @@ -402,6 +405,8 @@ const DashboardBuilder: FC = () => { 'Go to the edit mode to configure the dashboard and add charts', ) } + buttonText={canEdit && t('Edit the dashboard')} + buttonAction={() => dispatch(setEditMode(true))} image="dashboard.svg" /> )} From 57205c82f3f1cd40755bb323b6c9d8642f8f5958 Mon Sep 17 00:00:00 2001 From: Kamil Gabryjelski Date: Tue, 29 Mar 2022 16:09:59 +0200 Subject: [PATCH 4/5] Add tests --- .../components/gridComponents/Tab.jsx | 10 ++- .../components/gridComponents/Tab.test.tsx | 77 +++++++++++++++++-- 2 files changed, 78 insertions(+), 9 deletions(-) diff --git a/superset-frontend/src/dashboard/components/gridComponents/Tab.jsx b/superset-frontend/src/dashboard/components/gridComponents/Tab.jsx index 55dea7a67b0b..77156278504d 100644 --- a/superset-frontend/src/dashboard/components/gridComponents/Tab.jsx +++ b/superset-frontend/src/dashboard/components/gridComponents/Tab.jsx @@ -44,6 +44,7 @@ const propTypes = { renderType: PropTypes.oneOf([RENDER_TAB, RENDER_TAB_CONTENT]).isRequired, onDropOnTab: PropTypes.func, editMode: PropTypes.bool.isRequired, + canEdit: PropTypes.bool.isRequired, filters: PropTypes.object.isRequired, // grid related @@ -57,6 +58,7 @@ const propTypes = { handleComponentDrop: PropTypes.func.isRequired, updateComponents: PropTypes.func.isRequired, setDirectPathToChild: PropTypes.func.isRequired, + setEditMode: PropTypes.func.isRequired, }; const defaultProps = { @@ -193,13 +195,17 @@ class Tab extends React.PureComponent { ) : ( {t('You can add the components in the')}{' '} - setEditMode(true)}> + setEditMode(true)} + > {t('edit mode')} )) } - image={'chart.svg'} + image="chart.svg" /> )} {tabComponent.children.map((componentId, componentIndex) => ( diff --git a/superset-frontend/src/dashboard/components/gridComponents/Tab.test.tsx b/superset-frontend/src/dashboard/components/gridComponents/Tab.test.tsx index 1e99b6496cff..82aab1701435 100644 --- a/superset-frontend/src/dashboard/components/gridComponents/Tab.test.tsx +++ b/superset-frontend/src/dashboard/components/gridComponents/Tab.test.tsx @@ -23,6 +23,7 @@ import { render, screen } from 'spec/helpers/testing-library'; import DashboardComponent from 'src/dashboard/containers/DashboardComponent'; import EditableTitle from 'src/components/EditableTitle'; import DragDroppable from 'src/dashboard/components/dnd/DragDroppable'; +import { setEditMode } from 'src/dashboard/actions/dashboardState'; import Tab from './Tab'; @@ -54,8 +55,13 @@ jest.mock('src/dashboard/components/dnd/DragDroppable', () => ); }), ); +jest.mock('src/dashboard/actions/dashboardState', () => ({ + setEditMode: jest.fn(() => ({ + type: 'SET_EDIT_MODE', + })), +})); -const creteProps = () => ({ +const createProps = () => ({ id: 'TAB-YT6eNksV-', parentId: 'TABS-L-d9eyOE-b', depth: 2, @@ -98,7 +104,7 @@ beforeEach(() => { }); test('Render tab (no content)', () => { - const props = creteProps(); + const props = createProps(); props.renderType = 'RENDER_TAB'; render(, { useRedux: true, useDnd: true }); expect(screen.getByText('🚀 Aspiring Developers')).toBeInTheDocument(); @@ -107,7 +113,7 @@ test('Render tab (no content)', () => { }); test('Render tab (no content) editMode:true', () => { - const props = creteProps(); + const props = createProps(); props.editMode = true; props.renderType = 'RENDER_TAB'; render(, { useRedux: true, useDnd: true }); @@ -117,7 +123,7 @@ test('Render tab (no content) editMode:true', () => { }); test('Edit table title', () => { - const props = creteProps(); + const props = createProps(); props.editMode = true; props.renderType = 'RENDER_TAB'; render(, { useRedux: true, useDnd: true }); @@ -131,7 +137,7 @@ test('Edit table title', () => { }); test('Render tab (with content)', () => { - const props = creteProps(); + const props = createProps(); props.isFocused = true; render(, { useRedux: true, useDnd: true }); expect(DashboardComponent).toBeCalledTimes(2); @@ -174,8 +180,39 @@ test('Render tab (with content)', () => { expect(DragDroppable).toBeCalledTimes(0); }); +test('Render tab content with no children', () => { + const props = createProps(); + props.component.children = []; + render(, { + useRedux: true, + useDnd: true, + }); + expect( + screen.getByText('There are no components added to this tab'), + ).toBeVisible(); + expect(screen.getByAltText('empty')).toBeVisible(); + expect(screen.queryByText('edit mode')).not.toBeInTheDocument(); +}); + +test('Render tab content with no children, canEdit: true', () => { + const props = createProps(); + props.component.children = []; + render(, { + useRedux: true, + useDnd: true, + initialState: { + dashboardInfo: { + dash_edit_perm: true, + }, + }, + }); + expect(screen.getByText('edit mode')).toBeVisible(); + userEvent.click(screen.getByRole('button', { name: 'edit mode' })); + expect(setEditMode).toHaveBeenCalled(); +}); + test('Render tab (with content) editMode:true', () => { - const props = creteProps(); + const props = createProps(); props.isFocused = true; props.editMode = true; render(, { useRedux: true, useDnd: true }); @@ -220,7 +257,7 @@ test('Render tab (with content) editMode:true', () => { }); test('Should call "handleDrop" and "handleTopDropTargetDrop"', () => { - const props = creteProps(); + const props = createProps(); props.isFocused = true; props.editMode = true; render(, { useRedux: true, useDnd: true }); @@ -233,3 +270,29 @@ test('Should call "handleDrop" and "handleTopDropTargetDrop"', () => { expect(props.onDropOnTab).toBeCalledTimes(1); expect(props.handleComponentDrop).toBeCalledTimes(2); }); + +test('Render tab content with no children, editMode: true, canEdit: true', () => { + const props = createProps(); + props.editMode = true; + // props.canEdit = true; + props.component.children = []; + render(, { + useRedux: true, + useDnd: true, + initialState: { + dashboardInfo: { + dash_edit_perm: true, + }, + }, + }); + expect( + screen.getByText('Drag and drop components to this tab'), + ).toBeVisible(); + expect(screen.getByAltText('empty')).toBeVisible(); + expect( + screen.getByRole('link', { name: 'create a new chart' }), + ).toBeVisible(); + expect( + screen.getByRole('link', { name: 'create a new chart' }), + ).toHaveAttribute('href', '/chart/add'); +}); From 8357c95b6a6201088808c0c8d7b0d4fdd78467d0 Mon Sep 17 00:00:00 2001 From: Kamil Gabryjelski Date: Thu, 31 Mar 2022 11:15:27 +0200 Subject: [PATCH 5/5] Fix test --- .../components/DashboardBuilder/DashboardBuilder.tsx | 8 ++------ superset-frontend/src/dashboard/stylesheets/builder.less | 1 + 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx index a19059e9c43e..ab1e95b9a1b4 100644 --- a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx +++ b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx @@ -151,10 +151,6 @@ const StyledContent = styled.div<{ ${({ fullSizeChartId }) => fullSizeChartId && `z-index: 101;`} `; -const DashboardContentWrapper = styled.div` - height: 100%; -`; - const StyledDashboardContent = styled.div<{ dashboardFiltersOpen: boolean; editMode: boolean; @@ -410,7 +406,7 @@ const DashboardBuilder: FC = () => { image="dashboard.svg" /> )} - @@ -432,7 +428,7 @@ const DashboardBuilder: FC = () => { /> )} - +
); diff --git a/superset-frontend/src/dashboard/stylesheets/builder.less b/superset-frontend/src/dashboard/stylesheets/builder.less index 1512e4c6fa08..422d455622a4 100644 --- a/superset-frontend/src/dashboard/stylesheets/builder.less +++ b/superset-frontend/src/dashboard/stylesheets/builder.less @@ -22,6 +22,7 @@ flex-grow: 1; display: flex; flex-direction: column; + height: 100%; } /* only top-level tabs have popover, give it more padding to match header + tabs */