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 Navigation] Add creation UI #160179

Merged
Merged
Show file tree
Hide file tree
Changes from 63 commits
Commits
Show all changes
65 commits
Select commit Hold shift + click to select a range
b1d9f30
Make navigation embeddable creatable
Heenawter Jun 21, 2023
e0c4bd2
Print dashboard list
Heenawter Jun 21, 2023
7a0ac38
Filter out current dashboard
Heenawter Jun 21, 2023
6faccad
Switch to `EuiSelectable` for dashboard picker
Heenawter Jun 21, 2023
73ad237
Add list of links to render
Heenawter Jun 22, 2023
2198908
Fixing bug with current dashboard
Heenawter Jun 22, 2023
50638fc
Force current dashboard to the top of the list
Heenawter Jun 22, 2023
6982698
Add custom labels
Heenawter Jun 22, 2023
bc9c863
Simplify explicit input to only include what is **absolutely necessary**
Heenawter Jun 22, 2023
6e54b30
Make always editable - will need to change this
Heenawter Jun 23, 2023
499b001
Remove unnecessary stuff from input + move editor to component
Heenawter Jun 23, 2023
d58c714
Fetch dashboard titles+descriptions and set in component state
Heenawter Jun 23, 2023
fa69298
Update styling with Andrea's designs
Heenawter Jun 23, 2023
6dadf88
Custom search
Heenawter Jun 23, 2023
5768166
First attempt at allowing external links
Heenawter Jun 26, 2023
e70ed3c
Make it scrollable
Heenawter Jun 26, 2023
8523c77
Remove image embeddable dependency + start clean up
Heenawter Jun 26, 2023
2720ebe
Improve loading state
Heenawter Jun 26, 2023
21d35ce
Search by title only
Heenawter Jun 27, 2023
338abbb
Switch back to embeddable handling current dashboard logic
Heenawter Jun 27, 2023
3c760a6
Clean up + fix state management
Heenawter Jun 27, 2023
c8029e3
Handle deselection better
Heenawter Jun 27, 2023
0215127
[CI] Auto-commit changed files from 'node scripts/precommit_hook.js -…
kibanamachine Jun 27, 2023
f6c59f9
Remove add button in view mode
Heenawter Jun 27, 2023
0642e60
Update editor designs
Heenawter Jun 27, 2023
a35e341
Small clean up
Heenawter Jun 27, 2023
a013db2
Switch all hardcoded strings to i18n strings
Heenawter Jun 28, 2023
2bf27bb
First attempt at edit - broken
Heenawter Jun 28, 2023
38f87ed
Make each link an embeddable - half working
Heenawter Jun 28, 2023
a1c5127
Clean up
Heenawter Jun 28, 2023
acf7867
Add external link embeddable
Heenawter Jun 28, 2023
1a62f9c
Clean up + fix unsaved changes bug
Heenawter Jun 29, 2023
94974fd
Clean up + update dashboard title when necessary
Heenawter Jun 29, 2023
7725e52
Update dashboard link on dashboard clone/save as
Heenawter Jun 29, 2023
7687f36
Clean up before redesign
Heenawter Jun 30, 2023
32a6b85
Start work in edit process + disable/enable expected actions
Heenawter Jun 30, 2023
6bfef0a
Switch to flyout design
Heenawter Jun 30, 2023
6a76e0e
Fix bug with label
Heenawter Jun 30, 2023
ef92a99
First attempt at multi flyout creation
Heenawter Jul 4, 2023
80eac65
Editing/creating almost working
Heenawter Jul 4, 2023
304e3b7
Add custom editors
Heenawter Jul 4, 2023
977d135
Continuing to work on creation process
Heenawter Jul 5, 2023
bde1630
Fill in dashboard info for link list
Heenawter Jul 5, 2023
1b095df
Start clean up
Heenawter Jul 5, 2023
fcbbd04
Clean up strings
Heenawter Jul 5, 2023
4da5749
Remove outer folder
Heenawter Jul 5, 2023
5f07ba0
Replace remaining strings
Heenawter Jul 5, 2023
e55947d
Add a11y support
Heenawter Jul 5, 2023
f4ad6a2
Fix i18n string namespaces
Heenawter Jul 5, 2023
049608c
Cleaner implementation
Heenawter Jul 5, 2023
173edd0
Final clean up
Heenawter Jul 5, 2023
31b4d8c
Go back to links stored in explicit input
Heenawter Jul 6, 2023
9b41ed8
Clean up strings
Heenawter Jul 6, 2023
2b0ae54
Final clean up
Heenawter Jul 6, 2023
2cbe2c9
Truncate link text in panel editor
Heenawter Jul 6, 2023
1c71b1c
Address first round of feedback
Heenawter Jul 6, 2023
7d1864b
Address linting issues
Heenawter Jul 6, 2023
47e09f5
Add debounce to dashboard search
Heenawter Jul 6, 2023
3644406
Merge branch 'navigation-embeddable' into add-creation-ui_2023-06-21
Heenawter Jul 7, 2023
c0c81e2
Remove `flush=left` for add button
Heenawter Jul 7, 2023
d71df9e
Rename open flyout method
Heenawter Jul 7, 2023
9b10044
Add another TODO comment
Heenawter Jul 7, 2023
852e990
Address design feedback
Heenawter Jul 7, 2023
b0358d1
Close only link editor flyout on cancel
Heenawter Jul 10, 2023
efbfddf
Fix missed boolean
Heenawter Jul 10, 2023
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 @@ -101,7 +101,7 @@ export function DashboardEditingToolbar() {

let explicitInput: Awaited<ReturnType<typeof embeddableFactory.getExplicitInput>>;
try {
explicitInput = await embeddableFactory.getExplicitInput();
explicitInput = await embeddableFactory.getExplicitInput(undefined, dashboard);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had to modify the getExplicitInput for embeddable factories to accept an optional parent - this is because, when creating the navigation container (i.e. before the container embeddable actually exists), I need to know the ID of the parent dashboard so that I can add the current tag to the dashboard list even when creating a brand new navigation embeddable:

Screenshot 2023-07-06 at 2 43 52 PM

} catch (e) {
// error likely means user canceled embeddable creation
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,11 +65,12 @@ export const dashboardContentManagementServiceFactory: DashboardContentManagemen
dashboardSessionStorage,
}),
findDashboards: {
search: ({ hasReference, hasNoReference, search, size }) =>
search: ({ hasReference, hasNoReference, search, size, options }) =>
searchDashboards({
contentManagement,
hasNoReference,
hasReference,
options,
search,
size,
}),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import { DASHBOARD_CONTENT_ID } from '../../../dashboard_constants';

export interface SearchDashboardsArgs {
contentManagement: DashboardStartDependencies['contentManagement'];
options?: DashboardCrudTypes['SearchIn']['options'];
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added search options so that the navigation embeddable dashboard search could match only on title and not description.

hasNoReference?: SavedObjectsFindOptionsReference[];
hasReference?: SavedObjectsFindOptionsReference[];
search: string;
Expand All @@ -33,6 +34,7 @@ export async function searchDashboards({
contentManagement,
hasNoReference,
hasReference,
options,
search,
size,
}: SearchDashboardsArgs): Promise<SearchDashboardsResponse> {
Expand All @@ -52,6 +54,7 @@ export async function searchDashboards({
excluded: (hasNoReference ?? []).map(({ id }) => id),
},
},
options,
});
return {
total,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,10 @@ export interface SaveDashboardReturn {
*/
export interface FindDashboardsService {
search: (
props: Pick<SearchDashboardsArgs, 'hasReference' | 'hasNoReference' | 'search' | 'size'>
props: Pick<
SearchDashboardsArgs,
'hasReference' | 'hasNoReference' | 'search' | 'size' | 'options'
>
) => Promise<SearchDashboardsResponse>;
findByIds: (ids: string[]) => Promise<FindDashboardsByIdResponse[]>;
findByTitle: (title: string) => Promise<{ id: string } | undefined>;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ export class EditPanelAction implements Action<ActionContext> {
}

const oldExplicitInput = embeddable.getExplicitInput();
const newExplicitInput = await factory.getExplicitInput(oldExplicitInput);
const newExplicitInput = await factory.getExplicitInput(oldExplicitInput, embeddable.parent);
embeddable.parent?.replaceEmbeddable(embeddable.id, newExplicitInput);
return;
}
Expand Down
3 changes: 1 addition & 2 deletions src/plugins/embeddable/public/lib/containers/container.ts
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,7 @@ export abstract class Container<

public async getExplicitInputIsEqual(lastInput: TContainerInput) {
const { panels: lastPanels, ...restOfLastInput } = lastInput;
const { panels: currentPanels, ...restOfCurrentInput } = this.getInput();
const { panels: currentPanels, ...restOfCurrentInput } = this.getExplicitInput();
Copy link
Contributor Author

@Heenawter Heenawter Jul 6, 2023

Choose a reason for hiding this comment

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

Note
Since the navigation embeddable is no longer a container, this bug no longer impacts it; however, I figured I might as well keep this fix.

This was a bug that was never caught because we've never had a container embeddable as a panel in Dashboard before - basically, because we were calling getInput here instead of getExplicitInput, the dashboard always had unsaved changes back when the navigation embeddable was a container :)

const otherInputIsEqual = isEqual(restOfLastInput, restOfCurrentInput);
if (!otherInputIsEqual) return false;

Expand Down Expand Up @@ -371,7 +371,6 @@ export abstract class Container<
initializeSettings?: EmbeddableContainerSettings
) {
let initializeOrder = Object.keys(initialInput.panels);

if (initializeSettings?.childIdInitializeOrder) {
const initializeOrderSet = new Set<string>();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,10 @@ export interface EmbeddableFactory<
*
* Can be used to edit an embeddable by re-requesting explicit input. Initial input can be provided to allow the editor to show the current state.
*/
getExplicitInput(initialInput?: Partial<TEmbeddableInput>): Promise<Partial<TEmbeddableInput>>;
getExplicitInput(
initialInput?: Partial<TEmbeddableInput>,
parent?: IContainer
): Promise<Partial<TEmbeddableInput>>;

/**
* Creates a new embeddable instance based off the saved object id.
Expand Down
19 changes: 10 additions & 9 deletions src/plugins/embeddable/public/lib/panel/embeddable_panel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -430,27 +430,28 @@ export class EmbeddablePanel extends React.Component<Props, State> {
};

private getActionContextMenuPanel = async () => {
let regularActions =
const regularActions =
(await this.props.getActions?.(CONTEXT_MENU_TRIGGER, {
embeddable: this.props.embeddable,
})) ?? [];

const { disabledActions } = this.props.embeddable.getInput();

let allActions = regularActions.concat(
Object.values(this.state.universalActions ?? {}) as Array<Action<object>>
);
if (disabledActions) {
const removeDisabledActions = removeById(disabledActions);
regularActions = regularActions.filter(removeDisabledActions);
allActions = allActions.filter(removeDisabledActions);
Copy link
Contributor Author

@Heenawter Heenawter Jul 6, 2023

Choose a reason for hiding this comment

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

Because drilldowns are considered a "universal action," I had to do the filtering of disabled actions after the universal actions were added to the action list so that I could filter the drilldown action out of the link panel.

With this change, the drilldown UI action is now disabled for the navigation embeddable:

image

Copy link
Contributor

@ThomThomson ThomThomson Jul 7, 2023

Choose a reason for hiding this comment

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

I'll need to mirror this change in #159837.

Done in 1958bd7

}

let sortedActions = regularActions
.concat(Object.values(this.state.universalActions || {}) as Array<Action<object>>)
.sort(sortByOrderField);

if (this.props.actionPredicate) {
sortedActions = sortedActions.filter(({ id }) => this.props.actionPredicate!(id));
allActions = allActions.filter(({ id }) => this.props.actionPredicate!(id));
}
allActions.sort(sortByOrderField);

const panels = await buildContextMenuForActions({
actions: sortedActions.map((action) => ({
actions: allActions.map((action) => ({
action,
context: { embeddable: this.props.embeddable },
trigger: contextMenuTrigger,
Expand All @@ -460,7 +461,7 @@ export class EmbeddablePanel extends React.Component<Props, State> {

return {
panels,
actions: sortedActions,
actions: allActions,
};
};
}
11 changes: 4 additions & 7 deletions src/plugins/navigation_embeddable/kibana.jsonc
Original file line number Diff line number Diff line change
@@ -1,17 +1,14 @@
{
"type": "plugin",
"id": "@kbn/navigation-embeddable-plugin",
"owner": "@elastic/kibana-presentation",
"id": "@kbn/navigation-embeddable-plugin",
"description": "An embeddable for quickly navigating between dashboards.",
"plugin": {
"id": "navigationEmbeddable",
"server": false,
"browser": true,
"requiredPlugins": [
"embeddable"
],
"optionalPlugins": [],
"requiredBundles": [
]
"requiredPlugins": ["dashboard", "embeddable", "kibanaReact", "presentationUtil"],
"optionalPlugins": ["triggersActionsUi"],
"requiredBundles": []
}
}
25 changes: 25 additions & 0 deletions src/plugins/navigation_embeddable/public/_mixins.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
@import '../../../core/public/mixins';

@keyframes euiFlyoutAnimation {
0% {
opacity: 0;
transform: translateX(100%);
}

100% {
opacity: 1;
transform: translateX(0%);
}
}

@mixin euiFlyout {
@include kibanaFullBodyHeight();
border-left: $euiBorderThin;
position: fixed;
width: 50%;
z-index: $euiZFlyout;
background: $euiColorEmptyShade;
display: flex;
flex-direction: column;
align-items: stretch;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

import React from 'react';
import useAsync from 'react-use/lib/useAsync';

import { EuiButtonEmpty } from '@elastic/eui';
import { DashboardContainer } from '@kbn/dashboard-plugin/public/dashboard_container';

import {
DASHBOARD_LINK_TYPE,
NavigationEmbeddableLink,
NavigationLinkInfo,
} from '../../embeddable/types';
import { fetchDashboard } from './dashboard_link_tools';
import { useNavigationEmbeddable } from '../../embeddable/navigation_embeddable';

export const DashboardLinkComponent = ({ link }: { link: NavigationEmbeddableLink }) => {
const navEmbeddable = useNavigationEmbeddable();

const dashboardContainer = navEmbeddable.parent as DashboardContainer;
const parentDashboardTitle = dashboardContainer.select((state) => state.explicitInput.title);
const parentDashboardId = dashboardContainer.select((state) => state.componentState.lastSavedId);

const { loading: loadingDestinationDashboard, value: destinationDashboard } =
useAsync(async () => {
return await fetchDashboard(link.destination);
}, [link, parentDashboardId]);
Comment on lines +31 to +33
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Consider the following scenario:

  1. You add a link to your current dashboard to a navigation embeddable
  2. You rename your current dashboard without saving as/cloning, so the parentDashboardId stays the same
  3. The displayed text for the link added in step one should update to the new title
  4. Now, you Save as your dashboard with a different title from step 2 - so, parentDashboardId has now changed
  5. As expected, the link from step 1 no longer points to your "current" dashboard, so it is now clickable - also, because the parentDashboardId, we re-fetch the destinationDashboard so that we can get the updated title from step 3, since parentDashboardTitle is no longer representing it.

That is why I added parentDashboardId to the dependencies here


return (
<EuiButtonEmpty
isLoading={loadingDestinationDashboard}
iconType={NavigationLinkInfo[DASHBOARD_LINK_TYPE].icon}
{...(link.destination === parentDashboardId
? {
color: 'text',
}
: {
color: 'primary',
onClick: () => {}, // TODO: As part of https://github.com/elastic/kibana/issues/154381, connect to drilldown
})}
>
{link.label ||
(link.destination === parentDashboardId
? parentDashboardTitle
: destinationDashboard?.attributes.title)}
</EuiButtonEmpty>
);
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

import { debounce } from 'lodash';
import useAsync from 'react-use/lib/useAsync';
import React, { useEffect, useMemo, useState } from 'react';

import {
EuiBadge,
EuiSpacer,
EuiHighlight,
EuiSelectable,
EuiFieldSearch,
EuiSelectableOption,
} from '@elastic/eui';
import { DashboardContainer } from '@kbn/dashboard-plugin/public/dashboard_container';
import { DashboardItem } from '../../embeddable/types';
import { memoizedFetchDashboards } from './dashboard_link_tools';
import { DashboardLinkEmbeddableStrings } from './dashboard_link_strings';

export const DashboardLinkDestinationPicker = ({
setDestination,
setPlaceholder,
currentDestination,
parentDashboard,
...other
}: {
setDestination: (destination?: string) => void;
setPlaceholder: (placeholder?: string) => void;
currentDestination?: string;
parentDashboard?: DashboardContainer;
}) => {
const [searchString, setSearchString] = useState<string>('');
const [selectedDashboard, setSelectedDashboard] = useState<DashboardItem | undefined>();
const [dashboardListOptions, setDashboardListOptions] = useState<EuiSelectableOption[]>([]);

const parentDashboardId = parentDashboard?.select((state) => state.componentState.lastSavedId);

const { loading: loadingDashboardList, value: dashboardList } = useAsync(async () => {
return await memoizedFetchDashboards(searchString, undefined, parentDashboardId);
}, [searchString, parentDashboardId]);
Comment on lines +44 to +46
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Similar to https://github.com/elastic/kibana/pull/160179/files#r1254915984, we want a save as action to trigger the dashboard list to re-fetch itself - that way, the new "current" dashboard can once again be forced to the top of the dashboard list.


useEffect(() => {
const dashboardOptions =
(dashboardList ?? []).map((dashboard: DashboardItem) => {
return {
data: dashboard,
label: dashboard.attributes.title,
...(dashboard.id === parentDashboardId
? {
prepend: (
<EuiBadge>{DashboardLinkEmbeddableStrings.getCurrentDashboardLabel()}</EuiBadge>
),
}
: {}),
} as EuiSelectableOption;
}) ?? [];

setDashboardListOptions(dashboardOptions);
}, [dashboardList, parentDashboardId, searchString]);

const debouncedSetSearch = useMemo(
() =>
debounce((newSearch: string) => {
setSearchString(newSearch);
}, 250),
[setSearchString]
);

useEffect(() => {
if (selectedDashboard) {
setDestination(selectedDashboard.id);
setPlaceholder(selectedDashboard.attributes.title);
} else {
setDestination(undefined);
setPlaceholder(undefined);
}
}, [selectedDashboard, setDestination, setPlaceholder]);

/* {...other} is needed so all inner elements are treated as part of the form */
return (
<div {...other}>
<EuiFieldSearch
isClearable={true}
placeholder={DashboardLinkEmbeddableStrings.getSearchPlaceholder()}
onChange={(e) => {
debouncedSetSearch(e.target.value);
}}
/>
<EuiSpacer size="s" />
<EuiSelectable
singleSelection={true}
options={dashboardListOptions}
isLoading={loadingDashboardList}
listProps={{ onFocusBadge: false, bordered: true, isVirtualized: true }}
aria-label={DashboardLinkEmbeddableStrings.getDashboardPickerAriaLabel()}
onChange={(newOptions, _, selected) => {
if (selected.checked) {
setSelectedDashboard(selected.data as DashboardItem);
} else {
setSelectedDashboard(undefined);
}
setDashboardListOptions(newOptions);
}}
renderOption={(option) => {
return <EuiHighlight search={searchString}>{option.label}</EuiHighlight>;
}}
>
{(list) => list}
</EuiSelectable>
</div>
);
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

import { i18n } from '@kbn/i18n';

export const DashboardLinkEmbeddableStrings = {
getDisplayName: () =>
i18n.translate('navigationEmbeddable.dashboardLink.displayName', {
defaultMessage: 'Dashboard',
}),
getDescription: () =>
i18n.translate('navigationEmbeddable.dsahboardLink.description', {
defaultMessage: 'Go to dashboard',
}),
getSearchPlaceholder: () =>
i18n.translate('navigationEmbeddable.dashboardLink.editor.searchPlaceholder', {
defaultMessage: 'Search for a dashboard',
}),
getDashboardPickerAriaLabel: () =>
i18n.translate('navigationEmbeddable.dashboardLink.editor.dashboardPickerAriaLabel', {
defaultMessage: 'Pick a destination dashboard',
}),
getCurrentDashboardLabel: () =>
i18n.translate('navigationEmbeddable.dashboardLink.editor.currentDashboardLabel', {
defaultMessage: 'Current',
}),
};
Loading