-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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][Lens] Add "convert to lens" action to dashboard #146363
Changes from 20 commits
7df2590
1b00b5e
0fca07e
f78d79b
8676538
fe7c5cd
efe7d80
a2397db
15d3380
2e22b1e
c7fdc8e
b6f99e8
8a74239
0a6c890
cf4ac72
91020f0
54161f8
a5447b4
135b36d
1489189
9b35aa5
ed576bd
1d260b1
705101f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -106,6 +106,17 @@ | |
&:focus { | ||
background-color: $euiFocusBackgroundColor; | ||
} | ||
|
||
} | ||
|
||
.embPanel__optionsMenuPopover-notification::after { | ||
position: absolute; | ||
top: 0; | ||
right: 0; | ||
content: '•'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would it be better to use an There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @MichaelMarcialis, Adding EuiIcon anyway will require styles for correct position. For me in that case using pseudo element is good solution. Could you please describe a reason why is better adding new element instead of using pseudo element? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The only reason I asked was for the sake of consistency with other areas of Kibana that use the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @MichaelMarcialis, Could you please tell me where we have the same dot-notification idea in kibana? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe this is the first use of a |
||
transform: translate(50%, -50%); | ||
color: $euiColorAccent; | ||
font-size: $euiSizeL; | ||
} | ||
|
||
.embPanel .embPanel__optionsMenuButton { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,135 @@ | ||
/* | ||
* 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 { take } from 'rxjs/operators'; | ||
import { EuiFlexGroup, EuiFlexItem, EuiBadge } from '@elastic/eui'; | ||
import { METRIC_TYPE } from '@kbn/analytics'; | ||
import { reactToUiComponent } from '@kbn/kibana-react-plugin/public'; | ||
import { ActionExecutionContext } from '@kbn/ui-actions-plugin/public'; | ||
import { TimefilterContract } from '@kbn/data-plugin/public'; | ||
import { i18n } from '@kbn/i18n'; | ||
import { IEmbeddable, ViewMode } from '@kbn/embeddable-plugin/public'; | ||
import { Action } from '@kbn/ui-actions-plugin/public'; | ||
import { VisualizeEmbeddable } from '../embeddable'; | ||
import { DASHBOARD_VISUALIZATION_PANEL_TRIGGER } from '../triggers'; | ||
import { getUiActions, getApplication, getEmbeddable, getUsageCollection } from '../services'; | ||
|
||
export const ACTION_EDIT_IN_LENS = 'ACTION_EDIT_IN_LENS'; | ||
|
||
export interface EditInLensContext { | ||
embeddable: IEmbeddable; | ||
} | ||
|
||
const displayName = i18n.translate('visualizations.actions.editInLens.displayName', { | ||
defaultMessage: 'Convert to Lens', | ||
}); | ||
|
||
const ReactMenuItem: React.FC = () => { | ||
return ( | ||
<EuiFlexGroup alignItems="center"> | ||
<EuiFlexItem>{displayName}</EuiFlexItem> | ||
<EuiFlexItem grow={false}> | ||
<EuiBadge color={'accent'}> | ||
{i18n.translate('visualizations.tonNavMenu.tryItBadgeText', { | ||
defaultMessage: 'Try it', | ||
})} | ||
</EuiBadge> | ||
</EuiFlexItem> | ||
</EuiFlexGroup> | ||
); | ||
}; | ||
|
||
const UiMenuItem = reactToUiComponent(ReactMenuItem); | ||
|
||
const isVisualizeEmbeddable = (embeddable: IEmbeddable): embeddable is VisualizeEmbeddable => { | ||
return 'getVis' in embeddable; | ||
}; | ||
|
||
export class EditInLensAction implements Action<EditInLensContext> { | ||
public id = ACTION_EDIT_IN_LENS; | ||
public readonly type = ACTION_EDIT_IN_LENS; | ||
public order = 49; | ||
public showNotification = true; | ||
public currentAppId: string | undefined; | ||
|
||
constructor(private readonly timefilter: TimefilterContract) {} | ||
|
||
async execute(context: ActionExecutionContext<EditInLensContext>): Promise<void> { | ||
const application = getApplication(); | ||
if (application?.currentAppId$) { | ||
application.currentAppId$ | ||
.pipe(take(1)) | ||
.subscribe((appId: string | undefined) => (this.currentAppId = appId)); | ||
application.currentAppId$.subscribe(() => { | ||
getEmbeddable().getStateTransfer().isTransferInProgress = false; | ||
}); | ||
} | ||
const { embeddable } = context; | ||
if (isVisualizeEmbeddable(embeddable)) { | ||
const vis = embeddable.getVis(); | ||
const navigateToLensConfig = await vis.type.navigateToLens?.(vis, this.timefilter); | ||
const parentSearchSource = vis.data.searchSource?.getParent(); | ||
const searchFilters = parentSearchSource?.getField('filter'); | ||
const searchQuery = parentSearchSource?.getField('query'); | ||
const updatedWithMeta = { | ||
...navigateToLensConfig, | ||
title: | ||
vis.title || | ||
embeddable.getOutput().title || | ||
i18n.translate('visualizations.actions.editInLens.visulizationTitle', { | ||
defaultMessage: '{type} visualization', | ||
values: { | ||
type: vis.type.title, | ||
}, | ||
}), | ||
embeddableId: embeddable.id, | ||
originatingApp: this.currentAppId, | ||
searchFilters, | ||
searchQuery, | ||
isEmbeddable: true, | ||
}; | ||
if (navigateToLensConfig) { | ||
if (this.currentAppId) { | ||
getUsageCollection().reportUiCounter( | ||
this.currentAppId, | ||
METRIC_TYPE.CLICK, | ||
ACTION_EDIT_IN_LENS | ||
); | ||
} | ||
getEmbeddable().getStateTransfer().isTransferInProgress = true; | ||
getUiActions().getTrigger(DASHBOARD_VISUALIZATION_PANEL_TRIGGER).exec(updatedWithMeta); | ||
} | ||
} | ||
} | ||
|
||
getDisplayName(context: ActionExecutionContext<EditInLensContext>): string { | ||
return displayName; | ||
} | ||
|
||
MenuItem = UiMenuItem; | ||
|
||
getIconType(context: ActionExecutionContext<EditInLensContext>): string | undefined { | ||
return 'merge'; | ||
} | ||
|
||
async isCompatible(context: ActionExecutionContext<EditInLensContext>) { | ||
const { embeddable } = context; | ||
if (!isVisualizeEmbeddable(embeddable)) { | ||
return false; | ||
} | ||
const vis = embeddable.getVis(); | ||
if (!vis) { | ||
return false; | ||
} | ||
const canNavigateToLens = | ||
embeddable.getExpressionVariables?.()?.canNavigateToLens ?? | ||
(await vis.type.navigateToLens?.(vis, this.timefilter)); | ||
return Boolean(canNavigateToLens && embeddable.getInput().viewMode === ViewMode.EDIT); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm curious about this change, as the editor is in charge of returning the full input, and
hidePanelTitles
is part of the input, you could restore it by ensuring that the value isn't deleted between the Dashboard -> TSVB -> Lens navigation.This change likely works okay, but it seems a little poorly defined from the code side to say that the
hidePanelTitles
should never update onsave and return
if the new embeddable's type is different.If we eventually allowed editing of the title and the
hidePanelTitles
key from inside the editor, we would likely expect changes there to be applied, not overwritten.If at all possible, I would encourage you to try to maintain the
hidePanelTitles
value instead.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ThomThomson, In that case I will need to put this setting in navigate to lens config, then get this when user click on "Replace in dashboard" and define it in new embeddable input on Lens side. For me it looks strange that Lens will know about specific dashboard panel setting -
hidePanelTitles
and only when we use conversion logicThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are weird settings to be honest,
hidePanelTitles
, andtitle
are dashboard implementation details, but they are stored inexplicitInput
which means they should be managed on the individual embeddable level. It isn't ideal, but that is how the architecture is at the moment.That said, I'm okay if you want to leave this here in order to make the implementation on your side simpler, but it would be nice to have a comment at least explaining that this is temporary, and only required because the key is stored in
explicitInput
when it should be stored outside of it instead.On our side, we could look into moving this to a better place.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added comment for that