Skip to content

Commit

Permalink
[Lens][Embeddable] Fix display options regressions (elastic#201998)
Browse files Browse the repository at this point in the history
## Summary

Fixes elastic#201829

This PR fixes a regression introduced with elastic#178965 where specific
visualization display options were ignored in the embeddable context.
Additional tests have been added to avoid further regressions in the
future.

### Checklist

- [x] [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
  • Loading branch information
dej611 authored Nov 29, 2024
1 parent 841d052 commit fd589b8
Show file tree
Hide file tree
Showing 9 changed files with 62 additions and 11 deletions.
2 changes: 1 addition & 1 deletion x-pack/plugins/lens/public/react_embeddable/data_loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ export function loadEmbeddableData(
handleEvent,
disableTriggers,
updateBlockingErrors,
renderCount: internalApi.renderCount$.getValue(),
getDisplayOptions: internalApi.getDisplayOptions,
}),
getUsedDataViews(
currentState.attributes.references,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import {
IndexPatternMap,
IndexPatternRef,
UserMessage,
VisualizationDisplayOptions,
isLensFilterEvent,
isLensMultiFilterEvent,
isLensTableRowContextMenuClickEvent,
Expand Down Expand Up @@ -61,7 +62,7 @@ interface GetExpressionRendererPropsParams {
api: LensApi;
addUserMessages: (messages: UserMessage[]) => void;
updateBlockingErrors: (error: Error) => void;
renderCount: number;
getDisplayOptions: () => VisualizationDisplayOptions;
}

async function getExpressionFromDocument(
Expand Down Expand Up @@ -146,7 +147,7 @@ export async function getExpressionRendererParams(
addUserMessages,
updateBlockingErrors,
searchContext,
renderCount,
getDisplayOptions,
}: GetExpressionRendererPropsParams
): Promise<{
params: ExpressionWrapperProps | null;
Expand Down Expand Up @@ -215,7 +216,7 @@ export async function getExpressionRendererParams(
variables: getVariables(api, state),
style: state.style,
className: state.className,
noPadding: state.noPadding,
noPadding: getDisplayOptions().noPadding,
};
return {
indexPatterns,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ export function initializeDashboardServices(
},
},
serialize: () => {
const { style, noPadding, className } = apiHasLensComponentProps(parentApi)
const { style, className } = apiHasLensComponentProps(parentApi)
? parentApi
: ({} as LensComponentProps);
const settings = apiPublishesSettings(parentApi)
Expand All @@ -155,7 +155,6 @@ export function initializeDashboardServices(
return {
...serializeTitles(),
style,
noPadding,
className,
...settings,
palette: initialState.palette,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,21 +6,25 @@
*/

import { BehaviorSubject } from 'rxjs';
import { initializeTitles } from '@kbn/presentation-publishing';
import type { DataView } from '@kbn/data-views-plugin/common';
import { buildObservableVariable, createEmptyLensState } from '../helper';
import type {
ExpressionWrapperProps,
LensEmbeddableStartServices,
LensInternalApi,
LensOverrides,
LensRuntimeState,
} from '../types';
import { apiHasAbortController } from '../type_guards';
import { apiHasAbortController, apiHasLensComponentProps } from '../type_guards';
import type { UserMessage } from '../../types';

export function initializeInternalApi(
initialState: LensRuntimeState,
parentApi: unknown
parentApi: unknown,
{ visualizationMap }: LensEmbeddableStartServices
): LensInternalApi {
const { titlesApi } = initializeTitles(initialState);
const [hasRenderCompleted$] = buildObservableVariable<boolean>(false);
const [expressionParams$] = buildObservableVariable<ExpressionWrapperProps | null>(null);
const expressionAbortController$ = new BehaviorSubject<AbortController | undefined>(undefined);
Expand Down Expand Up @@ -87,5 +91,30 @@ export function initializeInternalApi(
validationMessages$.next([]);
},
setAsCreated: () => isNewlyCreated$.next(false),
getDisplayOptions: () => {
const latestAttributes = attributes$.getValue();
if (!latestAttributes.visualizationType) {
return {};
}

let displayOptions =
visualizationMap[latestAttributes.visualizationType]?.getDisplayOptions?.() ?? {};

if (apiHasLensComponentProps(parentApi) && parentApi.noPadding != null) {
displayOptions = {
...displayOptions,
noPadding: parentApi.noPadding,
};
}

if (displayOptions.noPanelTitle == null && titlesApi.hidePanelTitle?.getValue()) {
displayOptions = {
...displayOptions,
noPanelTitle: true,
};
}

return displayOptions;
},
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ export const createLensEmbeddableFactory = (
* Observables and functions declared here are used internally to store mutating state values
* This is an internal API not exposed outside of the embeddable.
*/
const internalApi = initializeInternalApi(initialState, parentApi);
const internalApi = initializeInternalApi(initialState, parentApi, services);

const visualizationContextHelper = initializeVisualizationContext(internalApi);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,7 @@ const LensInternalApiMock: LensInternalApi = {
dispatchError: jest.fn(),
updateValidationMessages: jest.fn(),
setAsCreated: jest.fn(),
getDisplayOptions: jest.fn(() => ({})),
};

export function getLensInternalApiMock(overrides: Partial<LensInternalApi> = {}): LensInternalApi {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,12 @@ import { PublishingSubject } from '@kbn/presentation-publishing';
import React from 'react';
import { LensEmbeddableComponent } from './lens_embeddable_component';

jest.mock('../expression_wrapper', () => ({
ExpressionWrapper: () => (
<div className="lnsExpressionRenderer" data-test-subj="lens-embeddable" />
),
}));

type GetValueType<Type> = Type extends PublishingSubject<infer X> ? X : never;

function getDefaultProps({
Expand All @@ -39,4 +45,17 @@ describe('Lens Embeddable component', () => {
render(<LensEmbeddableComponent {...props} />);
expect(screen.queryByTestId('lens-embeddable')).not.toBeInTheDocument();
});

it('shoud not render the title if the visualization forces the title to be hidden', () => {
const getDisplayOptions = jest.fn(() => ({ noPanelTitle: true }));
const props = getDefaultProps({
internalApiOverrides: {
getDisplayOptions,
},
});

render(<LensEmbeddableComponent {...props} />);
expect(props.internalApi.getDisplayOptions).toHaveBeenCalled();
expect(screen.getByTestId('lens-embeddable').parentElement).not.toHaveAttribute('data-title');
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ export function LensEmbeddableComponent({
const rootRef = useDispatcher(hasRendered, api);

// Publish the data attributes only if avaialble/visible
const title = api.hidePanelTitle?.getValue()
const title = internalApi.getDisplayOptions()?.noPanelTitle
? undefined
: { 'data-title': api.panelTitle?.getValue() ?? api.defaultPanelTitle?.getValue() };
const description = api.panelDescription?.getValue()
Expand Down
4 changes: 3 additions & 1 deletion x-pack/plugins/lens/public/react_embeddable/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ import type {
SharingSavedObjectProps,
Simplify,
UserMessage,
VisualizationDisplayOptions,
VisualizationMap,
} from '../types';
import type { LensPluginStartDependencies } from '../plugin';
Expand Down Expand Up @@ -276,7 +277,7 @@ export type LensSerializedState = Simplify<
LensUnifiedSearchContext &
LensPanelProps &
SerializedTitles &
LensSharedProps &
Omit<LensSharedProps, 'noPadding'> &
Partial<DynamicActionsSerializedState> & { isNewPanel?: boolean }
>;

Expand Down Expand Up @@ -414,6 +415,7 @@ export type LensInternalApi = Simplify<
validationMessages$: PublishingSubject<UserMessage[]>;
updateValidationMessages: (newMessages: UserMessage[]) => void;
resetAllMessages: () => void;
getDisplayOptions: () => VisualizationDisplayOptions;
}
>;

Expand Down

0 comments on commit fd589b8

Please sign in to comment.