Skip to content

Commit

Permalink
[TextBased] Allow inline editing from dashboards (#161146)
Browse files Browse the repository at this point in the history
## Summary

Part of #158802

For panels created from Discover with text based languages, not navigate
to Lens but open a flyout instead.


![sql](https://github.com/elastic/kibana/assets/17003240/489402ac-dcdb-468b-89b9-a84f4c4f2ca5)


Follow up PR: 
- Remove the SQL option from Lens dataview picker and move the FTs in
Discover/Dashboard

Note:
- Changing the query on the dashboard level is going to be added in 8.11

### 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)
- [ ] [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 <42973632+kibanamachine@users.noreply.github.com>
  • Loading branch information
stratoula and kibanamachine committed Jul 7, 2023
1 parent 3495720 commit fde9539
Show file tree
Hide file tree
Showing 19 changed files with 467 additions and 69 deletions.
2 changes: 1 addition & 1 deletion packages/kbn-optimizer/limits.yml
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ pageLoadAssetSize:
kibanaUsageCollection: 16463
kibanaUtils: 79713
kubernetesSecurity: 77234
lens: 37000
lens: 38000
licenseManagement: 41817
licensing: 29004
lists: 22900
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ export function FiltersNotificationPopover({
}: FiltersNotificationProps) {
const { embeddable } = context;
const [isPopoverOpen, setIsPopoverOpen] = useState(false);
const [disableEditbutton, setDisableEditButton] = useState(false);

return (
<EuiPopover
Expand All @@ -57,26 +58,31 @@ export function FiltersNotificationPopover({
anchorPosition="upCenter"
>
<EuiPopoverTitle>{displayName}</EuiPopoverTitle>
<FiltersNotificationPopoverContents context={context} />
<FiltersNotificationPopoverContents
context={context}
setDisableEditButton={setDisableEditButton}
/>
<EuiPopoverFooter>
<EuiFlexGroup
gutterSize="s"
alignItems="center"
justifyContent="flexEnd"
responsive={false}
wrap={true}
>
<EuiFlexItem grow={false}>
<EuiButton
data-test-subj={'filtersNotificationModal__editButton'}
size="s"
fill
onClick={() => editPanelAction.execute({ embeddable })}
>
{dashboardFilterNotificationActionStrings.getEditButtonTitle()}
</EuiButton>
</EuiFlexItem>
</EuiFlexGroup>
{!disableEditbutton && (
<EuiFlexGroup
gutterSize="s"
alignItems="center"
justifyContent="flexEnd"
responsive={false}
wrap={true}
>
<EuiFlexItem grow={false}>
<EuiButton
data-test-subj={'filtersNotificationModal__editButton'}
size="s"
fill
onClick={() => editPanelAction.execute({ embeddable })}
>
{dashboardFilterNotificationActionStrings.getEditButtonTitle()}
</EuiButton>
</EuiFlexItem>
</EuiFlexGroup>
)}
</EuiPopoverFooter>
</EuiPopover>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,13 @@ import { DashboardContainer } from '../dashboard_container/embeddable/dashboard_

export interface FiltersNotificationProps {
context: FiltersNotificationActionContext;
setDisableEditButton: (flag: boolean) => void;
}

export function FiltersNotificationPopoverContents({ context }: FiltersNotificationProps) {
export function FiltersNotificationPopoverContents({
context,
setDisableEditButton,
}: FiltersNotificationProps) {
const { embeddable } = context;
const [isLoading, setIsLoading] = useState(true);
const [filters, setFilters] = useState<Filter[]>([]);
Expand All @@ -53,6 +57,7 @@ export function FiltersNotificationPopoverContents({ context }: FiltersNotificat
const language = getAggregateQueryMode(embeddableQuery);
setQueryLanguage(language);
setQueryString(embeddableQuery[language as keyof AggregateQuery]);
setDisableEditButton(true);
}
}
setIsLoading(false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,10 @@ export function useChartConfigPanel({
dataView={dataView}
adaptersTables={lensTablesAdapter}
updateAll={updateSuggestion}
setIsFlyoutVisible={setIsFlyoutVisible}
closeFlyout={() => {
setIsFlyoutVisible(false);
}}
wrapInFlyout
datasourceId="textBased"
/>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -740,6 +740,43 @@ describe('getLensAttributes', () => {
`);
});

it('should return correct attributes for text based languages with adhoc dataview', () => {
const adHocDataview = {
...dataView,
isPersisted: () => false,
} as DataView;
const lensAttrs = getLensAttributes({
title: 'test',
filters,
query,
dataView: adHocDataview,
timeInterval,
breakdownField: undefined,
suggestion: currentSuggestionMock,
});
expect(lensAttrs.attributes).toEqual({
state: expect.objectContaining({
adHocDataViews: {
'index-pattern-with-timefield-id': {},
},
}),
references: [
{
id: 'index-pattern-with-timefield-id',
name: 'indexpattern-datasource-current-indexpattern',
type: 'index-pattern',
},
{
id: 'index-pattern-with-timefield-id',
name: 'indexpattern-datasource-layer-unifiedHistogram',
type: 'index-pattern',
},
],
title: 'test',
visualizationType: 'lnsHeatmap',
});
});

it('should return suggestion title if no title is given', () => {
expect(
getLensAttributes({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,13 @@ export const getLensAttributes = ({
filters,
query,
visualization,
...(dataView &&
dataView.id &&
!dataView.isPersisted() && {
adHocDataViews: {
[dataView.id]: dataView.toSpec(false),
},
}),
},
visualizationType: suggestion ? suggestion.visualizationId : 'lnsXY',
} as TypedLensByValueInput['attributes'];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,8 @@ export function getEditLensConfiguration(
attributes,
dataView,
updateAll,
setIsFlyoutVisible,
closeFlyout,
wrapInFlyout,
datasourceId,
adaptersTables,
}: EditLensConfigurationProps) => {
Expand Down Expand Up @@ -88,15 +89,38 @@ export function getEditLensConfiguration(
const lensStore: LensRootStore = makeConfigureStore(storeDeps, {
lens: getPreloadedState(storeDeps) as LensAppState,
} as unknown as PreloadedState<LensState>);
const closeFlyout = () => {
setIsFlyoutVisible?.(false);

const getWrapper = (children: JSX.Element) => {
if (wrapInFlyout) {
return (
<EuiFlyout
type="push"
ownFocus
onClose={() => {
closeFlyout?.();
}}
aria-labelledby={i18n.translate('xpack.lens.config.editLabel', {
defaultMessage: 'Edit configuration',
})}
size="s"
hideCloseButton
css={css`
background: none;
`}
>
{children}
</EuiFlyout>
);
} else {
return children;
}
};

const configPanelProps = {
attributes,
dataView,
updateAll,
setIsFlyoutVisible,
closeFlyout,
datasourceId,
adaptersTables,
coreStart,
Expand All @@ -105,25 +129,10 @@ export function getEditLensConfiguration(
datasourceMap,
};

return (
<EuiFlyout
type="push"
ownFocus
onClose={closeFlyout}
aria-labelledby={i18n.translate('xpack.lens.config.editLabel', {
defaultMessage: 'Edit configuration',
})}
size="s"
className="lnsEditConfigurationFlyout"
css={css`
background: none;
`}
hideCloseButton
>
<Provider store={lensStore}>
<LensEditConfigurationFlyout {...configPanelProps} />
</Provider>
</EuiFlyout>
return getWrapper(
<Provider store={lensStore}>
<LensEditConfigurationFlyout {...configPanelProps} />
</Provider>
);
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -120,22 +120,22 @@ describe('LensEditConfigurationFlyout', () => {
startDependencies,
visualizationMap,
datasourceMap,
setIsFlyoutVisible: jest.fn(),
closeFlyout: jest.fn(),
datasourceId: 'testDatasource',
} as unknown as EditConfigPanelProps;
}

it('should call the setIsFlyout callback if collapse button is clicked', async () => {
const setIsFlyoutVisibleSpy = jest.fn();
it('should call the closeFlyout callback if collapse button is clicked', async () => {
const closeFlyoutSpy = jest.fn();
const props = getDefaultProps();
const newProps = {
...props,
setIsFlyoutVisible: setIsFlyoutVisibleSpy,
closeFlyout: closeFlyoutSpy,
};
const { instance } = await prepareAndMountComponent(newProps);
expect(instance.find(EuiFlyoutBody).exists()).toBe(true);
instance.find('[data-test-subj="collapseFlyoutButton"]').at(1).simulate('click');
expect(setIsFlyoutVisibleSpy).toHaveBeenCalled();
expect(closeFlyoutSpy).toHaveBeenCalled();
});

it('should compute the frame public api correctly', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@ export interface EditConfigPanelProps {
startDependencies: LensPluginStartDependencies;
visualizationMap: VisualizationMap;
datasourceMap: DatasourceMap;
setIsFlyoutVisible?: (flag: boolean) => void;
closeFlyout?: () => void;
wrapInFlyout?: boolean;
datasourceId: 'formBased' | 'textBased';
adaptersTables?: Record<string, Datatable>;
}
Expand All @@ -57,7 +58,7 @@ export function LensEditConfigurationFlyout({
datasourceMap,
datasourceId,
updateAll,
setIsFlyoutVisible,
closeFlyout,
adaptersTables,
}: EditConfigPanelProps) {
const currentDataViewId = dataView.id ?? '';
Expand Down Expand Up @@ -112,10 +113,6 @@ export function LensEditConfigurationFlyout({
};
}, [activeData, dataViews, datasourceLayers, dateRange]);

const closeFlyout = () => {
setIsFlyoutVisible?.(false);
};

const layerPanelsProps = {
framePublicAPI,
datasourceMap,
Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/lens/public/async_services.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,3 +50,4 @@ export * from './app_plugin/save_modal_container';
export * from './chart_info_api';

export * from './trigger_actions/open_in_discover_helpers';
export * from './trigger_actions/open_lens_config/helpers';

0 comments on commit fde9539

Please sign in to comment.