Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: Implement global header in Dashboard #20146

Merged
merged 9 commits into from
May 25, 2022
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,13 @@ function openAdvancedProperties() {

function openDashboardEditProperties() {
// open dashboard properties edit modal
cy.get('#save-dash-split-button').trigger('click', { force: true });
cy.get(
'.header-with-actions .right-button-panel .ant-dropdown-trigger',
).trigger('click', {
force: true,
});
cy.get('[data-test=header-actions-menu]')
.contains('Edit dashboard properties')
.contains('Edit properties')
.click({ force: true });
}

Expand All @@ -80,7 +84,7 @@ describe('Dashboard edit action', () => {
cy.get('.dashboard-grid', { timeout: 50000 })
.should('be.visible') // wait for 50 secs to load dashboard
.then(() => {
cy.get('.dashboard-header [aria-label=edit-alt]')
cy.get('[data-test="dashboard-header"] span[aria-label=edit-alt]')
.should('be.visible')
.click();
openDashboardEditProperties();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,11 @@ import {
} from './dashboard.helper';

function openDashboardEditProperties() {
cy.get('.dashboard-header [aria-label=edit-alt]').click();
cy.get('#save-dash-split-button').trigger('click', { force: true });
cy.get('.dropdown-menu').contains('Edit dashboard properties').click();
cy.get('[data-test="dashboard-header"] span[aria-label=edit-alt]').click();
cy.get(
'.header-with-actions .right-button-panel .ant-dropdown-trigger',
).trigger('click', { force: true });
cy.get('.dropdown-menu').contains('Edit properties').click();
}

describe('Dashboard save action', () => {
Expand Down Expand Up @@ -142,7 +144,7 @@ describe('Dashboard save action', () => {
cy.get('.ant-modal-body').should('not.exist');

// save dashboard changes
cy.get('.dashboard-header').contains('Save').click();
cy.get('[data-test="dashboard-header"]').contains('Save').click();

// assert success flash
cy.contains('saved successfully').should('be.visible');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ describe('Download Chart > Distribution bar chart', () => {
};

cy.visitChartByParams(JSON.stringify(formData));
cy.get('.right-button-panel .ant-dropdown-trigger').click();
cy.get('[data-test="dashboard-header"] .ant-dropdown-trigger').click();
cy.get(':nth-child(1) > .ant-dropdown-menu-submenu-title').click();
cy.get(
'.ant-dropdown-menu-submenu > .ant-dropdown-menu li:nth-child(3)',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -630,7 +630,8 @@ export const dashboardView = {
trashIcon: dataTestLocator('dashboard-delete-component-button'),
refreshChart: dataTestLocator('refresh-chart-menu-item'),
},
threeDotsMenuIcon: '#save-dash-split-button',
threeDotsMenuIcon:
'.header-with-actions .right-button-panel .ant-dropdown-trigger',
threeDotsMenuDropdown: dataTestLocator('header-actions-menu'),
refreshDashboard: dataTestLocator('refresh-dashboard-menu-item'),
saveAsMenuOption: dataTestLocator('save-as-menu-item'),
Expand Down
21 changes: 21 additions & 0 deletions superset-frontend/src/assets/images/icons/redo.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
21 changes: 21 additions & 0 deletions superset-frontend/src/assets/images/icons/undo.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,10 @@ export const DynamicEditableTitle = ({
refreshMode: 'debounce',
});

useEffect(() => {
setCurrentTitle(title);
}, [title]);

useEffect(() => {
if (isEditing && contentRef?.current) {
contentRef.current.focus();
Expand Down Expand Up @@ -202,6 +206,7 @@ export const DynamicEditableTitle = ({
className="dynamic-title"
aria-label={label ?? t('Title')}
ref={contentRef}
data-test="editable-title"
>
{currentTitle}
</span>
Expand Down
2 changes: 2 additions & 0 deletions superset-frontend/src/components/Icons/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,8 @@ const IconFileNames = [
'tags',
'ballot',
'category',
'undo',
'redo',
];

const iconOverrides: Record<string, React.FC> = {};
Expand Down
18 changes: 16 additions & 2 deletions superset-frontend/src/components/PageHeaderWithActions/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,21 @@ const headerStyles = (theme: SupersetTheme) => css`
align-items: center;
flex-wrap: nowrap;
justify-content: space-between;
height: 100%;
background-color: ${theme.colors.grayscale.light5};
height: ${theme.gridUnit * 16}px;
padding: 0 ${theme.gridUnit * 4}px;

.editable-title {
overflow: hidden;

& > input[type='button'],
& > span {
overflow: hidden;
text-overflow: ellipsis;
max-width: 100%;
white-space: nowrap;
}
}

span[role='button'] {
display: flex;
Expand Down Expand Up @@ -113,7 +127,7 @@ export const PageHeaderWithActions = ({
}: PageHeaderWithActionsProps) => {
const theme = useTheme();
return (
<div css={headerStyles}>
<div css={headerStyles} className="header-with-actions">
<div className="title-panel">
<DynamicEditableTitle {...editableTitleProps} />
{showTitlePanelItems && (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,11 @@ export default function HeaderReportDropDown({
triggerNode.closest('.action-button')
}
>
<span role="button" className="action-button" tabIndex={0}>
<span
role="button"
className="action-button action-schedule-report"
tabIndex={0}
>
<Icons.Calendar />
</span>
</NoAnimationDropdown>
Expand All @@ -253,7 +257,7 @@ export default function HeaderReportDropDown({
role="button"
title={t('Schedule email report')}
tabIndex={0}
className="action-button"
className="action-button action-schedule-report"
onClick={() => setShowModal(true)}
>
<Icons.Calendar />
Expand Down
35 changes: 23 additions & 12 deletions superset-frontend/src/dashboard/components/Header/Header.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ function setup(props: HeaderProps, initialState = {}) {
async function openActionsDropdown() {
const btn = screen.getByRole('img', { name: 'more-horiz' });
userEvent.click(btn);
expect(await screen.findByRole('menu')).toBeInTheDocument();
expect(await screen.findByTestId('header-actions-menu')).toBeInTheDocument();
}

test('should render', () => {
Expand All @@ -134,7 +134,9 @@ test('should render', () => {
test('should render the title', () => {
const mockedProps = createProps();
setup(mockedProps);
expect(screen.getByText('Dashboard Title')).toBeInTheDocument();
expect(screen.getByTestId('editable-title')).toHaveTextContent(
'Dashboard Title',
);
});

test('should render the editable title', () => {
Expand All @@ -161,21 +163,30 @@ test('should render the "Draft" status', () => {
});

test('should publish', () => {
setup(editableProps);
const mockedProps = createProps();
const canEditProps = {
...mockedProps,
dashboardInfo: {
...mockedProps.dashboardInfo,
dash_edit_perm: true,
dash_save_perm: true,
},
};
setup(canEditProps);
const draft = screen.getByText('Draft');
expect(editableProps.savePublished).not.toHaveBeenCalled();
expect(mockedProps.savePublished).toHaveBeenCalledTimes(0);
userEvent.click(draft);
expect(editableProps.savePublished).toHaveBeenCalledTimes(1);
expect(mockedProps.savePublished).toHaveBeenCalledTimes(1);
});

test('should render the "Undo" action as disabled', () => {
setup(editableProps);
expect(screen.getByTitle('Undo').parentElement).toBeDisabled();
expect(screen.getByTestId('undo-action').parentElement).toBeDisabled();
});

test('should undo', () => {
setup(undoProps);
const undo = screen.getByTitle('Undo');
const undo = screen.getByTestId('undo-action');
expect(undoProps.onUndo).not.toHaveBeenCalled();
userEvent.click(undo);
expect(undoProps.onUndo).toHaveBeenCalledTimes(1);
Expand All @@ -191,12 +202,12 @@ test('should undo with key listener', () => {

test('should render the "Redo" action as disabled', () => {
setup(editableProps);
expect(screen.getByTitle('Redo').parentElement).toBeDisabled();
expect(screen.getByTestId('redo-action').parentElement).toBeDisabled();
});

test('should redo', () => {
setup(redoProps);
const redo = screen.getByTitle('Redo');
const redo = screen.getByTestId('redo-action');
expect(redoProps.onRedo).not.toHaveBeenCalled();
userEvent.click(redo);
expect(redoProps.onRedo).toHaveBeenCalledTimes(1);
Expand All @@ -212,7 +223,7 @@ test('should redo with key listener', () => {

test('should render the "Discard changes" button', () => {
setup(editableProps);
expect(screen.getByText('Discard changes')).toBeInTheDocument();
expect(screen.getByText('Discard')).toBeInTheDocument();
});

test('should render the "Save" button as disabled', () => {
Expand Down Expand Up @@ -297,8 +308,8 @@ test('should toggle the edit mode', () => {
},
};
setup(canEditProps);
const editDashboard = screen.getByTitle('Edit dashboard');
expect(screen.queryByTitle('Edit dashboard')).toBeInTheDocument();
const editDashboard = screen.getByText('Edit dashboard');
expect(screen.queryByText('Edit dashboard')).toBeInTheDocument();
userEvent.click(editDashboard);
expect(mockedProps.logEvent).toHaveBeenCalled();
});
Expand Down
Loading