Skip to content

Commit

Permalink
[Dashboard] Copy panel refactor (#166991)
Browse files Browse the repository at this point in the history 
Redesigns the copy to dashboard modal and the Dashboard picker element to fix some UX issues they were causing.
Makes panels copied with the `copy panel to` dialog retain their original sizes.
  • Loading branch information
@ThomThomson
ThomThomson authored Sep 26, 2023
1 parent ea8b7c0 commit 01f4d61
Show file tree
Hide file tree
Showing 36 changed files with 483 additions and 368 deletions.
4 changes: 0 additions & 4 deletions src/plugins/dashboard/public/dashboard_actions/_dashboard_actions_strings.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,6 @@ export const dashboardCopyToDashboardActionStrings = {
i18n.translate('dashboard.panel.copyToDashboard.existingDashboardOptionLabel', {
defaultMessage: 'Existing dashboard',
}),
getDescription: () =>
i18n.translate('dashboard.panel.copyToDashboard.description', {
defaultMessage: 'Choose the destination dashboard.',
}),
};

export const dashboardAddToLibraryActionStrings = {
Expand Down
14 changes: 5 additions & 9 deletions src/plugins/dashboard/public/dashboard_actions/copy_to_dashboard_action.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,10 @@

import React from 'react';

import { toMountPoint } from '@kbn/kibana-react-plugin/public';
import { toMountPoint } from '@kbn/react-kibana-mount';
import { CoreStart } from '@kbn/core-lifecycle-browser';
import type { IEmbeddable } from '@kbn/embeddable-plugin/public';
import { Action, IncompatibleActionError } from '@kbn/ui-actions-plugin/public';
import type { PresentationUtilPluginStart } from '@kbn/presentation-util-plugin/public';

import { pluginServices } from '../services/plugin_services';
import { CopyToDashboardModal } from './copy_to_dashboard_modal';
Expand Down Expand Up @@ -39,16 +39,12 @@ export class CopyToDashboardAction implements Action<CopyToDashboardActionContex
public order = 1;

private dashboardCapabilities;
private theme$;
private openModal;

constructor(private PresentationUtilContext: PresentationUtilPluginStart['ContextProvider']) {
constructor(private core: CoreStart) {
({
dashboardCapabilities: this.dashboardCapabilities,
overlays: { openModal: this.openModal },
settings: {
theme: { theme$: this.theme$ },
},
} = pluginServices.getServices());
}

Expand Down Expand Up @@ -81,15 +77,15 @@ export class CopyToDashboardAction implements Action<CopyToDashboardActionContex
throw new IncompatibleActionError();
}

const { theme, i18n } = this.core;
const session = this.openModal(
toMountPoint(
<CopyToDashboardModal
PresentationUtilContext={this.PresentationUtilContext}
closeModal={() => session.close()}
dashboardId={(embeddable.parent as DashboardContainer).getDashboardSavedObjectId()}
embeddable={embeddable}
/>,
{ theme$: this.theme$ }
{ theme, i18n }
),
{
maxWidth: 400,
Expand Down
167 changes: 76 additions & 91 deletions src/plugins/dashboard/public/dashboard_actions/copy_to_dashboard_modal.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,21 +9,21 @@ import React, { useCallback, useState } from 'react';
import { omit } from 'lodash';

import {
EuiText,
EuiRadio,
EuiPanel,
EuiButton,
EuiSpacer,
EuiFormRow,
EuiFocusTrap,
EuiModalBody,
EuiButtonEmpty,
EuiModalFooter,
EuiModalHeader,
EuiModalHeaderTitle,
EuiOutsideClickDetector,
} from '@elastic/eui';
import { IEmbeddable, PanelNotFoundError } from '@kbn/embeddable-plugin/public';
import {
EmbeddablePackageState,
IEmbeddable,
PanelNotFoundError,
} from '@kbn/embeddable-plugin/public';
import { LazyDashboardPicker, withSuspense } from '@kbn/presentation-util-plugin/public';

import { DashboardPanelState } from '../../common';
Expand All @@ -33,7 +33,6 @@ import { dashboardCopyToDashboardActionStrings } from './_dashboard_actions_stri
import { createDashboardEditUrl, CREATE_NEW_DASHBOARD_URL } from '../dashboard_constants';

interface CopyToDashboardModalProps {
PresentationUtilContext: React.FC;
embeddable: IEmbeddable;
dashboardId?: string;
closeModal: () => void;
Expand All @@ -42,7 +41,6 @@ interface CopyToDashboardModalProps {
const DashboardPicker = withSuspense(LazyDashboardPicker);

export function CopyToDashboardModal({
PresentationUtilContext,
dashboardId,
embeddable,
closeModal,
Expand All @@ -65,11 +63,15 @@ export function CopyToDashboardModal({
throw new PanelNotFoundError();
}

const state = {
const state: EmbeddablePackageState = {
type: embeddable.type,
input: {
...omit(panelToCopy.explicitInput, 'id'),
},
size: {
width: panelToCopy.gridData.w,
height: panelToCopy.gridData.h,
},
};

const path =
Expand All @@ -88,90 +90,73 @@ export function CopyToDashboardModal({
const descriptionId = 'copyToDashboardDescription';

return (
<EuiFocusTrap clickOutsideDisables={true}>
<EuiOutsideClickDetector onOutsideClick={closeModal}>
<div
role="dialog"
aria-modal="true"
aria-labelledby={titleId}
aria-describedby={descriptionId}
>
<PresentationUtilContext>
<EuiModalHeader>
<EuiModalHeaderTitle id={titleId} component="h2">
{dashboardCopyToDashboardActionStrings.getDisplayName()}
</EuiModalHeaderTitle>
</EuiModalHeader>
<div role="dialog" aria-modal="true" aria-labelledby={titleId} aria-describedby={descriptionId}>
<EuiModalHeader>
<EuiModalHeaderTitle id={titleId} component="h2">
{dashboardCopyToDashboardActionStrings.getDisplayName()}
</EuiModalHeaderTitle>
</EuiModalHeader>

<EuiModalBody>
<>
<EuiText>
<p id={descriptionId}>{dashboardCopyToDashboardActionStrings.getDescription()}</p>
</EuiText>
<EuiSpacer />
<EuiFormRow hasChildLabel={false}>
<EuiPanel
color="subdued"
hasShadow={false}
data-test-subj="add-to-dashboard-options"
>
<div>
{canEditExisting && (
<>
<EuiRadio
checked={dashboardOption === 'existing'}
data-test-subj="add-to-existing-dashboard-option"
id="existing-dashboard-option"
name="dashboard-option"
label={dashboardCopyToDashboardActionStrings.getExistingDashboardOption()}
onChange={() => setDashboardOption('existing')}
/>
<div className="savAddDashboard__searchDashboards">
<DashboardPicker
isDisabled={dashboardOption !== 'existing'}
idsToOmit={dashboardId ? [dashboardId] : undefined}
onChange={(dashboard) => setSelectedDashboard(dashboard)}
/>
</div>
<EuiSpacer size="s" />
</>
)}
{canCreateNew && (
<>
<EuiRadio
checked={dashboardOption === 'new'}
data-test-subj="add-to-new-dashboard-option"
id="new-dashboard-option"
name="dashboard-option"
disabled={!dashboardId}
label={dashboardCopyToDashboardActionStrings.getNewDashboardOption()}
onChange={() => setDashboardOption('new')}
/>
<EuiSpacer size="s" />
</>
)}
</div>
</EuiPanel>
</EuiFormRow>
</>
</EuiModalBody>
<EuiModalBody>
<>
<EuiFormRow hasChildLabel={false}>
<div data-test-subj="add-to-dashboard-options">
{canEditExisting && (
<>
<EuiRadio
checked={dashboardOption === 'existing'}
data-test-subj="add-to-existing-dashboard-option"
id="existing-dashboard-option"
name="dashboard-option"
label={dashboardCopyToDashboardActionStrings.getExistingDashboardOption()}
onChange={() => setDashboardOption('existing')}
/>
<EuiSpacer size="s" />
<div>
<DashboardPicker
isDisabled={dashboardOption !== 'existing'}
idsToOmit={dashboardId ? [dashboardId] : undefined}
onChange={(dashboard) => {
setSelectedDashboard(dashboard);
setDashboardOption('existing');
}}
/>
</div>
<EuiSpacer size="s" />
</>
)}
{canCreateNew && (
<>
<EuiRadio
checked={dashboardOption === 'new'}
data-test-subj="add-to-new-dashboard-option"
id="new-dashboard-option"
name="dashboard-option"
disabled={!dashboardId}
label={dashboardCopyToDashboardActionStrings.getNewDashboardOption()}
onChange={() => setDashboardOption('new')}
/>
<EuiSpacer size="s" />
</>
)}
</div>
</EuiFormRow>
</>
</EuiModalBody>

<EuiModalFooter>
<EuiButtonEmpty data-test-subj="cancelCopyToButton" onClick={() => closeModal()}>
{dashboardCopyToDashboardActionStrings.getCancelButtonName()}
</EuiButtonEmpty>
<EuiButton
fill
data-test-subj="confirmCopyToButton"
onClick={onSubmit}
disabled={dashboardOption === 'existing' && !selectedDashboard}
>
{dashboardCopyToDashboardActionStrings.getAcceptButtonName()}
</EuiButton>
</EuiModalFooter>
</PresentationUtilContext>
</div>
</EuiOutsideClickDetector>
</EuiFocusTrap>
<EuiModalFooter>
<EuiButtonEmpty data-test-subj="cancelCopyToButton" onClick={() => closeModal()}>
{dashboardCopyToDashboardActionStrings.getCancelButtonName()}
</EuiButtonEmpty>
<EuiButton
fill
data-test-subj="confirmCopyToButton"
onClick={onSubmit}
disabled={dashboardOption === 'existing' && !selectedDashboard}
>
{dashboardCopyToDashboardActionStrings.getAcceptButtonName()}
</EuiButton>
</EuiModalFooter>
</div>
);
}
4 changes: 2 additions & 2 deletions src/plugins/dashboard/public/dashboard_actions/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ export const buildAllDashboardActions = async ({
plugins,
allowByValueEmbeddables,
}: BuildAllDashboardActionsProps) => {
const { uiActions, share, presentationUtil, savedObjectsTaggingOss, contentManagement } = plugins;
const { uiActions, share, savedObjectsTaggingOss, contentManagement } = plugins;

const clonePanelAction = new ClonePanelAction();
uiActions.registerAction(clonePanelAction);
Expand Down Expand Up @@ -73,7 +73,7 @@ export const buildAllDashboardActions = async ({
uiActions.registerAction(libraryNotificationAction);
uiActions.attachAction(PANEL_NOTIFICATION_TRIGGER, libraryNotificationAction.id);

const copyToDashboardAction = new CopyToDashboardAction(presentationUtil.ContextProvider);
const copyToDashboardAction = new CopyToDashboardAction(core);
uiActions.registerAction(copyToDashboardAction);
uiActions.attachAction(CONTEXT_MENU_TRIGGER, copyToDashboardAction.id);
}
Expand Down
71 changes: 70 additions & 1 deletion src/plugins/dashboard/public/dashboard_container/embeddable/create/create_dashboard.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@ test('creates new embeddable with incoming embeddable if id does not match exist
.fn()
.mockReturnValue(mockContactCardFactory);

await createDashboard({
const dashboard = await createDashboard({
getIncomingEmbeddable: () => incomingEmbeddable,
getInitialInput: () => ({
panels: {
Expand All @@ -301,6 +301,75 @@ test('creates new embeddable with incoming embeddable if id does not match exist
}),
expect.any(Object)
);
expect(dashboard!.getState().explicitInput.panels.i_match.explicitInput).toStrictEqual(
expect.objectContaining({
id: 'i_match',
firstName: 'wow look at this new panel wow',
})
);
expect(dashboard!.getState().explicitInput.panels.i_do_not_match.explicitInput).toStrictEqual(
expect.objectContaining({
id: 'i_do_not_match',
firstName: 'phew... I will not be replaced',
})
);

// expect panel to be created with the default size.
expect(dashboard!.getState().explicitInput.panels.i_match.gridData.w).toBe(24);
expect(dashboard!.getState().explicitInput.panels.i_match.gridData.h).toBe(15);
});

test('creates new embeddable with specified size if size is provided', async () => {
const incomingEmbeddable: EmbeddablePackageState = {
type: CONTACT_CARD_EMBEDDABLE,
input: {
id: 'new_panel',
firstName: 'what a tiny lil panel',
} as ContactCardEmbeddableInput,
size: { width: 1, height: 1 },
embeddableId: 'new_panel',
};
const mockContactCardFactory = {
create: jest.fn().mockReturnValue({ destroy: jest.fn() }),
getDefaultInput: jest.fn().mockResolvedValue({}),
};
pluginServices.getServices().embeddable.getEmbeddableFactory = jest
.fn()
.mockReturnValue(mockContactCardFactory);

const dashboard = await createDashboard({
getIncomingEmbeddable: () => incomingEmbeddable,
getInitialInput: () => ({
panels: {
i_do_not_match: getSampleDashboardPanel<ContactCardEmbeddableInput>({
explicitInput: {
id: 'i_do_not_match',
firstName: 'phew... I will not be replaced',
},
type: CONTACT_CARD_EMBEDDABLE,
}),
},
}),
});

// flush promises
await new Promise((r) => setTimeout(r, 1));

expect(mockContactCardFactory.create).toHaveBeenCalledWith(
expect.objectContaining({
id: 'new_panel',
firstName: 'what a tiny lil panel',
}),
expect.any(Object)
);
expect(dashboard!.getState().explicitInput.panels.new_panel.explicitInput).toStrictEqual(
expect.objectContaining({
id: 'new_panel',
firstName: 'what a tiny lil panel',
})
);
expect(dashboard!.getState().explicitInput.panels.new_panel.gridData.w).toBe(1);
expect(dashboard!.getState().explicitInput.panels.new_panel.gridData.h).toBe(1);
});

test('creates a control group from the control group factory and waits for it to be initialized', async () => {
Expand Down
Loading

0 comments on commit 01f4d61

Please sign in to comment.