Skip to content

Commit

Permalink
[Dashboard Navigation] Add Links to Visualization library (elastic#17…
Browse files Browse the repository at this point in the history
…0810)

Closes elastic#162840

## Summary

This PR adds a visualization alias for the new Links embeddable so that
all Links library items can be managed/edited from the Visualization
library, like so:


https://github.com/elastic/kibana/assets/8698078/8541506b-cfdd-4a2f-8bc2-841220def7a3

However, in order to get the above working, it was unfortunately not as
simple as just adding a visualization alias. Because the Links
embeddable does not have a dedicated editing app (all editing/creation
is done through a flyout), the usual `aliasPath` + `aliasApp` redirect
that happens for editing an alias did not work in this case.

To get around this, I've had to make changes to how aliases are
registered, as well as both the Visualization `VisualizeListing`
component and the generic `TableListViewTableComp` content management
component:

- **Summary of visualization alias changes:**

First off, I made changes to the typing of aliases - specifically,
rather than taking two independent `aliasPath` and `aliasApp` props,
I've combined them into a singular `alias` prop which will either be of
type `{ alias: string; path: string; }` or `{ embeddableType: string;
}`. This makes it easier to determine (a) whether a visualization is of
type `BaseVisType` or `VisTypeAlias` and (b) if it **is** of type
`VisTypeAlias`, how the editing of that vis should be handled.
Specifically, if `alias` is of type `{ alias: string; path: string; }`,
then it is a normal visualization and behaviour should be the same as it
was previously; however, if it is of type `{ embeddableType: string; }`,
then this is an **inline** alias - i.e. editing should be done inline
via the embeddable factory's edit method.

- **Summary of `VisualizeListing`  changes**

The primary changes here were made to the `editItem` callback -
specifically, if the fetched saved object has neither an `editApp` nor
an `editUrl`, then it will now try to fetch the embeddable factory for
the given saved object type and, if this factory exists, it will call
the `getExplicitInput` method in order to handle editing.

- **Summary of `TableListViewTableComp` changes**

Previously, an error would be thrown if both a `getDetailViewLink` and
an `onClickTitle` prop were provided - while I understand the original
reasoning for adding this catch, this no longer works if we want to
support inline editing like this. In this case, I needed **only** the
Link embeddable items to have an `onClick` while keeping the behaviour
for other visualizations the same (i.e. all other visualization types
should continue to link off to their specific editor apps) - however,
since this method is not provided **per item**, I had no way of making
an exception for just one specific item type.

Therefore, to get around this, it is now considered to be valid for
**both** the `getDetailViewLink` and `onClickTitle` props to be defined
for the `TableListViewTableComp` component. In order to prevent conflict
between the two props, I have made it so that, if both are provided,
`getDetailViewLink` **always takes precedence** over `onClickTitle` - in
this case, `onClickTitle` will **only** be called if `getDetailViewLink`
returns `undefined` for a given item. I have added a comment to
hopefully make this clear for consumers.

### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [x] Any UI touched in this PR is usable by keyboard only (learn more
about [keyboard accessibility](https://webaim.org/techniques/keyboard/))
- [x] Any UI touched in this PR does not create any new axe failures
(run axe in browser:
[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),
[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))
- [x] This renders correctly on smaller devices using a responsive
layout. (You can test this [in your
browser](https://www.browserstack.com/guide/responsive-testing-on-local-server))
- [x] This was checked for [cross-browser
compatibility](https://www.elastic.co/support/matrix#matrix_browsers)

### For maintainers

- [ ] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)
  • Loading branch information
Heenawter authored Nov 22, 2023
1 parent 74dea1e commit 8eaebb6
Show file tree
Hide file tree
Showing 25 changed files with 227 additions and 104 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -57,15 +57,15 @@ export function ItemDetails<T extends UserContentCommonSchema>({
);

const onClickTitleHandler = useMemo(() => {
if (!onClickTitle) {
if (!onClickTitle || getDetailViewLink?.(item)) {
return undefined;
}

return ((e) => {
e.preventDefault();
onClickTitle(item);
}) as React.MouseEventHandler<HTMLAnchorElement>;
}, [item, onClickTitle]);
}, [item, onClickTitle, getDetailViewLink]);

const renderTitle = useCallback(() => {
const href = getDetailViewLink ? getDetailViewLink(item) : undefined;
Expand All @@ -79,7 +79,7 @@ export function ItemDetails<T extends UserContentCommonSchema>({
<RedirectAppLinks coreStart={redirectAppLinksCoreStart}>
{/* eslint-disable-next-line @elastic/eui/href-or-on-click */}
<EuiLink
href={getDetailViewLink ? getDetailViewLink(item) : undefined}
href={getDetailViewLink?.(item)}
onClick={onClickTitleHandler}
data-test-subj={`${id}ListingTitleLink-${item.attributes.title.split(' ').join('-')}`}
>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -289,12 +289,6 @@ function TableListViewTableComp<T extends UserContentCommonSchema>({
);
}

if (getDetailViewLink && onClickTitle) {
throw new Error(
`[TableListView] Either "getDetailViewLink" or "onClickTitle" can be provided. Not both.`
);
}

if (contentEditor.isReadonly === false && contentEditor.onSave === undefined) {
throw new Error(
`[TableListView] A value for [contentEditor.onSave()] must be provided when [contentEditor.isReadonly] is false.`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,12 +54,14 @@ export function DashboardEditingToolbar({ isDisabled }: { isDisabled?: boolean }
trackUiMetric(METRIC_TYPE.CLICK, `${visType.name}:create`);
}

if ('aliasPath' in visType) {
appId = visType.aliasApp;
path = visType.aliasPath;
} else {
if (!('alias' in visType)) {
// this visualization is not an alias
appId = 'visualize';
path = `#/create?type=${encodeURIComponent(visType.name)}`;
} else if (visType.alias && 'path' in visType.alias) {
// this visualization **is** an alias, and it has an app to redirect to for creation
appId = visType.alias.app;
path = visType.alias.path;
}
} else {
appId = 'visualize';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,10 +104,11 @@ export const EditorMenu = ({ createNewVisType, createNewEmbeddable, isDisabled }
const promotedVisTypes = getSortedVisTypesByGroup(VisGroups.PROMOTED);
const aggsBasedVisTypes = getSortedVisTypesByGroup(VisGroups.AGGBASED);
const toolVisTypes = getSortedVisTypesByGroup(VisGroups.TOOLS);
const visTypeAliases = getVisTypeAliases().sort(
({ promotion: a = false }: VisTypeAlias, { promotion: b = false }: VisTypeAlias) =>
const visTypeAliases = getVisTypeAliases()
.sort(({ promotion: a = false }: VisTypeAlias, { promotion: b = false }: VisTypeAlias) =>
a === b ? 0 : a ? -1 : 1
);
)
.filter(({ disableCreate }: VisTypeAlias) => !disableCreate);

const factories = unwrappedEmbeddableFactories.filter(
({ isEditable, factory: { type, canCreateNew, isContainerType } }) =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,21 +10,21 @@ import React, { useCallback, useMemo } from 'react';
import useObservable from 'react-use/lib/useObservable';

import {
EuiText,
EuiImage,
EuiButton,
EuiFlexItem,
EuiFlexGroup,
EuiButtonEmpty,
EuiFlexGroup,
EuiFlexItem,
EuiImage,
EuiPageTemplate,
EuiText,
} from '@elastic/eui';
import { METRIC_TYPE } from '@kbn/analytics';
import { ViewMode } from '@kbn/embeddable-plugin/public';

import { DASHBOARD_UI_METRIC_ID } from '../../../dashboard_constants';
import { pluginServices } from '../../../services/plugin_services';
import { emptyScreenStrings } from '../../_dashboard_container_strings';
import { useDashboardContainer } from '../../embeddable/dashboard_container';
import { DASHBOARD_UI_METRIC_ID } from '../../../dashboard_constants';
import { emptyScreenStrings } from '../../_dashboard_container_strings';

export function DashboardEmptyScreen() {
const {
Expand Down Expand Up @@ -53,7 +53,7 @@ export function DashboardEmptyScreen() {
const originatingApp = embeddableAppContext?.currentAppId;

const goToLens = useCallback(() => {
if (!lensAlias || !lensAlias.aliasPath) return;
if (!lensAlias || !lensAlias.alias) return;
const trackUiMetric = usageCollection.reportUiCounter?.bind(
usageCollection,
DASHBOARD_UI_METRIC_ID
Expand All @@ -62,8 +62,8 @@ export function DashboardEmptyScreen() {
if (trackUiMetric) {
trackUiMetric(METRIC_TYPE.CLICK, `${lensAlias.name}:create`);
}
getStateTransfer().navigateToEditor(lensAlias.aliasApp, {
path: lensAlias.aliasPath,
getStateTransfer().navigateToEditor(lensAlias.alias.app, {
path: lensAlias.alias.path,
state: {
originatingApp,
originatingPath,
Expand Down
3 changes: 2 additions & 1 deletion src/plugins/links/kibana.jsonc
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,10 @@
"dashboard",
"embeddable",
"kibanaReact",
"kibanaUtils",
"presentationUtil",
"uiActionsEnhanced",
"kibanaUtils"
"visualizations"
],
"optionalPlugins": ["triggersActionsUi"],
"requiredBundles": ["savedObjects"]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,9 @@
*/

import type { SearchQuery } from '@kbn/content-management-plugin/common';
import { SerializableAttributes, VisualizationClient } from '@kbn/visualizations-plugin/public';
import { CONTENT_ID as contentTypeId, CONTENT_ID } from '../../common';
import type { LinksCrudTypes } from '../../common/content_management';
import { CONTENT_ID as contentTypeId } from '../../common';
import { contentManagement } from '../services/kibana_services';

const get = async (id: string) => {
Expand Down Expand Up @@ -65,3 +66,9 @@ export const linksClient = {
delete: deleteLinks,
search,
};

export function getLinksClient<
Attr extends SerializableAttributes = SerializableAttributes
>(): VisualizationClient<typeof CONTENT_ID, Attr> {
return linksClient as unknown as VisualizationClient<typeof CONTENT_ID, Attr>;
}
43 changes: 32 additions & 11 deletions src/plugins/links/public/editor/open_editor_flyout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,20 @@
*/

import React from 'react';
import { skip, take } from 'rxjs/operators';

import { withSuspense } from '@kbn/shared-ux-utility';
import { toMountPoint } from '@kbn/react-kibana-mount';
import { EuiLoadingSpinner, EuiPanel } from '@elastic/eui';
import { tracksOverlays } from '@kbn/embeddable-plugin/public';
import { DashboardContainer } from '@kbn/dashboard-plugin/public/dashboard_container';
import { tracksOverlays } from '@kbn/embeddable-plugin/public';
import { toMountPoint } from '@kbn/react-kibana-mount';
import { withSuspense } from '@kbn/shared-ux-utility';

import { LinksInput, LinksByReferenceInput, LinksEditorFlyoutReturn } from '../embeddable/types';
import { coreServices } from '../services/kibana_services';
import { runSaveToLibrary } from '../content_management/save_to_library';
import { OverlayRef } from '@kbn/core-mount-utils-browser';
import { Link, LinksLayoutType } from '../../common/content_management';
import { runSaveToLibrary } from '../content_management/save_to_library';
import { LinksByReferenceInput, LinksEditorFlyoutReturn, LinksInput } from '../embeddable/types';
import { getLinksAttributeService } from '../services/attribute_service';
import { coreServices } from '../services/kibana_services';

const LazyLinksEditor = React.lazy(() => import('../components/editor/links_editor'));

Expand All @@ -40,7 +42,8 @@ export async function openEditorFlyout(
const { attributes } = await attributeService.unwrapAttributes(initialInput);
const isByReference = attributeService.inputIsRefType(initialInput);
const initialLinks = attributes?.links;
const overlayTracker = tracksOverlays(parentDashboard) ? parentDashboard : undefined;
const overlayTracker =
parentDashboard && tracksOverlays(parentDashboard) ? parentDashboard : undefined;

if (!initialLinks) {
/**
Expand All @@ -55,6 +58,22 @@ export async function openEditorFlyout(
}

return new Promise((resolve, reject) => {
const closeEditorFlyout = (editorFlyout: OverlayRef) => {
if (overlayTracker) {
overlayTracker.clearOverlays();
} else {
editorFlyout.close();
}
};

/**
* Close the flyout whenever the app changes - this handles cases for when the flyout is open outside of the
* Dashboard app (`overlayTracker` is not available)
*/
coreServices.application.currentAppId$.pipe(skip(1), take(1)).subscribe(() => {
if (!overlayTracker) editorFlyout.close();
});

const onSaveToLibrary = async (newLinks: Link[], newLayout: LinksLayoutType) => {
const newAttributes = {
...attributes,
Expand All @@ -74,7 +93,7 @@ export async function openEditorFlyout(
attributes: newAttributes,
});
parentDashboard?.reload();
if (overlayTracker) overlayTracker.clearOverlays();
closeEditorFlyout(editorFlyout);
};

const onAddToDashboard = (newLinks: Link[], newLayout: LinksLayoutType) => {
Expand All @@ -94,12 +113,12 @@ export async function openEditorFlyout(
attributes: newAttributes,
});
parentDashboard?.reload();
if (overlayTracker) overlayTracker.clearOverlays();
closeEditorFlyout(editorFlyout);
};

const onCancel = () => {
reject();
if (overlayTracker) overlayTracker.clearOverlays();
closeEditorFlyout(editorFlyout);
};

const editorFlyout = coreServices.overlays.openFlyout(
Expand All @@ -125,6 +144,8 @@ export async function openEditorFlyout(
}
);

if (overlayTracker) overlayTracker.openOverlay(editorFlyout);
if (overlayTracker) {
overlayTracker.openOverlay(editorFlyout);
}
});
}
Original file line number Diff line number Diff line change
Expand Up @@ -125,8 +125,6 @@ export class LinksFactoryDefinition
initialInput: LinksInput,
parent?: DashboardContainer
): Promise<LinksEditorFlyoutReturn> {
if (!parent) return { newInput: {} };

const { openEditorFlyout } = await import('../editor/open_editor_flyout');

const { newInput, attributes } = await openEditorFlyout(
Expand Down
63 changes: 59 additions & 4 deletions src/plugins/links/public/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,22 +6,28 @@
* Side Public License, v 1.
*/

import { CoreSetup, CoreStart, Plugin } from '@kbn/core/public';
import {
ContentManagementPublicSetup,
ContentManagementPublicStart,
} from '@kbn/content-management-plugin/public';
import { CoreSetup, CoreStart, Plugin } from '@kbn/core/public';
import { DashboardStart } from '@kbn/dashboard-plugin/public';
import { DashboardContainer } from '@kbn/dashboard-plugin/public/dashboard_container';
import { EmbeddableSetup, EmbeddableStart } from '@kbn/embeddable-plugin/public';
import { PresentationUtilPluginStart } from '@kbn/presentation-util-plugin/public';
import { VisualizationsSetup } from '@kbn/visualizations-plugin/public';

import { APP_NAME } from '../common';
import { APP_ICON, APP_NAME, CONTENT_ID, LATEST_VERSION } from '../common';
import { LinksCrudTypes } from '../common/content_management';
import { LinksStrings } from './components/links_strings';
import { getLinksClient } from './content_management/links_content_management_client';
import { LinksFactoryDefinition } from './embeddable';
import { CONTENT_ID, LATEST_VERSION } from '../common';
import { LinksByReferenceInput } from './embeddable/types';
import { setKibanaServices } from './services/kibana_services';

export interface LinksSetupDependencies {
embeddable: EmbeddableSetup;
visualizations: VisualizationsSetup;
contentManagement: ContentManagementPublicSetup;
}

Expand All @@ -39,7 +45,9 @@ export class LinksPlugin

public setup(core: CoreSetup<LinksStartDependencies>, plugins: LinksSetupDependencies) {
core.getStartServices().then(([_, deps]) => {
plugins.embeddable.registerEmbeddableFactory(CONTENT_ID, new LinksFactoryDefinition());
const linksFactory = new LinksFactoryDefinition();

plugins.embeddable.registerEmbeddableFactory(CONTENT_ID, linksFactory);

plugins.contentManagement.registry.register({
id: CONTENT_ID,
Expand All @@ -48,6 +56,53 @@ export class LinksPlugin
},
name: APP_NAME,
});

const getExplicitInput = async ({
savedObjectId,
parent,
}: {
savedObjectId?: string;
parent?: DashboardContainer;
}) => {
try {
await linksFactory.getExplicitInput({ savedObjectId } as LinksByReferenceInput, parent);
} catch {
// swallow any errors - this just means that the user cancelled editing
}
return;
};

plugins.visualizations.registerAlias({
disableCreate: true, // do not allow creation through visualization listing page
name: CONTENT_ID,
title: APP_NAME,
icon: APP_ICON,
description: LinksStrings.getDescription(),
stage: 'experimental',
appExtensions: {
visualizations: {
docTypes: [CONTENT_ID],
searchFields: ['title^3'],
client: getLinksClient,
toListItem(linkItem: LinksCrudTypes['Item']) {
const { id, type, updatedAt, attributes } = linkItem;
const { title, description } = attributes;

return {
id,
title,
editor: { onEdit: (savedObjectId: string) => getExplicitInput({ savedObjectId }) },
description,
updatedAt,
icon: APP_ICON,
typeTitle: APP_NAME,
stage: 'experimental',
savedObjectType: type,
};
},
},
},
});
});
}

Expand Down
4 changes: 3 additions & 1 deletion src/plugins/links/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,9 @@
"@kbn/logging",
"@kbn/core-plugins-server",
"@kbn/react-kibana-mount",
"@kbn/react-kibana-context-theme"
"@kbn/react-kibana-context-theme",
"@kbn/visualizations-plugin",
"@kbn/core-mount-utils-browser"
],
"exclude": ["target/**/*"]
}
Original file line number Diff line number Diff line change
Expand Up @@ -504,11 +504,13 @@ describe('saved_visualize_utils', () => {
{
id: 'wat',
image: undefined,
editor: {
editUrl: '/edit/wat',
},
readOnly: false,
references: undefined,
icon: undefined,
savedObjectType: 'visualization',
editUrl: '/edit/wat',
type: 'test',
typeName: 'test',
typeTitle: undefined,
Expand Down
Loading

0 comments on commit 8eaebb6

Please sign in to comment.