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

[Dashboard] Makes lens default editor for creating new panels #96181

Merged
merged 16 commits into from
Apr 18, 2021
Merged
Show file tree
Hide file tree
Changes from all 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
@@ -0,0 +1,17 @@
<!-- Do not edit this file. It is automatically generated by API Documenter. -->

[Home](./index.md) &gt; [kibana-plugin-plugins-embeddable-public](./kibana-plugin-plugins-embeddable-public.md) &gt; [EmbeddableFactory](./kibana-plugin-plugins-embeddable-public.embeddablefactory.md) &gt; [getDescription](./kibana-plugin-plugins-embeddable-public.embeddablefactory.getdescription.md)

## EmbeddableFactory.getDescription() method

Returns a description about the embeddable.

<b>Signature:</b>

```typescript
getDescription(): string;
```
<b>Returns:</b>

`string`

Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
<!-- Do not edit this file. It is automatically generated by API Documenter. -->

[Home](./index.md) &gt; [kibana-plugin-plugins-embeddable-public](./kibana-plugin-plugins-embeddable-public.md) &gt; [EmbeddableFactory](./kibana-plugin-plugins-embeddable-public.embeddablefactory.md) &gt; [getIconType](./kibana-plugin-plugins-embeddable-public.embeddablefactory.geticontype.md)

## EmbeddableFactory.getIconType() method

Returns an EUI Icon type to be displayed in a menu.

<b>Signature:</b>

```typescript
getIconType(): string;
```
<b>Returns:</b>

`string`

Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<!-- Do not edit this file. It is automatically generated by API Documenter. -->

[Home](./index.md) &gt; [kibana-plugin-plugins-embeddable-public](./kibana-plugin-plugins-embeddable-public.md) &gt; [EmbeddableFactory](./kibana-plugin-plugins-embeddable-public.embeddablefactory.md) &gt; [grouping](./kibana-plugin-plugins-embeddable-public.embeddablefactory.grouping.md)

## EmbeddableFactory.grouping property

Indicates the grouping this factory should appear in a sub-menu. Example, this is used for grouping options in the editors menu in Dashboard for creating new embeddables

<b>Signature:</b>

```typescript
readonly grouping?: UiActionsPresentableGrouping;
```
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ export interface EmbeddableFactory<TEmbeddableInput extends EmbeddableInput = Em

| Property | Type | Description |
| --- | --- | --- |
| [grouping](./kibana-plugin-plugins-embeddable-public.embeddablefactory.grouping.md) | <code>UiActionsPresentableGrouping</code> | Indicates the grouping this factory should appear in a sub-menu. Example, this is used for grouping options in the editors menu in Dashboard for creating new embeddables |
| [isContainerType](./kibana-plugin-plugins-embeddable-public.embeddablefactory.iscontainertype.md) | <code>boolean</code> | True if is this factory create embeddables that are Containers. Used in the add panel to conditionally show whether these can be added to another container. It's just not supported right now, but once nested containers are officially supported we can probably get rid of this interface. |
| [isEditable](./kibana-plugin-plugins-embeddable-public.embeddablefactory.iseditable.md) | <code>() =&gt; Promise&lt;boolean&gt;</code> | Returns whether the current user should be allowed to edit this type of embeddable. Most of the time this should be based off the capabilities service, hence it's async. |
| [savedObjectMetaData](./kibana-plugin-plugins-embeddable-public.embeddablefactory.savedobjectmetadata.md) | <code>SavedObjectMetaData&lt;TSavedObjectAttributes&gt;</code> | |
Expand All @@ -29,6 +30,8 @@ export interface EmbeddableFactory<TEmbeddableInput extends EmbeddableInput = Em
| [create(initialInput, parent)](./kibana-plugin-plugins-embeddable-public.embeddablefactory.create.md) | Resolves to undefined if a new Embeddable cannot be directly created and the user will instead be redirected elsewhere.<!-- -->This will likely change in future iterations when we improve in place editing capabilities. |
| [createFromSavedObject(savedObjectId, input, parent)](./kibana-plugin-plugins-embeddable-public.embeddablefactory.createfromsavedobject.md) | Creates a new embeddable instance based off the saved object id. |
| [getDefaultInput(partial)](./kibana-plugin-plugins-embeddable-public.embeddablefactory.getdefaultinput.md) | Can be used to get any default input, to be passed in to during the creation process. Default input will not be stored in a parent container, so any inherited input from a container will trump default input parameters. |
| [getDescription()](./kibana-plugin-plugins-embeddable-public.embeddablefactory.getdescription.md) | Returns a description about the embeddable. |
| [getDisplayName()](./kibana-plugin-plugins-embeddable-public.embeddablefactory.getdisplayname.md) | Returns a display name for this type of embeddable. Used in "Create new... " options in the add panel for containers. |
| [getExplicitInput()](./kibana-plugin-plugins-embeddable-public.embeddablefactory.getexplicitinput.md) | Can be used to request explicit input from the user, to be passed in to <code>EmbeddableFactory:create</code>. Explicit input is stored on the parent container for this embeddable. It overrides any inherited input passed down from the parent container. |
| [getIconType()](./kibana-plugin-plugins-embeddable-public.embeddablefactory.geticontype.md) | Returns an EUI Icon type to be displayed in a menu. |

Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,5 @@
<b>Signature:</b>

```typescript
export declare type EmbeddableFactoryDefinition<I extends EmbeddableInput = EmbeddableInput, O extends EmbeddableOutput = EmbeddableOutput, E extends IEmbeddable<I, O> = IEmbeddable<I, O>, T extends SavedObjectAttributes = SavedObjectAttributes> = Pick<EmbeddableFactory<I, O, E, T>, 'create' | 'type' | 'isEditable' | 'getDisplayName'> & Partial<Pick<EmbeddableFactory<I, O, E, T>, 'createFromSavedObject' | 'isContainerType' | 'getExplicitInput' | 'savedObjectMetaData' | 'canCreateNew' | 'getDefaultInput' | 'telemetry' | 'extract' | 'inject' | 'migrations'>>;
export declare type EmbeddableFactoryDefinition<I extends EmbeddableInput = EmbeddableInput, O extends EmbeddableOutput = EmbeddableOutput, E extends IEmbeddable<I, O> = IEmbeddable<I, O>, T extends SavedObjectAttributes = SavedObjectAttributes> = Pick<EmbeddableFactory<I, O, E, T>, 'create' | 'type' | 'isEditable' | 'getDisplayName'> & Partial<Pick<EmbeddableFactory<I, O, E, T>, 'createFromSavedObject' | 'isContainerType' | 'getExplicitInput' | 'savedObjectMetaData' | 'canCreateNew' | 'getDefaultInput' | 'telemetry' | 'extract' | 'inject' | 'migrations' | 'grouping' | 'getIconType' | 'getDescription'>>;
```
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,15 @@ export declare function openAddPanelFlyout(options: {
overlays: OverlayStart;
notifications: NotificationsStart;
SavedObjectFinder: React.ComponentType<any>;
showCreateNewMenu?: boolean;
}): OverlayRef;
```

## Parameters

| Parameter | Type | Description |
| --- | --- | --- |
| options | <code>{</code><br/><code> embeddable: IContainer;</code><br/><code> getFactory: EmbeddableStart['getEmbeddableFactory'];</code><br/><code> getAllFactories: EmbeddableStart['getEmbeddableFactories'];</code><br/><code> overlays: OverlayStart;</code><br/><code> notifications: NotificationsStart;</code><br/><code> SavedObjectFinder: React.ComponentType&lt;any&gt;;</code><br/><code>}</code> | |
| options | <code>{</code><br/><code> embeddable: IContainer;</code><br/><code> getFactory: EmbeddableStart['getEmbeddableFactory'];</code><br/><code> getAllFactories: EmbeddableStart['getEmbeddableFactories'];</code><br/><code> overlays: OverlayStart;</code><br/><code> notifications: NotificationsStart;</code><br/><code> SavedObjectFinder: React.ComponentType&lt;any&gt;;</code><br/><code> showCreateNewMenu?: boolean;</code><br/><code>}</code> | |

<b>Returns:</b>

Expand Down
3 changes: 2 additions & 1 deletion src/plugins/dashboard/kibana.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@
"share",
"uiActions",
"urlForwarding",
"presentationUtil"
"presentationUtil",
"visualizations"
],
"optionalPlugins": [
"home",
Expand Down
15 changes: 14 additions & 1 deletion src/plugins/dashboard/public/application/_dashboard_app.scss
Original file line number Diff line number Diff line change
Expand Up @@ -66,4 +66,17 @@
.dshUnsavedListingItem__actions {
flex-direction: column;
}
}
}

// Temporary fix for two tone icons to make them monochrome
.dshSolutionToolbar__editorContextMenu--dark {
.euiIcon path {
fill: $euiColorGhost;
}
}
Comment on lines +71 to +76
Copy link
Contributor

@cchaos cchaos Apr 20, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cqliu1 Is this something that needs to be fixed in EUI? If so can you point to the issue you were having that made you need to target .euiIcon directly and their SVG paths?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, the dual color app icons don’t use the same color as the text on a button, so I had to manually adjust the icon color with CSS. I’ll file an issue in the EUI repo.


.dshSolutionToolbar__editorContextMenu--light {
.euiIcon path {
fill: $euiColorInk;
}
}
2 changes: 2 additions & 0 deletions src/plugins/dashboard/public/application/dashboard_router.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ export async function mountApp({
embeddable: embeddableStart,
kibanaLegacy: { dashboardConfig },
savedObjectsTaggingOss,
visualizations,
} = pluginsStart;

const spacesApi = pluginsStart.spacesOss?.isSpacesAvailable ? pluginsStart.spacesOss : undefined;
Expand Down Expand Up @@ -123,6 +124,7 @@ export async function mountApp({
visualizeCapabilities: { save: Boolean(coreStart.application.capabilities.visualize?.save) },
storeSearchSession: Boolean(coreStart.application.capabilities.dashboard.storeSearchSession),
},
visualizations,
};

const getUrlStateStorage = (history: RouteComponentProps['history']) =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ export class DashboardContainerFactoryDefinition

public readonly getDisplayName = () => {
return i18n.translate('dashboard.factory.displayName', {
defaultMessage: 'dashboard',
defaultMessage: 'Dashboard',
});
};

Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import { createKbnUrlStateStorage } from '../../services/kibana_utils';
import { savedObjectsPluginMock } from '../../../../saved_objects/public/mocks';
import { DashboardListing, DashboardListingProps } from './dashboard_listing';
import { embeddablePluginMock } from '../../../../embeddable/public/mocks';
import { visualizationsPluginMock } from '../../../../visualizations/public/mocks';
import { DashboardAppServices, DashboardCapabilities } from '../types';
import { dataPluginMock } from '../../../../data/public/mocks';
import { chromeServiceMock, coreMock } from '../../../../../core/public/mocks';
Expand Down Expand Up @@ -76,6 +77,7 @@ function makeDefaultServices(): DashboardAppServices {
dashboardPanelStorage,
savedDashboards,
core,
visualizations: visualizationsPluginMock.createStartContract(),
};
}

Expand Down
127 changes: 85 additions & 42 deletions src/plugins/dashboard/public/application/top_nav/dashboard_top_nav.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,26 +6,24 @@
* Side Public License, v 1.
*/

import { METRIC_TYPE } from '@kbn/analytics';
import { EuiHorizontalRule } from '@elastic/eui';
import { i18n } from '@kbn/i18n';
import angular from 'angular';
import React, { useCallback, useEffect, useMemo, useState } from 'react';

import UseUnmount from 'react-use/lib/useUnmount';
import { BaseVisType, VisTypeAlias } from '../../../../visualizations/public';
import {
AddFromLibraryButton,
PrimaryActionButton,
QuickButtonGroup,
SolutionToolbar,
QuickButtonProps,
} from '../../../../presentation_util/public';
import { useKibana } from '../../services/kibana_react';
import { IndexPattern, SavedQuery, TimefilterContract } from '../../services/data';
import {
EmbeddableFactoryNotFoundError,
EmbeddableInput,
isErrorEmbeddable,
openAddPanelFlyout,
ViewMode,
} from '../../services/embeddable';
import { isErrorEmbeddable, openAddPanelFlyout, ViewMode } from '../../services/embeddable';
import {
getSavedObjectFinder,
SavedObjectSaveOpts,
Expand Down Expand Up @@ -55,6 +53,7 @@ import { DashboardConstants } from '../../dashboard_constants';
import { getNewDashboardTitle, unsavedChangesBadge } from '../../dashboard_strings';
import { DASHBOARD_PANELS_UNSAVED_ID } from '../lib/dashboard_panel_storage';
import { DashboardContainer } from '..';
import { EditorMenu } from './editor_menu';

export interface DashboardTopNavState {
chromeIsVisible: boolean;
Expand Down Expand Up @@ -104,12 +103,22 @@ export function DashboardTopNav({
dashboardCapabilities,
dashboardPanelStorage,
allowByValueEmbeddables,
visualizations,
usageCollection,
} = useKibana<DashboardAppServices>().services;

const [state, setState] = useState<DashboardTopNavState>({ chromeIsVisible: false });
const [isSaveInProgress, setIsSaveInProgress] = useState(false);

const lensAlias = visualizations.getAliases().find(({ name }) => name === 'lens');
const quickButtonVisTypes = ['markdown', 'maps'];
const stateTransferService = embeddable.getStateTransfer();
const IS_DARK_THEME = uiSettings.get('theme:darkMode');

const trackUiMetric = usageCollection?.reportUiCounter.bind(
usageCollection,
DashboardConstants.DASHBOARDS_ID
);

useEffect(() => {
const visibleSubscription = chrome.getIsVisible$().subscribe((chromeIsVisible) => {
Expand Down Expand Up @@ -152,27 +161,36 @@ export function DashboardTopNav({
uiSettings,
]);

const createNew = useCallback(async () => {
const type = 'visualization';
const factory = embeddable.getEmbeddableFactory(type);
const createNewVisType = useCallback(
(visType?: BaseVisType | VisTypeAlias) => () => {
let path = '';
let appId = '';

if (!factory) {
throw new EmbeddableFactoryNotFoundError(type);
}
if (visType) {
if (trackUiMetric) {
trackUiMetric(METRIC_TYPE.CLICK, visType.name);
}

await factory.create({} as EmbeddableInput, dashboardContainer);
}, [dashboardContainer, embeddable]);
if ('aliasPath' in visType) {
appId = visType.aliasApp;
path = visType.aliasPath;
} else {
appId = 'visualize';
path = `#/create?type=${encodeURIComponent(visType.name)}`;
}
} else {
appId = 'visualize';
path = '#/create?';
}

const createNewVisType = useCallback(
(newVisType: string) => async () => {
stateTransferService.navigateToEditor('visualize', {
path: `#/create?type=${encodeURIComponent(newVisType)}`,
stateTransferService.navigateToEditor(appId, {
path,
state: {
originatingApp: DashboardConstants.DASHBOARDS_ID,
},
});
},
[stateTransferService]
[trackUiMetric, stateTransferService]
);

const clearAddPanel = useCallback(() => {
Expand Down Expand Up @@ -563,38 +581,57 @@ export function DashboardTopNav({

const { TopNavMenu } = navigation.ui;

const quickButtons = [
{
iconType: 'visText',
createType: i18n.translate('dashboard.solutionToolbar.markdownQuickButtonLabel', {
defaultMessage: 'Markdown',
}),
onClick: createNewVisType('markdown'),
'data-test-subj': 'dashboardMarkdownQuickButton',
},
{
iconType: 'controlsHorizontal',
createType: i18n.translate('dashboard.solutionToolbar.inputControlsQuickButtonLabel', {
defaultMessage: 'Input control',
}),
onClick: createNewVisType('input_control_vis'),
'data-test-subj': 'dashboardInputControlsQuickButton',
},
];
const getVisTypeQuickButton = (visTypeName: string) => {
const visType =
visualizations.get(visTypeName) ||
visualizations.getAliases().find(({ name }) => name === visTypeName);

if (visType) {
if ('aliasPath' in visType) {
const { name, icon, title } = visType as VisTypeAlias;

return {
iconType: icon,
createType: title,
onClick: createNewVisType(visType as VisTypeAlias),
'data-test-subj': `dashboardQuickButton${name}`,
isDarkModeEnabled: IS_DARK_THEME,
};
} else {
const { name, icon, title, titleInWizard } = visType as BaseVisType;

return {
iconType: icon,
createType: titleInWizard || title,
onClick: createNewVisType(visType as BaseVisType),
'data-test-subj': `dashboardQuickButton${name}`,
isDarkModeEnabled: IS_DARK_THEME,
};
}
}

return;
};

const quickButtons = quickButtonVisTypes
.map(getVisTypeQuickButton)
.filter((button) => button) as QuickButtonProps[];

return (
<>
<TopNavMenu {...getNavBarProps()} />
<EuiHorizontalRule margin="none" />
{viewMode !== ViewMode.VIEW ? (
<SolutionToolbar>
<SolutionToolbar isDarkModeEnabled={IS_DARK_THEME}>
{{
primaryActionButton: (
<PrimaryActionButton
isDarkModeEnabled={IS_DARK_THEME}
label={i18n.translate('dashboard.solutionToolbar.addPanelButtonLabel', {
defaultMessage: 'Create panel',
defaultMessage: 'Create visualization',
})}
onClick={createNew}
iconType="plusInCircleFilled"
onClick={createNewVisType(lensAlias)}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this button take a href?

Copy link
Contributor Author

@cqliu1 cqliu1 Apr 14, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the benefit to adding an href here? Are you suggesting using an href instead of an onClick handler?

I don't think just having an href is sufficient bc we need to use navigateToEditor from the embeddable state transfer service to get the by-value embeddable Save and return workflow.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you are seeing some of the same limitations as we did in Lens, and we haven't been able to find a better way: #96723 (comment)

iconType="lensApp"
data-test-subj="dashboardAddNewPanelButton"
/>
),
Expand All @@ -605,6 +642,12 @@ export function DashboardTopNav({
data-test-subj="dashboardAddPanelButton"
/>
),
extraButtons: [
<EditorMenu
createNewVisType={createNewVisType}
dashboardContainer={dashboardContainer}
/>,
],
}}
</SolutionToolbar>
) : null}
Expand Down
Loading