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

[Lens] Inline editing of lens panels on a dashboard or canvas #166169

Merged
merged 59 commits into from
Sep 27, 2023
Merged
Show file tree
Hide file tree
Changes from 51 commits
Commits
Show all changes
59 commits
Select commit Hold shift + click to select a range
636f9c5
[Lens] Inline editing of lens panels
stratoula Sep 11, 2023
5bd75d6
Fix types CI check
stratoula Sep 11, 2023
e6167bc
Merge branch 'main' into inline-editing-byvalue
stratoula Sep 12, 2023
97bb468
Remove unecessary query check
stratoula Sep 12, 2023
21a2b49
Revert
stratoula Sep 12, 2023
bfaaba5
Fixes in initialization logic
stratoula Sep 12, 2023
b71a16c
Some cleanup
stratoula Sep 12, 2023
d99c926
Fix types
stratoula Sep 12, 2023
69e80bf
Merge branch 'main' into inline-editing-byvalue
stratoula Sep 12, 2023
fad8506
Fix jest test
stratoula Sep 13, 2023
0029688
Remove test case
stratoula Sep 13, 2023
34d32a9
Some refactoring and props renaming
stratoula Sep 13, 2023
93020bb
Display the action on dashboard edit mode and when the user has the p…
stratoula Sep 13, 2023
cf620ad
Refactoring, fix the activeData refresh bug
stratoula Sep 13, 2023
0b18c38
Remove the reset button from Discover
stratoula Sep 13, 2023
78f0ea2
Change the unified histogram implementation to get the embeddable out…
stratoula Sep 14, 2023
9bd9cdd
Enable reset button in Discover
stratoula Sep 14, 2023
02cfa2f
Navigate to the editor
stratoula Sep 14, 2023
a66be7e
Merge branch 'main' into inline-editing-byvalue
stratoula Sep 18, 2023
08d704c
fix translation
stratoula Sep 18, 2023
be818f0
Fix content management change and userMessages, add indexpattern chan…
stratoula Sep 18, 2023
02a8884
Fixes broken jest tests
stratoula Sep 18, 2023
0e4b64b
Remove edit action and fix FTs
stratoula Sep 18, 2023
22ae74e
Display clone panel
stratoula Sep 18, 2023
c48bd2a
fixes the bug with adding a layer with another dataview
stratoula Sep 19, 2023
2dcc215
Fix bugs concerning the indexpattern id
stratoula Sep 19, 2023
3a49513
Add more unit tests
stratoula Sep 19, 2023
52d0ae2
Adds Functional tests
stratoula Sep 19, 2023
3de9971
Merge with main and resolve conflicts
stratoula Sep 20, 2023
c2324c8
Fix some bugs
stratoula Sep 20, 2023
997e886
Fixes
stratoula Sep 20, 2023
26122aa
Revert
stratoula Sep 20, 2023
34ea0b7
Fixes CI
stratoula Sep 20, 2023
b2dfd6c
Adds back navigation buttons
stratoula Sep 20, 2023
1357877
Merge branch 'main' into inline-editing-byvalue
stratoula Sep 20, 2023
93ca7d9
Fixes inline tests
stratoula Sep 20, 2023
1ccef1a
Fixes last FT fai;
stratoula Sep 20, 2023
cebc687
Update src/plugins/unified_histogram/public/container/hooks/use_state…
stratoula Sep 22, 2023
96294f4
Update src/plugins/unified_histogram/public/container/hooks/use_state…
stratoula Sep 22, 2023
96f8d52
Update src/plugins/unified_histogram/public/chart/chart_config_panel.tsx
stratoula Sep 22, 2023
3ed8d35
Merge branch 'main' into inline-editing-byvalue
stratoula Sep 22, 2023
49acc97
Fixes Safari scroll issue
stratoula Sep 22, 2023
1056df8
Removes the extra shadown from the dimension flyout when in editing mode
stratoula Sep 22, 2023
c1453fa
Merge branch 'main' into inline-editing-byvalue
stratoula Sep 25, 2023
f9ecad4
Apply PR comments
stratoula Sep 25, 2023
056c584
Merge branch 'main' into inline-editing-byvalue
stratoula Sep 25, 2023
17b910f
Fixes dancing in layer settings too
stratoula Sep 25, 2023
6f32df6
Removes dancing from the load annotations from library flyout
stratoula Sep 25, 2023
f8f23e7
Merge branch 'main' into inline-editing-byvalue
stratoula Sep 25, 2023
441313e
Merge branch 'main' into inline-editing-byvalue
stratoula Sep 25, 2023
5db6354
Merge branch 'main' into inline-editing-byvalue
stratoula Sep 26, 2023
f64ac1b
Fixes broken types
stratoula Sep 26, 2023
ca55c78
Fixes pointer events problem
stratoula Sep 26, 2023
83ed9a4
Address PR comments
stratoula Sep 26, 2023
2e5f380
Merge branch 'main' into inline-editing-byvalue
stratoula Sep 26, 2023
dcdbe04
Merge branch 'main' into inline-editing-byvalue
stratoula Sep 26, 2023
dd1f8f5
Merge branch 'main' into inline-editing-byvalue
stratoula Sep 26, 2023
5d41f4f
Fix
stratoula Sep 27, 2023
062ae4a
Merge branch 'main' into inline-editing-byvalue
stratoula Sep 27, 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 @@ -75,6 +75,7 @@ export class EditPanelAction implements Action<ActionContext> {
const canEditEmbeddable = Boolean(
embeddable &&
embeddable.getOutput().editable &&
!embeddable.getOutput().inlineEditable &&
(embeddable.getOutput().editUrl ||
Copy link
Contributor Author

Choose a reason for hiding this comment

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

ℹ️ hides the embeddable edit action if inline editing exists

(embeddable.getOutput().editApp && embeddable.getOutput().editPath) ||
embeddable.getOutput().editableWithExplicitInput)
Expand Down
2 changes: 2 additions & 0 deletions src/plugins/embeddable/public/lib/embeddables/i_embeddable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ export interface EmbeddableOutput {
title?: string;
description?: string;
editable?: boolean;
// set this to true if the embeddable allows inline editing
inlineEditable?: boolean;
// Whether the embeddable can be edited inline by re-requesting the explicit input from the user
editableWithExplicitInput?: boolean;
savedObjectId?: string;
Expand Down
45 changes: 45 additions & 0 deletions src/plugins/unified_histogram/public/__mocks__/lens_adapters.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
/*
* 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 { UnifiedHistogramChartLoadEvent } from '../types';

export const lensAdaptersMock = {
tables: {
tables: {
default: {
columns: [
{
id: 'col-0-1',
meta: {
dimensionName: 'Slice size',
type: 'number',
},
name: 'Field 1',
},
{
id: 'col-0-2',
meta: {
dimensionName: 'Slice',
type: 'number',
},
name: 'Field 2',
},
],
rows: [
{
'col-0-1': 0,
'col-0-2': 0,
'col-0-3': 0,
'col-0-4': 0,
},
],
type: 'datatable',
},
},
},
} as unknown as UnifiedHistogramChartLoadEvent['adapters'];

This file was deleted.

23 changes: 16 additions & 7 deletions src/plugins/unified_histogram/public/chart/chart.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
*/

import React, { ReactElement, useMemo, useState, useEffect, useCallback, memo } from 'react';
import type { Observable } from 'rxjs';
import {
EuiButtonIcon,
EuiContextMenu,
Expand All @@ -17,8 +18,11 @@ import {
EuiProgress,
} from '@elastic/eui';
import { i18n } from '@kbn/i18n';
import type { EmbeddableComponentProps, Suggestion } from '@kbn/lens-plugin/public';
import type { Datatable } from '@kbn/expressions-plugin/common';
import type {
EmbeddableComponentProps,
Suggestion,
LensEmbeddableOutput,
} from '@kbn/lens-plugin/public';
import { DataView, DataViewField, DataViewType } from '@kbn/data-views-plugin/public';
import type { LensEmbeddableInput } from '@kbn/lens-plugin/public';
import type { AggregateQuery, Filter, Query, TimeRange } from '@kbn/es-query';
Expand Down Expand Up @@ -69,7 +73,8 @@ export interface ChartProps {
disableTriggers?: LensEmbeddableInput['disableTriggers'];
disabledActions?: LensEmbeddableInput['disabledActions'];
input$?: UnifiedHistogramInput$;
lensTablesAdapter?: Record<string, Datatable>;
lensAdapters?: UnifiedHistogramChartLoadEvent['adapters'];
Copy link
Contributor Author

@stratoula stratoula Sep 19, 2023

Choose a reason for hiding this comment

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

ℹ️ Instead of sending only the tables we now send all the adapters object to Lens flyout. With this way we can fetch correctly the active data. (Solves a bug currently in main more visible on this PR, check first bullet here

lensEmbeddableOutput$?: Observable<LensEmbeddableOutput>;
isOnHistogramMode?: boolean;
isChartLoading?: boolean;
onResetChartHeight?: () => void;
Expand All @@ -78,7 +83,10 @@ export interface ChartProps {
onBreakdownFieldChange?: (breakdownField: DataViewField | undefined) => void;
onSuggestionChange?: (suggestion: Suggestion | undefined) => void;
onTotalHitsChange?: (status: UnifiedHistogramFetchStatus, result?: number | Error) => void;
onChartLoad?: (event: UnifiedHistogramChartLoadEvent) => void;
onChartLoad?: (
event: UnifiedHistogramChartLoadEvent,
lensEmbeddableOutput$?: Observable<LensEmbeddableOutput>
) => void;
onFilter?: LensEmbeddableInput['onFilter'];
onBrushEnd?: LensEmbeddableInput['onBrushEnd'];
withDefaultActions: EmbeddableComponentProps['withDefaultActions'];
Expand Down Expand Up @@ -107,7 +115,8 @@ export function Chart({
disableTriggers,
disabledActions,
input$: originalInput$,
lensTablesAdapter,
lensAdapters,
lensEmbeddableOutput$,
isOnHistogramMode,
isChartLoading,
onResetChartHeight,
Expand Down Expand Up @@ -463,8 +472,8 @@ export function Chart({
{...{
services,
lensAttributesContext,
dataView,
lensTablesAdapter,
lensAdapters,
lensEmbeddableOutput$,
currentSuggestion,
isFlyoutVisible,
setIsFlyoutVisible,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import { act } from 'react-dom/test-utils';
import { setTimeout } from 'timers/promises';
import { dataViewWithTimefieldMock } from '../__mocks__/data_view_with_timefield';
import { unifiedHistogramServicesMock } from '../__mocks__/services';
import { lensTablesAdapterMock } from '../__mocks__/lens_table_adapter';
import { lensAdaptersMock } from '../__mocks__/lens_adapters';
import { ChartConfigPanel } from './chart_config_panel';
import type { LensAttributesContext } from './utils/get_lens_attributes';

Expand All @@ -34,7 +34,7 @@ describe('ChartConfigPanel', () => {
isFlyoutVisible: true,
setIsFlyoutVisible: jest.fn(),
isPlainRecord: true,
lensTablesAdapter: lensTablesAdapterMock,
lensAdapters: lensAdaptersMock,
query: {
esql: 'from test',
},
Expand Down
35 changes: 17 additions & 18 deletions src/plugins/unified_histogram/public/chart/chart_config_panel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,20 +6,20 @@
* Side Public License, v 1.
*/
import React, { useCallback, useEffect, useRef, useState } from 'react';
import type { Observable } from 'rxjs';
import type { AggregateQuery, Query } from '@kbn/es-query';
import { isEqual } from 'lodash';
import type { Suggestion } from '@kbn/lens-plugin/public';
import type { DataView } from '@kbn/data-views-plugin/public';
import type { LensEmbeddableOutput, Suggestion } from '@kbn/lens-plugin/public';
import type { Datatable } from '@kbn/expressions-plugin/common';

import type { UnifiedHistogramServices } from '../types';
import type { UnifiedHistogramServices, UnifiedHistogramChartLoadEvent } from '../types';
import type { LensAttributesContext } from './utils/get_lens_attributes';

export function ChartConfigPanel({
services,
lensAttributesContext,
dataView,
lensTablesAdapter,
lensAdapters,
lensEmbeddableOutput$,
currentSuggestion,
isFlyoutVisible,
setIsFlyoutVisible,
Expand All @@ -29,10 +29,10 @@ export function ChartConfigPanel({
}: {
services: UnifiedHistogramServices;
lensAttributesContext: LensAttributesContext;
dataView: DataView;
isFlyoutVisible: boolean;
setIsFlyoutVisible: (flag: boolean) => void;
lensTablesAdapter?: Record<string, Datatable>;
lensAdapters?: UnifiedHistogramChartLoadEvent['adapters'];
lensEmbeddableOutput$?: Observable<LensEmbeddableOutput>;
currentSuggestion?: Suggestion;
isPlainRecord?: boolean;
query?: Query | AggregateQuery;
Expand All @@ -49,26 +49,25 @@ export function ChartConfigPanel({
...(datasourceState && { datasourceState }),
...(visualizationState && { visualizationState }),
} as Suggestion;
if (!isEqual(updatedSuggestion, currentSuggestion)) {
onSuggestionChange?.(updatedSuggestion);
}
onSuggestionChange?.(updatedSuggestion);
},
[currentSuggestion, onSuggestionChange]
);

useEffect(() => {
const tablesAdapters = lensAdapters?.tables?.tables;
const dataHasChanged =
Boolean(lensTablesAdapter) &&
!isEqual(previousAdapters.current, lensTablesAdapter) &&
Boolean(tablesAdapters) &&
!isEqual(previousAdapters.current, tablesAdapters) &&
query !== previousQuery?.current;
async function fetchLensConfigComponent() {
const Component = await services.lens.EditLensConfigPanelApi();
const panel = (
<Component
attributes={lensAttributesContext.attributes}
dataView={dataView}
adaptersTables={lensTablesAdapter}
updateAll={updateSuggestion}
updatePanelState={updateSuggestion}
lensAdapters={lensAdapters}
output$={lensEmbeddableOutput$}
closeFlyout={() => {
setIsFlyoutVisible(false);
}}
Expand All @@ -78,7 +77,7 @@ export function ChartConfigPanel({
);
setEditLensConfigPanel(panel);
previousSuggestion.current = currentSuggestion;
previousAdapters.current = lensTablesAdapter;
previousAdapters.current = tablesAdapters;
if (dataHasChanged) {
previousQuery.current = query;
}
Expand All @@ -92,14 +91,14 @@ export function ChartConfigPanel({
}, [
lensAttributesContext.attributes,
services.lens,
dataView,
updateSuggestion,
isPlainRecord,
currentSuggestion,
query,
isFlyoutVisible,
lensTablesAdapter,
setIsFlyoutVisible,
lensAdapters,
lensEmbeddableOutput$,
]);

return isPlainRecord ? editLensConfigPanel : null;
Expand Down
16 changes: 9 additions & 7 deletions src/plugins/unified_histogram/public/chart/histogram.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import { mountWithIntl } from '@kbn/test-jest-helpers';
import { Histogram } from './histogram';
import React from 'react';
import { of } from 'rxjs';
import { unifiedHistogramServicesMock } from '../__mocks__/services';
import { dataViewWithTimefieldMock } from '../__mocks__/data_view_with_timefield';
import { createDefaultInspectorAdapters } from '@kbn/expressions-plugin/common';
Expand Down Expand Up @@ -166,24 +167,25 @@ describe('Histogram', () => {
jest
.spyOn(adapters.requests, 'getRequests')
.mockReturnValue([{ response: { json: { rawResponse } } } as any]);
onLoad(true, undefined);
const embeddableOutput$ = jest.fn().mockReturnValue(of('output$'));
onLoad(true, undefined, embeddableOutput$);
expect(props.onTotalHitsChange).toHaveBeenLastCalledWith(
UnifiedHistogramFetchStatus.loading,
undefined
);
expect(props.onChartLoad).toHaveBeenLastCalledWith({ adapters: {} });
expect(props.onChartLoad).toHaveBeenLastCalledWith({ adapters: {} }, embeddableOutput$);
expect(buildBucketInterval.buildBucketInterval).not.toHaveBeenCalled();
expect(useTimeRange.useTimeRange).toHaveBeenLastCalledWith(
expect.objectContaining({ bucketInterval: undefined })
);
act(() => {
onLoad(false, adapters);
onLoad(false, adapters, embeddableOutput$);
});
expect(props.onTotalHitsChange).toHaveBeenLastCalledWith(
UnifiedHistogramFetchStatus.complete,
100
);
expect(props.onChartLoad).toHaveBeenLastCalledWith({ adapters });
expect(props.onChartLoad).toHaveBeenLastCalledWith({ adapters }, embeddableOutput$);
expect(buildBucketInterval.buildBucketInterval).toHaveBeenCalled();
expect(useTimeRange.useTimeRange).toHaveBeenLastCalledWith(
expect.objectContaining({ bucketInterval: mockBucketInterval })
Expand Down Expand Up @@ -236,7 +238,7 @@ describe('Histogram', () => {
UnifiedHistogramFetchStatus.complete,
100
);
expect(props.onChartLoad).toHaveBeenLastCalledWith({ adapters });
expect(props.onChartLoad).toHaveBeenLastCalledWith({ adapters }, undefined);
});

it('should execute onLoad correctly for textbased language and no Lens suggestions', async () => {
Expand Down Expand Up @@ -272,7 +274,7 @@ describe('Histogram', () => {
UnifiedHistogramFetchStatus.complete,
20
);
expect(props.onChartLoad).toHaveBeenLastCalledWith({ adapters });
expect(props.onChartLoad).toHaveBeenLastCalledWith({ adapters }, undefined);
});

it('should execute onLoad correctly for textbased language and Lens suggestions', async () => {
Expand Down Expand Up @@ -308,6 +310,6 @@ describe('Histogram', () => {
UnifiedHistogramFetchStatus.complete,
2
);
expect(props.onChartLoad).toHaveBeenLastCalledWith({ adapters });
expect(props.onChartLoad).toHaveBeenLastCalledWith({ adapters }, undefined);
});
});
19 changes: 15 additions & 4 deletions src/plugins/unified_histogram/public/chart/histogram.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,11 @@ import type { DefaultInspectorAdapters, Datatable } from '@kbn/expressions-plugi
import type { IKibanaSearchResponse } from '@kbn/data-plugin/public';
import type { estypes } from '@elastic/elasticsearch';
import type { TimeRange } from '@kbn/es-query';
import type { EmbeddableComponentProps, LensEmbeddableInput } from '@kbn/lens-plugin/public';
import type {
EmbeddableComponentProps,
LensEmbeddableInput,
LensEmbeddableOutput,
} from '@kbn/lens-plugin/public';
import { RequestStatus } from '@kbn/inspector-plugin/public';
import type { Observable } from 'rxjs';
import {
Expand Down Expand Up @@ -47,7 +51,10 @@ export interface HistogramProps {
disableTriggers?: LensEmbeddableInput['disableTriggers'];
disabledActions?: LensEmbeddableInput['disabledActions'];
onTotalHitsChange?: (status: UnifiedHistogramFetchStatus, result?: number | Error) => void;
onChartLoad?: (event: UnifiedHistogramChartLoadEvent) => void;
onChartLoad?: (
event: UnifiedHistogramChartLoadEvent,
lensEmbeddableOutput$?: Observable<LensEmbeddableOutput>
) => void;
davismcphee marked this conversation as resolved.
Show resolved Hide resolved
onFilter?: LensEmbeddableInput['onFilter'];
onBrushEnd?: LensEmbeddableInput['onBrushEnd'];
withDefaultActions: EmbeddableComponentProps['withDefaultActions'];
Expand Down Expand Up @@ -118,7 +125,11 @@ export function Histogram({
}, [attributes, containerHeight, containerWidth]);

const onLoad = useStableCallback(
(isLoading: boolean, adapters: Partial<DefaultInspectorAdapters> | undefined) => {
(
isLoading: boolean,
adapters: Partial<DefaultInspectorAdapters> | undefined,
lensEmbeddableOutput$?: Observable<LensEmbeddableOutput>
) => {
const lensRequest = adapters?.requests?.getRequests()[0];
const requestFailed = lensRequest?.status === RequestStatus.ERROR;
const json = lensRequest?.response?.json as
Expand Down Expand Up @@ -155,7 +166,7 @@ export function Histogram({
setBucketInterval(newBucketInterval);
}

onChartLoad?.({ adapters: adapters ?? {} });
onChartLoad?.({ adapters: adapters ?? {} }, lensEmbeddableOutput$);
}
);

Expand Down
Loading