Skip to content

Commit

Permalink
[Text based] Configure Lens suggestion on the fly from Discover (#159559
Browse files Browse the repository at this point in the history
)

## Summary

Part of #158802

This PR removes the navigation from Discover to Lens and renders a push
flyout instead.


![textbased](https://github.com/elastic/kibana/assets/17003240/92c6f290-6cf9-4daa-920e-f1409595d765)


Next tasks (follow-up PRs):

- [ ] Remove the text based support from Lens dataview picker. The FTs
should be removed from there and possibly moved to discover FTs
- [ ] Apply the same flyout in dashboard for text based panels
- [ ] Allow drag and drop between dimensions
- [ ] Investigate why the Field select doesnt close when you click
outside the dropdown

### Checklist

- [ ] Any text added follows [EUI's writing
guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses
sentence case text and includes [i18n
support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)
- [ ]
[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials
- [ ] [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
- [ ] Any UI touched in this PR is usable by keyboard only (learn more
about [keyboard accessibility](https://webaim.org/techniques/keyboard/))
- [ ] 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))
- [ ] 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))
- [ ] This was checked for [cross-browser
compatibility](https://www.elastic.co/support/matrix#matrix_browsers)

---------

Co-authored-by: kibanamachine <[email protected]>
Co-authored-by: Andrea Del Rio <[email protected]>
  • Loading branch information
3 people authored Jul 3, 2023
1 parent 90b3e71 commit e8b2303
Show file tree
Hide file tree
Showing 43 changed files with 1,477 additions and 92 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
/*
* 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 type { Datatable } from '@kbn/expressions-plugin/common';

export const lensTablesAdapterMock: Record<string, Datatable> = {
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',
},
};
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

import React from 'react';
import { dataPluginMock } from '@kbn/data-plugin/public/mocks';
import { expressionsPluginMock } from '@kbn/expressions-plugin/public/mocks';
import { fieldFormatsMock } from '@kbn/field-formats-plugin/common/mocks';
Expand Down Expand Up @@ -33,6 +33,7 @@ export const unifiedHistogramServicesMock = {
suggestions: jest.fn(() => allSuggestionsMock),
};
}),
EditLensConfigPanelApi: jest.fn().mockResolvedValue(<span>Lens Config Panel Component</span>),
},
storage: {
get: jest.fn(),
Expand Down
11 changes: 11 additions & 0 deletions src/plugins/unified_histogram/public/chart/chart.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,17 @@ describe('Chart', () => {
expect(component.find(SuggestionSelector).exists()).toBeTruthy();
});

it('should render the edit on the fly button when chart is visible and suggestions exist', async () => {
const component = await mountComponent({
currentSuggestion: currentSuggestionMock,
allSuggestions: allSuggestionsMock,
isPlainRecord: true,
});
expect(
component.find('[data-test-subj="unifiedHistogramEditFlyoutVisualization"]').exists()
).toBeTruthy();
});

it('should render the save button when chart is visible and suggestions exist', async () => {
const component = await mountComponent({
currentSuggestion: currentSuggestionMock,
Expand Down
76 changes: 72 additions & 4 deletions src/plugins/unified_histogram/public/chart/chart.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,7 @@
* Side Public License, v 1.
*/

import { ReactElement, useMemo, useState } from 'react';
import React, { memo } from 'react';
import React, { ReactElement, useMemo, useState, useEffect, useCallback, memo } from 'react';
import {
EuiButtonIcon,
EuiContextMenu,
Expand All @@ -18,6 +17,7 @@ import {
} from '@elastic/eui';
import { i18n } from '@kbn/i18n';
import type { Suggestion } from '@kbn/lens-plugin/public';
import type { Datatable } from '@kbn/expressions-plugin/common';
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 All @@ -42,6 +42,7 @@ import { useTotalHits } from './hooks/use_total_hits';
import { useRequestParams } from './hooks/use_request_params';
import { useChartStyles } from './hooks/use_chart_styles';
import { useChartActions } from './hooks/use_chart_actions';
import { useChartConfigPanel } from './hooks/use_chart_config_panel';
import { getLensAttributes } from './utils/get_lens_attributes';
import { useRefetch } from './hooks/use_refetch';
import { useEditVisualization } from './hooks/use_edit_visualization';
Expand All @@ -67,6 +68,7 @@ export interface ChartProps {
disableTriggers?: LensEmbeddableInput['disableTriggers'];
disabledActions?: LensEmbeddableInput['disabledActions'];
input$?: UnifiedHistogramInput$;
lensTablesAdapter?: Record<string, Datatable>;
onResetChartHeight?: () => void;
onChartHiddenChange?: (chartHidden: boolean) => void;
onTimeIntervalChange?: (timeInterval: string) => void;
Expand Down Expand Up @@ -101,6 +103,7 @@ export function Chart({
disableTriggers,
disabledActions,
input$: originalInput$,
lensTablesAdapter,
onResetChartHeight,
onChartHiddenChange,
onTimeIntervalChange,
Expand All @@ -112,6 +115,7 @@ export function Chart({
onBrushEnd,
}: ChartProps) {
const [isSaveModalVisible, setIsSaveModalVisible] = useState(false);
const [isFlyoutVisible, setIsFlyoutVisible] = useState(false);
const {
showChartOptionsPopover,
chartRef,
Expand Down Expand Up @@ -190,6 +194,7 @@ export function Chart({
histogramCss,
breakdownFieldSelectorGroupCss,
breakdownFieldSelectorItemCss,
suggestionsSelectorItemCss,
chartToolButtonCss,
} = useChartStyles(chartVisible);

Expand All @@ -215,6 +220,34 @@ export function Chart({
]
);

const ChartConfigPanel = useChartConfigPanel({
services,
lensAttributesContext,
dataView,
lensTablesAdapter,
currentSuggestion,
isFlyoutVisible,
setIsFlyoutVisible,
isPlainRecord,
query: originalQuery,
onSuggestionChange,
});

const onSuggestionSelectorChange = useCallback(
(s: Suggestion | undefined) => {
onSuggestionChange?.(s);
},
[onSuggestionChange]
);

useEffect(() => {
// close the flyout for dataview mode
// or if no chart is visible
if (!chartVisible && isFlyoutVisible) {
setIsFlyoutVisible(false);
}
}, [chartVisible, isFlyoutVisible]);

const onEditVisualization = useEditVisualization({
services,
dataView,
Expand All @@ -226,6 +259,24 @@ export function Chart({
const canSaveVisualization =
chartVisible && currentSuggestion && services.capabilities.dashboard?.showWriteControls;

const renderEditButton = useMemo(
() => (
<EuiButtonIcon
size="xs"
iconType="pencil"
onClick={() => setIsFlyoutVisible(true)}
data-test-subj="unifiedHistogramEditFlyoutVisualization"
aria-label={i18n.translate('unifiedHistogram.editVisualizationButton', {
defaultMessage: 'Edit visualization',
})}
disabled={isFlyoutVisible}
/>
),
[isFlyoutVisible]
);

const canEditVisualizationOnTheFly = isPlainRecord && chartVisible;

return (
<EuiFlexGroup
className={className}
Expand All @@ -237,6 +288,7 @@ export function Chart({
<EuiFlexItem grow={false} css={resultCountCss}>
<EuiFlexGroup
justifyContent="spaceBetween"
alignItems="center"
gutterSize="none"
responsive={false}
css={resultCountInnerCss}
Expand Down Expand Up @@ -267,11 +319,11 @@ export function Chart({
</EuiFlexItem>
)}
{chartVisible && currentSuggestion && allSuggestions && allSuggestions?.length > 1 && (
<EuiFlexItem css={breakdownFieldSelectorItemCss}>
<EuiFlexItem css={suggestionsSelectorItemCss}>
<SuggestionSelector
suggestions={allSuggestions}
activeSuggestion={currentSuggestion}
onSuggestionChange={onSuggestionChange}
onSuggestionChange={onSuggestionSelectorChange}
/>
</EuiFlexItem>
)}
Expand All @@ -296,6 +348,21 @@ export function Chart({
</EuiFlexItem>
</>
)}
{canEditVisualizationOnTheFly && (
<EuiFlexItem grow={false} css={chartToolButtonCss}>
{!isFlyoutVisible ? (
<EuiToolTip
content={i18n.translate('unifiedHistogram.editVisualizationButton', {
defaultMessage: 'Edit visualization',
})}
>
{renderEditButton}
</EuiToolTip>
) : (
renderEditButton
)}
</EuiFlexItem>
)}
{onEditVisualization && (
<EuiFlexItem grow={false} css={chartToolButtonCss}>
<EuiToolTip
Expand Down Expand Up @@ -387,6 +454,7 @@ export function Chart({
isSaveable={false}
/>
)}
{isFlyoutVisible && ChartConfigPanel}
</EuiFlexGroup>
);
}
1 change: 1 addition & 0 deletions src/plugins/unified_histogram/public/chart/histogram.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ export function Histogram({
const chartCss = css`
position: relative;
flex-grow: 1;
margin-block: ${euiTheme.size.xs};
& > div {
height: 100%;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
/*
* 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 type { TypedLensByValueInput } from '@kbn/lens-plugin/public';
import { renderHook } from '@testing-library/react-hooks';
import { act } from 'react-test-renderer';
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 { useChartConfigPanel } from './use_chart_config_panel';
import type { LensAttributesContext } from '../utils/get_lens_attributes';

describe('useChartConfigPanel', () => {
it('should return a jsx element to edit the visualization', async () => {
const lensAttributes = {
visualizationType: 'lnsXY',
title: 'test',
} as TypedLensByValueInput['attributes'];
const hook = renderHook(() =>
useChartConfigPanel({
services: unifiedHistogramServicesMock,
dataView: dataViewWithTimefieldMock,
lensAttributesContext: {
attributes: lensAttributes,
} as unknown as LensAttributesContext,
isFlyoutVisible: true,
setIsFlyoutVisible: jest.fn(),
isPlainRecord: true,
lensTablesAdapter: lensTablesAdapterMock,
query: {
sql: 'Select * from test',
},
})
);
await act(() => setTimeout(0));
expect(hook.result.current).toBeDefined();
expect(hook.result.current).not.toBeNull();
});

it('should return null if not in text based mode', async () => {
const lensAttributes = {
visualizationType: 'lnsXY',
title: 'test',
} as TypedLensByValueInput['attributes'];
const hook = renderHook(() =>
useChartConfigPanel({
services: unifiedHistogramServicesMock,
dataView: dataViewWithTimefieldMock,
lensAttributesContext: {
attributes: lensAttributes,
} as unknown as LensAttributesContext,
isFlyoutVisible: true,
setIsFlyoutVisible: jest.fn(),
isPlainRecord: false,
})
);
await act(() => setTimeout(0));
expect(hook.result.current).toBeNull();
});
});
Loading

0 comments on commit e8b2303

Please sign in to comment.