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] Add toolbar api #69263

Merged
merged 20 commits into from
Jun 26, 2020
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
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 @@ -10,11 +10,15 @@
.lnsWorkspacePanelWrapper__pageContentHeader {
@include euiTitle('xs');
padding: $euiSizeM;
border-bottom: $euiBorderThin;
// override EuiPage
margin-bottom: 0 !important; // sass-lint:disable-line no-important
}

.lnsWorkspacePanelWrapper__pageContentHeader--unsaved {
@include euiTitle('xxs');
flash1293 marked this conversation as resolved.
Show resolved Hide resolved
color: $euiTextSubduedColor;
}

.lnsWorkspacePanelWrapper__pageContentBody {
@include euiScrollBar;
flex-grow: 1;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,17 @@ export function EditorFrame(props: EditorFrameProps) {
}
workspacePanel={
allLoaded && (
<WorkspacePanelWrapper title={state.title}>
<WorkspacePanelWrapper
title={state.title}
framePublicAPI={framePublicAPI}
dispatch={dispatch}
visualizationState={state.visualization.state}
activeVisualization={
state.visualization.activeId
? props.visualizationMap[state.visualization.activeId]
: undefined
}
>
<WorkspacePanel
activeDatasourceId={state.activeDatasourceId}
activeVisualizationId={state.visualization.activeId}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import React from 'react';
import { Visualization } from '../../types';
import { createMockVisualization, createMockFramePublicAPI, FrameMock } from '../mocks';
import { mountWithIntl as mount } from 'test_utils/enzyme_helpers';
import { ReactWrapper } from 'enzyme';
import { WorkspacePanelWrapper, WorkspacePanelWrapperProps } from './workspace_panel_wrapper';

describe('workspace_panel_wrapper', () => {
let mockVisualization: jest.Mocked<Visualization>;
let mockFrameAPI: FrameMock;
let instance: ReactWrapper<WorkspacePanelWrapperProps>;

beforeEach(() => {
mockVisualization = createMockVisualization();
mockFrameAPI = createMockFramePublicAPI();
});

afterEach(() => {
instance.unmount();
});

it('should render its children', () => {
const MyChild = () => <span>The child elements</span>;
instance = mount(
<WorkspacePanelWrapper
dispatch={jest.fn()}
framePublicAPI={mockFrameAPI}
visualizationState={{}}
activeVisualization={mockVisualization}
>
<MyChild />
</WorkspacePanelWrapper>
);

expect(instance.find(MyChild)).toHaveLength(1);
});

it('should call the toolbar renderer if provided', () => {
const renderToolbarMock = jest.fn();
const visState = { internalState: 123 };
instance = mount(
<WorkspacePanelWrapper
dispatch={jest.fn()}
framePublicAPI={mockFrameAPI}
visualizationState={visState}
children={<span />}
activeVisualization={{ ...mockVisualization, renderToolbar: renderToolbarMock }}
/>
);

expect(renderToolbarMock).toHaveBeenCalledWith(expect.anything(), {
Copy link
Contributor

Choose a reason for hiding this comment

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

I usually do expect.any(Element)

state: visState,
frame: mockFrameAPI,
setState: expect.anything(),
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -4,25 +4,82 @@
* you may not use this file except in compliance with the Elastic License.
*/

import React from 'react';
import { EuiPageContent, EuiPageContentHeader, EuiPageContentBody } from '@elastic/eui';
import React, { useCallback } from 'react';
import { i18n } from '@kbn/i18n';
import classNames from 'classnames';
import {
EuiPageContent,
EuiPageContentBody,
EuiPageContentHeader,
EuiFlexGroup,
EuiFlexItem,
} from '@elastic/eui';
import { FramePublicAPI, Visualization } from '../../types';
import { NativeRenderer } from '../../native_renderer';
import { Action } from './state_management';

interface Props {
title: string;
export interface WorkspacePanelWrapperProps {
children: React.ReactNode | React.ReactNode[];
framePublicAPI: FramePublicAPI;
visualizationState: unknown;
activeVisualization?: Visualization;
dispatch: (action: Action) => void;
title?: string;
}

export function WorkspacePanelWrapper({ children, title }: Props) {
export function WorkspacePanelWrapper({
children,
framePublicAPI,
visualizationState,
activeVisualization,
dispatch,
title,
}: WorkspacePanelWrapperProps) {
const setVisualizationState = useCallback(
(newState: unknown) => {
if (!activeVisualization) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add activeVisualization to dependency array of useCallback?

return;
}
dispatch({
type: 'UPDATE_VISUALIZATION_STATE',
visualizationId: activeVisualization.id,
newState,
clearStagedPreview: false,
});
},
[dispatch]
);
return (
<EuiPageContent className="lnsWorkspacePanelWrapper">
{title && (
<EuiPageContentHeader className="lnsWorkspacePanelWrapper__pageContentHeader">
<span data-test-subj="lns_ChartTitle">{title}</span>
</EuiPageContentHeader>
cchaos marked this conversation as resolved.
Show resolved Hide resolved
<EuiFlexGroup gutterSize="s" direction="column" alignItems="stretch">
{activeVisualization && activeVisualization.renderToolbar && (
<EuiFlexItem grow={false}>
<NativeRenderer
render={activeVisualization.renderToolbar}
nativeProps={{
frame: framePublicAPI,
state: visualizationState,
setState: setVisualizationState,
}}
/>
</EuiFlexItem>
)}
<EuiPageContentBody className="lnsWorkspacePanelWrapper__pageContentBody">
{children}
</EuiPageContentBody>
</EuiPageContent>
<EuiFlexItem>
<EuiPageContent className="lnsWorkspacePanelWrapper">
<EuiPageContentHeader
className={classNames('lnsWorkspacePanelWrapper__pageContentHeader', {
'lnsWorkspacePanelWrapper__pageContentHeader--unsaved': !title,
})}
>
<span data-test-subj="lns_ChartTitle">
{title ||
i18n.translate('xpack.lens.chartTitle.unsaved', { defaultMessage: 'Unsaved' })}
</span>
</EuiPageContentHeader>
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we make sure the content header only shows when there's a chart? So don't show "Unsaved" in the empty state (alongside the graphic).

Copy link
Contributor

Choose a reason for hiding this comment

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

@flash1293 It is very important and pretty similar to it used to behave. Before, it would show the title component once you have a visualization and then saved it. Now I'm just asking if we can put a second layer of logic to show the "Unsaved" title after chart creation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@cchaos I withdrew my comment, I found a nice solution :) Sorry for the noise.

<EuiPageContentBody className="lnsWorkspacePanelWrapper__pageContentBody">
{children}
</EuiPageContentBody>
</EuiPageContent>
</EuiFlexItem>
</EuiFlexGroup>
);
}
11 changes: 11 additions & 0 deletions x-pack/plugins/lens/public/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,12 @@ export type VisualizationLayerWidgetProps<T = unknown> = VisualizationConfigProp
setState: (newState: T) => void;
};

export interface VisualizationToolbarProps<T = unknown> {
setState: (newState: T) => void;
frame: FramePublicAPI;
state: T;
}

export type VisualizationDimensionEditorProps<T = unknown> = VisualizationConfigProps<T> & {
groupId: string;
accessor: string;
Expand Down Expand Up @@ -454,6 +460,11 @@ export interface Visualization<T = unknown, P = unknown> {
* for extra configurability, such as for styling the legend or axis
*/
renderLayerContextMenu?: (domElement: Element, props: VisualizationLayerWidgetProps<T>) => void;
/**
* Toolbar rendered above the visualization. This is meant to be used to provide chart-level
* settings for the visualization.
*/
renderToolbar?: (domElement: Element, props: VisualizationToolbarProps<T>) => void;
/**
* Visualizations can provide a custom icon which will open a layer-specific popover
* If no icon is provided, gear icon is default
Expand Down
3 changes: 2 additions & 1 deletion x-pack/test/functional/page_objects/lens_page.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ export function LensPageProvider({ getService, getPageObjects }: FtrProviderCont
const testSubjects = getService('testSubjects');
const retry = getService('retry');
const find = getService('find');
const globalNav = getService('globalNav');
const comboBox = getService('comboBox');
const PageObjects = getPageObjects([
'header',
Expand Down Expand Up @@ -164,7 +165,7 @@ export function LensPageProvider({ getService, getPageObjects }: FtrProviderCont
},

getTitle() {
return testSubjects.getVisibleText('lns_ChartTitle');
return globalNav.getLastBreadcrumb();
},
});
}