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

[Time to Visualize] Remove Panels from URL #86939

Merged
merged 43 commits into from
Feb 3, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
1e2bb42
First pass at removing panels from URL. Everything should work
ThomThomson Dec 24, 2020
8b2ac8b
Fix types
ThomThomson Dec 24, 2020
aa1140e
First version of unsaved panels listing
ThomThomson Jan 8, 2021
ac24142
Merge branch 'master' of github.com:elastic/kibana into feature/remov…
ThomThomson Jan 11, 2021
946c7c4
type fix
ThomThomson Jan 12, 2021
d4b0c11
Added confirm to create new when new dashboard already has unsaved ed…
ThomThomson Jan 12, 2021
25da78c
Fixed typo, ensured that snapshot share links work properly
ThomThomson Jan 13, 2021
ea9162d
Added unit testing for unsaved listing
ThomThomson Jan 14, 2021
de19cb7
Changed recently accessed functionality. Ensured proper unsaved state…
ThomThomson Jan 14, 2021
1699180
Merge branch 'master' of github.com:elastic/kibana into feature/remov…
ThomThomson Jan 14, 2021
e5f68d9
Removed issue caused by dashboard_security importing from dashboard_c…
ThomThomson Jan 14, 2021
c18bc1f
Merge branch 'master' of github.com:elastic/kibana into feature/remov…
ThomThomson Jan 18, 2021
b9e1c9a
split discarding changes from view / edit mode.
ThomThomson Jan 18, 2021
03214ef
Listing entries for Dashboards with unsaved edits now open in view mo…
ThomThomson Jan 18, 2021
c19d073
doc title change for New Dashboards, removed redirectTo from onDiscar…
ThomThomson Jan 19, 2021
0e6f6b2
fixing functional tests
ThomThomson Jan 19, 2021
f2cc036
added aria-labels to edit and discard buttons in unsaved listing
ThomThomson Jan 19, 2021
9503272
i18n fix
ThomThomson Jan 19, 2021
809199a
design tweaks to align with notifications UI
ryankeairns Jan 19, 2021
976b40d
Merge pull request #3 from ryankeairns/feature/removePanelsFromUrl
ThomThomson Jan 19, 2021
e7304a0
Merge branch 'master' into feature/removePanelsFromUrl
kibanamachine Jan 20, 2021
0a46cbb
Updated license headers, made both listing page create buttons use th…
ThomThomson Jan 20, 2021
0465fe3
worked around functional tests clearing session storage after each te…
ThomThomson Jan 21, 2021
dafb75b
reimplemented functional tests which manually edit the URL by using t…
ThomThomson Jan 21, 2021
be357af
manually create new dashboard in panel_context_menu test
ThomThomson Jan 21, 2021
634a8cc
revert cancel button name, remove panels retained toast
ThomThomson Jan 21, 2021
930087c
functional test fixes
ThomThomson Jan 25, 2021
1929a75
Merge branch 'master' of github.com:elastic/kibana into feature/remov…
ThomThomson Jan 25, 2021
3231d79
restored default color
ThomThomson Jan 26, 2021
2aab904
Added functional tests
ThomThomson Jan 27, 2021
53b2065
fixed jest
ThomThomson Jan 27, 2021
3f63de0
Merge branch 'master' of github.com:elastic/kibana into feature/remov…
ThomThomson Jan 27, 2021
9bdbaa2
Merge branch 'master' into feature/removePanelsFromUrl
kibanamachine Jan 27, 2021
09de507
Add responsive styles
ryankeairns Jan 28, 2021
55cda0e
Merge pull request #4 from ryankeairns/feature/removePanelsFromUrl
ThomThomson Jan 28, 2021
3d1cfeb
error handling for dashboard panel storage.
ThomThomson Jan 28, 2021
0190c26
Merge branch 'master' of github.com:elastic/kibana into feature/remov…
ThomThomson Jan 28, 2021
efd5b3f
refactor dashboard unsaved listing to show entries when loading
ThomThomson Jan 29, 2021
c714336
Merge branch 'master' into feature/removePanelsFromUrl
kibanamachine Feb 1, 2021
0bf8e68
revert import in dashboard_constants
ThomThomson Feb 1, 2021
d433d04
Merge branch 'feature/removePanelsFromUrl' of github.com:thomthomson/…
ThomThomson Feb 1, 2021
b4355d9
changed unsaved listing cancelation logic
ThomThomson Feb 3, 2021
c355032
Merge branch 'master' of github.com:elastic/kibana into feature/remov…
ThomThomson Feb 3, 2021
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
4 changes: 4 additions & 0 deletions src/plugins/dashboard/public/application/_dashboard_app.scss
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,10 @@
margin-bottom: 0 !important;
}

.dshUnsavedListingItem__loading {
color: $euiTextSubduedColor !important;
}

.dshUnsavedListingItem__actions {
margin-left: $euiSizeL + $euiSizeXS;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,6 @@ export class DashboardStateManager {
>;
private readonly stateContainerChangeSub: Subscription;
private readonly dashboardPanelStorage?: DashboardPanelStorage;
private readonly STATE_STORAGE_KEY = '_a';
public readonly kbnUrlStateStorage: IKbnUrlStateStorage;
private readonly stateSyncRef: ISyncStateRef;
private readonly allowByValueEmbeddables: boolean;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import {
EuiTitle,
} from '@elastic/eui';
import React, { useCallback, useEffect, useState } from 'react';
import useMount from 'react-use/lib/useMount';
import { DashboardSavedObject } from '../..';
import {
createConfirmStrings,
Expand All @@ -29,15 +28,16 @@ import { DashboardAppServices, DashboardRedirect } from '../types';
import { confirmDiscardUnsavedChanges } from './confirm_overlays';

const DashboardUnsavedItem = ({
dashboard,
id,
title,
onOpenClick,
onDiscardClick,
}: {
dashboard?: DashboardSavedObject;
id: string;
title?: string;
onOpenClick: () => void;
onDiscardClick: () => void;
}) => {
const title = dashboard?.title ?? getNewDashboardTitle();
return (
<div className="dshUnsavedListingItem">
<EuiFlexGroup
Expand All @@ -47,12 +47,20 @@ const DashboardUnsavedItem = ({
responsive={false}
>
<EuiFlexItem grow={false}>
<EuiIcon color="text" className="dshUnsavedListingItem__icon" type="dashboardApp" />
<EuiIcon
color="text"
className="dshUnsavedListingItem__icon"
type={title ? 'dashboardApp' : 'clock'}
/>
</EuiFlexItem>
<EuiFlexItem grow={false}>
<EuiTitle size="xxs">
<h4 className="dshUnsavedListingItem__title">
{dashboard?.title ?? getNewDashboardTitle()}
<h4
className={`dshUnsavedListingItem__title ${
title ? '' : 'dshUnsavedListingItem__loading'
}`}
>
{title || dashboardUnsavedListingStrings.getLoadingTitle()}
</h4>
</EuiTitle>
</EuiFlexItem>
Expand All @@ -68,9 +76,10 @@ const DashboardUnsavedItem = ({
flush="left"
size="s"
color="primary"
disabled={!title}
onClick={onOpenClick}
data-test-subj={`edit-unsaved-${title.split(' ').join('-')}`}
aria-label={dashboardUnsavedListingStrings.getEditAriaLabel(title)}
data-test-subj={title ? `edit-unsaved-${title.split(' ').join('-')}` : undefined}
aria-label={dashboardUnsavedListingStrings.getEditAriaLabel(title ?? id)}
>
ThomThomson marked this conversation as resolved.
Show resolved Hide resolved
{dashboardUnsavedListingStrings.getEditTitle()}
</EuiButtonEmpty>
Expand All @@ -80,9 +89,10 @@ const DashboardUnsavedItem = ({
flush="left"
size="s"
color="danger"
disabled={!title}
onClick={onDiscardClick}
data-test-subj={`discard-unsaved-${title.split(' ').join('-')}`}
aria-label={dashboardUnsavedListingStrings.getDiscardAriaLabel(title)}
data-test-subj={title ? `discard-unsaved-${title.split(' ').join('-')}` : undefined}
aria-label={dashboardUnsavedListingStrings.getDiscardAriaLabel(title ?? id)}
>
{dashboardUnsavedListingStrings.getDiscardTitle()}
</EuiButtonEmpty>
Expand All @@ -92,6 +102,10 @@ const DashboardUnsavedItem = ({
);
};

interface UnsavedItemMap {
[key: string]: DashboardSavedObject;
}

export const DashboardUnsavedListing = ({ redirectTo }: { redirectTo: DashboardRedirect }) => {
const {
services: {
Expand All @@ -101,8 +115,11 @@ export const DashboardUnsavedListing = ({ redirectTo }: { redirectTo: DashboardR
},
} = useKibana<DashboardAppServices>();

const [items, setItems] = useState<JSX.Element[]>([]);
const [dashboardIds, setDashboardIds] = useState<string[]>([]);
const [items, setItems] = useState<UnsavedItemMap>({});
const [mounted, setMounted] = useState(true);
const [dashboardIds, setDashboardIds] = useState<string[]>(
dashboardPanelStorage.getDashboardIdsWithUnsavedChanges()
);

const onOpen = useCallback(
(id?: string) => {
Expand All @@ -125,50 +142,52 @@ export const DashboardUnsavedListing = ({ redirectTo }: { redirectTo: DashboardR
[overlays, dashboardPanelStorage]
);

useMount(() => {
setDashboardIds(dashboardPanelStorage.getDashboardIdsWithUnsavedChanges());
useEffect(() => {
return () => setMounted(false);
Copy link
Contributor

@Dosant Dosant Feb 3, 2021

Choose a reason for hiding this comment

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

This runs on every state change. If you wanted something like componentWillUnmount then it would have to be implemented a bit different: https://github.com/streamich/react-use/blob/master/src/useUnmount.ts

But as I understand, we need this for request cancelation, then the best would be to handle it as part of the effect with the request

useEffect(() => {
   let canceled = false;
   request().then(() => {
     if (canceled) return;
   })

   return () => {
       canceled = true;
  }
})

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wow, huh. I meant to pass in an empty dependency array there, which would have made it run only on unmount. I need to be more careful with these things. Thanks for noticing this!

That said, the approach of canceling within the useEffect is cleaner anyway. I have implemented it.

});

useEffect(() => {
let hasNewDashboard = false;
const dashPromises = dashboardIds
.filter((id) => {
if (id !== DASHBOARD_PANELS_UNSAVED_ID) {
return true;
}
hasNewDashboard = true;
return false;
})
.filter((id) => id !== DASHBOARD_PANELS_UNSAVED_ID)
.map((dashboardId) => savedDashboards.get(dashboardId));
Promise.all(dashPromises).then((dashboards: DashboardSavedObject[]) => {
ThomThomson marked this conversation as resolved.
Show resolved Hide resolved
ThomThomson marked this conversation as resolved.
Show resolved Hide resolved
const newItems = dashboards.map((dashboard) => (
<DashboardUnsavedItem
key={dashboard.id}
dashboard={dashboard}
onOpenClick={() => onOpen(dashboard.id)}
onDiscardClick={() => onDiscard(dashboard.id)}
/>
));
if (hasNewDashboard) {
newItems.unshift(
<DashboardUnsavedItem
key={DASHBOARD_PANELS_UNSAVED_ID}
onOpenClick={() => onOpen()}
onDiscardClick={() => onDiscard()}
/>
);
const dashboardMap = {};
if (!mounted) {
return;
}
setItems(newItems);
setItems(
dashboards.reduce((map, dashboard) => {
return {
...map,
[dashboard.id || DASHBOARD_PANELS_UNSAVED_ID]: dashboard,
};
}, dashboardMap)
);
});
}, [dashboardIds, onOpen, onDiscard, savedDashboards]);
}, [dashboardIds, savedDashboards, mounted]);

return items.length === 0 ? null : (
return dashboardIds.length === 0 ? null : (
<>
<EuiCallOut
heading="h3"
title={dashboardUnsavedListingStrings.getUnsavedChangesTitle(items.length > 1)}
title={dashboardUnsavedListingStrings.getUnsavedChangesTitle(dashboardIds.length > 1)}
>
{items}
{dashboardIds.map((dashboardId: string) => {
const title: string | undefined =
dashboardId === DASHBOARD_PANELS_UNSAVED_ID
? getNewDashboardTitle()
: items[dashboardId]?.title;
const redirectId = dashboardId === DASHBOARD_PANELS_UNSAVED_ID ? undefined : dashboardId;
return (
<DashboardUnsavedItem
key={dashboardId}
id={dashboardId}
title={title}
onOpenClick={() => onOpen(redirectId)}
onDiscardClick={() => onDiscard(redirectId)}
/>
);
})}
</EuiCallOut>
<EuiSpacer size="m" />
</>
Expand Down
20 changes: 16 additions & 4 deletions src/plugins/dashboard/public/dashboard_constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@
* Public License, v 1.
*/

import { ViewMode } from './services/embeddable';
import { setStateToKbnUrl } from './services/kibana_utils';

const DASHBOARD_STATE_STORAGE_KEY = '_a';

export const DashboardConstants = {
Expand All @@ -20,11 +23,20 @@ export const DashboardConstants = {
};

export function createDashboardEditUrl(id?: string, editMode?: boolean) {
const edit = editMode ? `?${DASHBOARD_STATE_STORAGE_KEY}=(viewMode:edit)` : '';
if (id) {
return `${DashboardConstants.VIEW_DASHBOARD_URL}/${id}${edit}`;
if (!id) {
return `${DashboardConstants.CREATE_NEW_DASHBOARD_URL}`;
}
const viewUrl = `${DashboardConstants.VIEW_DASHBOARD_URL}/${id}`;
if (editMode) {
const editUrl = setStateToKbnUrl(
DASHBOARD_STATE_STORAGE_KEY,
{ viewMode: ViewMode.EDIT },
{ useHash: false, storeInHashQuery: false },
viewUrl
);
return editUrl;
}
return `${DashboardConstants.CREATE_NEW_DASHBOARD_URL}${edit}`;
return viewUrl;
}

export function createDashboardListingFilterUrl(filter: string | undefined) {
Expand Down
4 changes: 4 additions & 0 deletions src/plugins/dashboard/public/dashboard_strings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -358,6 +358,10 @@ export const dashboardUnsavedListingStrings = {
: dashboardListingTable.getEntityName(),
},
}),
getLoadingTitle: () =>
i18n.translate('dashboard.listing.unsaved.loading', {
defaultMessage: 'Loading',
}),
getEditAriaLabel: (title: string) =>
i18n.translate('dashboard.listing.unsaved.editAria', {
defaultMessage: 'Continue editing {title}',
Expand Down