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

[7.x] [Expressions] Align renderMode with the embeddable viewMode (#110199) #111186

Merged
merged 1 commit into from
Sep 3, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions src/plugins/embeddable/common/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { PersistableStateService } from '../../kibana_utils/common';

export enum ViewMode {
EDIT = 'edit',
PREVIEW = 'preview',
VIEW = 'view',
}

Expand Down
11 changes: 8 additions & 3 deletions src/plugins/expressions/common/expression_renderers/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,12 +53,11 @@ export type AnyExpressionRenderDefinition = ExpressionRenderDefinition<any>;
* This value can be set from a consumer embedding an expression renderer and is accessible
* from within the active render function as part of the handlers.
* The following modes are supported:
* * display (default): The chart is rendered in a container with the main purpose of viewing the chart (e.g. in a container like dashboard or canvas)
* * view (default): The chart is rendered in a container with the main purpose of viewing the chart (e.g. in a container like dashboard or canvas)
* * preview: The chart is rendered in very restricted space (below 100px width and height) and should only show a rough outline
* * edit: The chart is rendered within an editor and configuration elements within the chart should be displayed
* * noInteractivity: The chart is rendered in a non-interactive environment and should not provide any affordances for interaction like brushing
*/
export type RenderMode = 'noInteractivity' | 'edit' | 'preview' | 'display';
export type RenderMode = 'edit' | 'preview' | 'view';

export interface IInterpreterRenderHandlers {
/**
Expand All @@ -71,6 +70,12 @@ export interface IInterpreterRenderHandlers {
event: (event: any) => void;
hasCompatibleActions?: (event: any) => Promise<boolean>;
getRenderMode: () => RenderMode;

/**
* The chart is rendered in a non-interactive environment and should not provide any affordances for interaction like brushing.
*/
isInteractive: () => boolean;

isSyncColorsEnabled: () => boolean;
/**
* This uiState interface is actually `PersistedState` from the visualizations plugin,
Expand Down
1 change: 1 addition & 0 deletions src/plugins/expressions/public/loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ export class ExpressionLoader {
);

this.renderHandler = new ExpressionRenderHandler(element, {
interactive: params?.interactive,
onRenderError: params && params.onRenderError,
renderMode: params?.renderMode,
syncColors: params?.syncColors,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,7 @@ export const ReactExpressionRenderer = ({
}, [
hasCustomRenderErrorHandler,
onEvent,
expressionLoaderOptions.interactive,
expressionLoaderOptions.renderMode,
expressionLoaderOptions.syncColors,
]);
Expand Down
7 changes: 6 additions & 1 deletion src/plugins/expressions/public/render.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ export interface ExpressionRenderHandlerParams {
onRenderError?: RenderErrorHandlerFnType;
renderMode?: RenderMode;
syncColors?: boolean;
interactive?: boolean;
hasCompatibleActions?: (event: ExpressionRendererEvent) => Promise<boolean>;
}

Expand Down Expand Up @@ -54,6 +55,7 @@ export class ExpressionRenderHandler {
onRenderError,
renderMode,
syncColors,
interactive,
hasCompatibleActions = async () => false,
}: ExpressionRenderHandlerParams = {}
) {
Expand Down Expand Up @@ -90,11 +92,14 @@ export class ExpressionRenderHandler {
this.eventsSubject.next(data);
},
getRenderMode: () => {
return renderMode || 'display';
return renderMode || 'view';
},
isSyncColorsEnabled: () => {
return syncColors || false;
},
isInteractive: () => {
return interactive ?? true;
},
hasCompatibleActions,
};
}
Expand Down
1 change: 1 addition & 0 deletions src/plugins/expressions/public/types/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ export interface IExpressionLoaderParams {
customRenderers?: [];
uiState?: unknown;
inspectorAdapters?: Adapters;
interactive?: boolean;
onRenderError?: RenderErrorHandlerFnType;
searchSessionId?: string;
renderMode?: RenderMode;
Expand Down
3 changes: 2 additions & 1 deletion src/plugins/presentation_util/public/__stories__/render.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,9 @@ import React, { useRef, useEffect } from 'react';
import { ExpressionRenderDefinition, IInterpreterRenderHandlers } from 'src/plugins/expressions';

export const defaultHandlers: IInterpreterRenderHandlers = {
getRenderMode: () => 'display',
getRenderMode: () => 'view',
isSyncColorsEnabled: () => false,
isInteractive: () => true,
done: action('done'),
onDestroy: action('onDestroy'),
reload: action('reload'),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -402,6 +402,7 @@ export class VisualizeEmbeddable
searchSessionId: this.input.searchSessionId,
syncColors: this.input.syncColors,
uiState: this.vis.uiState,
interactive: !this.input.disableTriggers,
inspectorAdapters: this.inspectorAdapters,
executionContext: context,
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,6 @@ export function savedLens(): ExpressionFunctionDefinition<
title: args.title === null ? undefined : args.title,
disableTriggers: true,
palette: args.palette,
renderMode: 'noInteractivity',
},
embeddableType: EmbeddableTypes.lens,
generatedAt: Date.now(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,9 @@ export const defaultHandlers: RendererHandlers = {
destroy: () => action('destroy'),
getElementId: () => 'element-id',
getFilter: () => 'filter',
getRenderMode: () => 'display',
getRenderMode: () => 'view',
isSyncColorsEnabled: () => false,
isInteractive: () => true,
onComplete: (fn) => undefined,
onEmbeddableDestroyed: action('onEmbeddableDestroyed'),
onEmbeddableInputChange: action('onEmbeddableInputChange'),
Expand Down
3 changes: 2 additions & 1 deletion x-pack/plugins/canvas/public/lib/create_handlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,9 @@ export const createBaseHandlers = (): IInterpreterRenderHandlers => ({
update() {},
event() {},
onDestroy() {},
getRenderMode: () => 'display',
getRenderMode: () => 'view',
isSyncColorsEnabled: () => false,
isInteractive: () => true,
});

export const createHandlers = (baseHandlers = createBaseHandlers()): RendererHandlers => ({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ const LensMarkDownRendererComponent: React.FC<LensMarkDownRendererProps> = ({
style={{ height: LENS_VISUALIZATION_HEIGHT }}
timeRange={timeRange}
attributes={attributes}
renderMode="display"
renderMode="view"
/>
<LensChartTooltipFix />
</Container>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ describe('DatatableComponent', () => {
dispatchEvent={onDispatchEvent}
getType={jest.fn()}
rowHasRowClickTriggerActions={[false, false, false]}
renderMode="display"
renderMode="view"
paletteService={chartPluginMock.createPaletteRegistry()}
uiSettings={({ get: jest.fn() } as unknown) as IUiSettingsClient}
/>
Expand Down Expand Up @@ -427,7 +427,7 @@ describe('DatatableComponent', () => {
formatFactory={() => ({ convert: (x) => x } as IFieldFormat)}
dispatchEvent={onDispatchEvent}
getType={jest.fn()}
renderMode="display"
renderMode="view"
paletteService={chartPluginMock.createPaletteRegistry()}
uiSettings={({ get: jest.fn() } as unknown) as IUiSettingsClient}
/>
Expand Down Expand Up @@ -457,7 +457,7 @@ describe('DatatableComponent', () => {
formatFactory={() => ({ convert: (x) => x } as IFieldFormat)}
dispatchEvent={onDispatchEvent}
getType={jest.fn()}
renderMode="display"
renderMode="view"
paletteService={chartPluginMock.createPaletteRegistry()}
uiSettings={({ get: jest.fn() } as unknown) as IUiSettingsClient}
/>
Expand Down Expand Up @@ -485,7 +485,7 @@ describe('DatatableComponent', () => {
formatFactory={() => ({ convert: (x) => x } as IFieldFormat)}
dispatchEvent={onDispatchEvent}
getType={jest.fn()}
renderMode="display"
renderMode="view"
paletteService={chartPluginMock.createPaletteRegistry()}
uiSettings={({ get: jest.fn() } as unknown) as IUiSettingsClient}
/>
Expand Down Expand Up @@ -546,7 +546,7 @@ describe('DatatableComponent', () => {
formatFactory={() => ({ convert: (x) => x } as IFieldFormat)}
dispatchEvent={onDispatchEvent}
getType={jest.fn()}
renderMode="display"
renderMode="view"
paletteService={chartPluginMock.createPaletteRegistry()}
uiSettings={({ get: jest.fn() } as unknown) as IUiSettingsClient}
/>
Expand Down Expand Up @@ -581,7 +581,7 @@ describe('DatatableComponent', () => {
formatFactory={() => ({ convert: (x) => x } as IFieldFormat)}
dispatchEvent={onDispatchEvent}
getType={jest.fn()}
renderMode="display"
renderMode="view"
paletteService={chartPluginMock.createPaletteRegistry()}
uiSettings={({ get: jest.fn() } as unknown) as IUiSettingsClient}
/>
Expand Down Expand Up @@ -616,7 +616,7 @@ describe('DatatableComponent', () => {
formatFactory={() => ({ convert: (x) => x } as IFieldFormat)}
dispatchEvent={onDispatchEvent}
getType={jest.fn()}
renderMode="display"
renderMode="view"
paletteService={chartPluginMock.createPaletteRegistry()}
uiSettings={({ get: jest.fn() } as unknown) as IUiSettingsClient}
/>
Expand Down Expand Up @@ -650,7 +650,7 @@ describe('DatatableComponent', () => {
formatFactory={() => ({ convert: (x) => x } as IFieldFormat)}
dispatchEvent={onDispatchEvent}
getType={jest.fn()}
renderMode="display"
renderMode="view"
paletteService={chartPluginMock.createPaletteRegistry()}
uiSettings={({ get: jest.fn() } as unknown) as IUiSettingsClient}
/>
Expand Down
10 changes: 8 additions & 2 deletions x-pack/plugins/lens/public/embeddable/embeddable.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -566,7 +566,8 @@ describe('embeddable', () => {
timeRange,
query,
filters,
renderMode: 'noInteractivity',
renderMode: 'view',
disableTriggers: true,
} as LensEmbeddableInput;

const embeddable = new Embeddable(
Expand Down Expand Up @@ -599,7 +600,12 @@ describe('embeddable', () => {
await embeddable.initializeSavedVis(input);
embeddable.render(mountpoint);

expect(expressionRenderer.mock.calls[0][0].renderMode).toEqual('noInteractivity');
expect(expressionRenderer.mock.calls[0][0]).toEqual(
expect.objectContaining({
interactive: false,
renderMode: 'view',
})
);
});

it('should merge external context with query and filters of the saved object', async () => {
Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/lens/public/embeddable/embeddable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -361,6 +361,7 @@ export class Embeddable
searchSessionId={this.externalSearchContext.searchSessionId}
handleEvent={this.handleEvent}
onData$={this.updateActiveData}
interactive={!input.disableTriggers}
renderMode={input.renderMode}
syncColors={input.syncColors}
hasCompatibleActions={this.hasCompatibleActions}
Expand Down
3 changes: 3 additions & 0 deletions x-pack/plugins/lens/public/embeddable/expression_wrapper.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ export interface ExpressionWrapperProps {
expression: string | null;
errors: ErrorMessage[] | undefined;
variables?: Record<string, unknown>;
interactive?: boolean;
searchContext: ExecutionContextSearch;
searchSessionId?: string;
handleEvent: (event: ExpressionRendererEvent) => void;
Expand Down Expand Up @@ -113,6 +114,7 @@ export function ExpressionWrapper({
searchContext,
variables,
handleEvent,
interactive,
searchSessionId,
onData$,
renderMode,
Expand All @@ -137,6 +139,7 @@ export function ExpressionWrapper({
padding="s"
variables={variables}
expression={expression}
interactive={interactive}
searchContext={searchContext}
searchSessionId={searchSessionId}
onData$={onData$}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ export const getPieRenderer = (dependencies: {
{...config}
formatFactory={dependencies.formatFactory}
chartsThemeService={dependencies.chartsThemeService}
interactive={handlers.isInteractive()}
paletteService={dependencies.paletteService}
onClickValue={onClickValue}
renderMode={handlers.getRenderMode()}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ describe('PieVisualization component', () => {
onClickValue: jest.fn(),
chartsThemeService,
paletteService: chartPluginMock.createPaletteRegistry(),
renderMode: 'display' as const,
renderMode: 'view' as const,
syncColors: false,
};
}
Expand Down Expand Up @@ -302,10 +302,10 @@ describe('PieVisualization component', () => {
`);
});

test('does not set click listener on noInteractivity render mode', () => {
test('does not set click listener on non-interactive mode', () => {
const defaultArgs = getDefaultArgs();
const component = shallow(
<PieComponent args={{ ...args }} {...defaultArgs} renderMode="noInteractivity" />
<PieComponent args={{ ...args }} {...defaultArgs} interactive={false} />
);
expect(component.find(Settings).first().prop('onElementClick')).toBeUndefined();
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ export function PieComponent(
props: PieExpressionProps & {
formatFactory: FormatFactory;
chartsThemeService: ChartsPluginSetup['theme'];
interactive?: boolean;
paletteService: PaletteRegistry;
onClickValue: (data: LensFilterEvent['data']) => void;
renderMode: RenderMode;
Expand Down Expand Up @@ -289,9 +290,7 @@ export function PieComponent(
}
legendPosition={legendPosition || Position.Right}
legendMaxDepth={nestedLegend ? undefined : 1 /* Color is based only on first layer */}
onElementClick={
props.renderMode !== 'noInteractivity' ? onElementClickHandler : undefined
}
onElementClick={props.interactive ?? true ? onElementClickHandler : undefined}
legendAction={getLegendAction(firstTable, onClickValue)}
theme={{
...chartTheme,
Expand Down
10 changes: 5 additions & 5 deletions x-pack/plugins/lens/public/xy_visualization/expression.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -480,7 +480,7 @@ describe('xy_expression', () => {
defaultProps = {
formatFactory: getFormatSpy,
timeZone: 'UTC',
renderMode: 'display',
renderMode: 'view',
chartsThemeService,
chartsActiveCursorService,
paletteService,
Expand Down Expand Up @@ -1064,11 +1064,11 @@ describe('xy_expression', () => {
});
});

test('onBrushEnd is not set on noInteractivity mode', () => {
test('onBrushEnd is not set on non-interactive mode', () => {
const { args, data } = sampleArgs();

const wrapper = mountWithIntl(
<XYChart {...defaultProps} data={data} args={args} renderMode="noInteractivity" />
<XYChart {...defaultProps} data={data} args={args} interactive={false} />
);

expect(wrapper.find(Settings).first().prop('onBrushEnd')).toBeUndefined();
Expand Down Expand Up @@ -1334,11 +1334,11 @@ describe('xy_expression', () => {
});
});

test('onElementClick is not triggering event on noInteractivity mode', () => {
test('onElementClick is not triggering event on non-interactive mode', () => {
const { args, data } = sampleArgs();

const wrapper = mountWithIntl(
<XYChart {...defaultProps} data={data} args={args} renderMode="noInteractivity" />
<XYChart {...defaultProps} data={data} args={args} interactive={false} />
);

expect(wrapper.find(Settings).first().prop('onElementClick')).toBeUndefined();
Expand Down
8 changes: 5 additions & 3 deletions x-pack/plugins/lens/public/xy_visualization/expression.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ export type XYChartRenderProps = XYChartProps & {
formatFactory: FormatFactory;
timeZone: string;
minInterval: number | undefined;
interactive?: boolean;
onClickValue: (data: LensFilterEvent['data']) => void;
onSelectRange: (data: LensBrushEvent['data']) => void;
renderMode: RenderMode;
Expand Down Expand Up @@ -160,6 +161,7 @@ export const getXyChartRenderer = (dependencies: {
paletteService={dependencies.paletteService}
timeZone={dependencies.timeZone}
minInterval={calculateMinInterval(config)}
interactive={handlers.isInteractive()}
onClickValue={onClickValue}
onSelectRange={onSelectRange}
renderMode={handlers.getRenderMode()}
Expand Down Expand Up @@ -233,7 +235,7 @@ export function XYChart({
minInterval,
onClickValue,
onSelectRange,
renderMode,
interactive = true,
syncColors,
}: XYChartRenderProps) {
const {
Expand Down Expand Up @@ -528,8 +530,8 @@ export function XYChart({
}}
rotation={shouldRotate ? 90 : 0}
xDomain={xDomain}
onBrushEnd={renderMode !== 'noInteractivity' ? brushHandler : undefined}
onElementClick={renderMode !== 'noInteractivity' ? clickHandler : undefined}
onBrushEnd={interactive ? brushHandler : undefined}
onElementClick={interactive ? clickHandler : undefined}
legendAction={getLegendAction(
filteredLayers,
data.tables,
Expand Down