Skip to content

Commit

Permalink
[Expressions] Align renderMode with the embeddable viewMode (#110199)
Browse files Browse the repository at this point in the history
* Add preview view mode to the embeddable
* Rename display render mode to view
* Extract no interactivity render mode to a separate flag
  • Loading branch information
dokmic authored Sep 3, 2021
1 parent cb27ba0 commit df43d25
Show file tree
Hide file tree
Showing 21 changed files with 62 additions and 33 deletions.
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

0 comments on commit df43d25

Please sign in to comment.