Skip to content

Commit

Permalink
fix console errors in inspector (elastic#156894)
Browse files Browse the repository at this point in the history
Steps to view problem
* install sample data set
* Open lens visualization
* Open inspector. Notice console errors
<img width="300" alt="Screen Shot 2023-05-05 at 11 03 25 AM"
src="https://user-images.githubusercontent.com/373691/236521366-d8fb9302-e93b-4047-a0bf-d7c09dcc3ffb.png">

elastic/eui#6566 removed `closeButtonAriaLabel`
prop from [EuiFlyout](https://elastic.github.io/eui/#/layout/flyout) EUI
75.0.0 (Effecting 8.8 and 8.9). FlyoutService spreads options into
`EuiFlyout`, resulting in `closeButtonAriaLabel` getting added to dom
and causing error.

`OverlayFlyoutOpenOptions` type added by
elastic#37894. I replaced
`OverlayFlyoutOpenOptions` with `EuiFlyoutProps` to make it more clear
what props are accepted and provide stronger typing that stays in sync
with EUI typings

---------

Co-authored-by: kibanamachine <[email protected]>
(cherry picked from commit b803ba9)
  • Loading branch information
nreese committed May 15, 2023
1 parent fc37d3b commit 7ddae85
Show file tree
Hide file tree
Showing 7 changed files with 13 additions and 22 deletions.
16 changes: 3 additions & 13 deletions packages/core/overlays/core-overlays-browser/src/flyout.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/
import type { EuiFlyoutSize, EuiOverlayMaskProps } from '@elastic/eui';
import type { EuiFlyoutProps } from '@elastic/eui';
import type { MountPoint, OverlayRef } from '@kbn/core-mount-utils-browser';

/**
Expand All @@ -28,20 +28,10 @@ export interface OverlayFlyoutStart {
/**
* @public
*/
export interface OverlayFlyoutOpenOptions {
className?: string;
closeButtonAriaLabel?: string;
ownFocus?: boolean;
'data-test-subj'?: string;
'aria-label'?: string;
size?: EuiFlyoutSize;
maxWidth?: boolean | number | string;
hideCloseButton?: boolean;
outsideClickCloses?: boolean;
maskProps?: EuiOverlayMaskProps;
export type OverlayFlyoutOpenOptions = Omit<EuiFlyoutProps, 'onClose'> & {
/**
* EuiFlyout onClose handler.
* If provided the consumer is responsible for calling flyout.close() to close the flyout;
*/
onClose?: (flyout: OverlayRef) => void;
}
};
2 changes: 1 addition & 1 deletion src/plugins/inspector/public/plugin.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ export class InspectorPublicPlugin implements Plugin<Setup, Start> {
),
{
'data-test-subj': 'inspectorPanel',
closeButtonAriaLabel: closeButtonLabel,
closeButtonProps: { 'aria-label': closeButtonLabel },
}
);
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,12 +69,12 @@ test('passes through flyout options when opening flyout', () => {

overlays.openFlyout(<>foo</>, {
'data-test-subj': 'foo',
closeButtonAriaLabel: 'bar',
closeButtonProps: { 'aria-label': 'bar' },
});

expect(coreOverlays.openFlyout.mock.calls[0][1]).toEqual({
'data-test-subj': 'foo',
closeButtonAriaLabel: 'bar',
closeButtonProps: { 'aria-label': 'bar' },
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ export async function showCategorizeFlyout(
{
'data-test-subj': 'aiopsCategorizeFlyout',
ownFocus: true,
closeButtonAriaLabel: 'aiopsCategorizeFlyout',
closeButtonProps: { 'aria-label': 'aiopsCategorizeFlyout' },
onClose: onFlyoutClose,
size: 'l',
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -171,9 +171,11 @@ export const WorkspaceTopNavMenu = (props: WorkspaceTopNavMenuProps) => {
),
{
size: 'm',
closeButtonAriaLabel: i18n.translate('xpack.graph.settings.closeLabel', {
defaultMessage: 'Close',
}),
closeButtonProps: {
'aria-label': i18n.translate('xpack.graph.settings.closeLabel', {
defaultMessage: 'Close',
}),
},
'data-test-subj': 'graphSettingsFlyout',
ownFocus: true,
className: 'gphSettingsFlyout',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ export async function resolveJobSelection(
{
'data-test-subj': 'mlFlyoutJobSelector',
ownFocus: true,
closeButtonAriaLabel: 'jobSelectorFlyout',
closeButtonProps: { 'aria-label': 'jobSelectorFlyout' },
}
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,6 @@ export function createFlyout(
'data-test-subj': 'mlFlyoutLayerSelector',
ownFocus: true,
onClose: onFlyoutClose,
// @ts-expect-error should take any number/string compatible with the CSS width attribute
size: '35vw',
}
);
Expand Down

0 comments on commit 7ddae85

Please sign in to comment.