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

Upgrade EUI to v92.1.1 #174955

Merged
merged 13 commits into from
Jan 24, 2024
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@
"@elastic/datemath": "5.0.3",
"@elastic/elasticsearch": "npm:@elastic/[email protected]",
"@elastic/ems-client": "8.5.1",
"@elastic/eui": "92.0.0-backport.0",
"@elastic/eui": "92.1.1",
"@elastic/filesaver": "1.1.2",
"@elastic/node-crypto": "1.2.1",
"@elastic/numeral": "^2.5.1",
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -523,6 +523,14 @@ export const getEuiContextMapping = (): EuiTokensObject => {
defaultMessage: '{columnId}, column {col}, row {row}',
values: { columnId, row, col },
}),
'euiDataGridCell.expansionEnterPrompt': i18n.translate(
'core.euiDataGridCell.expansionEnterPrompt',
{ defaultMessage: 'Press the Enter key to expand this cell.' }
),
'euiDataGridCell.focusTrapEnterPrompt': i18n.translate(
'core.euiDataGridCell.focusTrapEnterPrompt',
{ defaultMessage: "Press the Enter key to interact with this cell's contents." }
),
'euiDataGridCellActions.expandButtonTitle': i18n.translate(
'core.euiDataGridCellActions.expandButtonTitle',
{
Expand Down
107 changes: 51 additions & 56 deletions packages/kbn-resizable-layout/src/panels_resizable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,19 @@
* Side Public License, v 1.
*/

import { EuiResizableContainer, useGeneratedHtmlId, useResizeObserver } from '@elastic/eui';
import {
EuiResizableContainer,
useGeneratedHtmlId,
useResizeObserver,
useEuiTheme,
mathWithUnits,
} from '@elastic/eui';
import type { ResizeTrigger } from '@elastic/eui/src/components/resizable_container/types';
import { css } from '@emotion/react';
import { isEqual, round } from 'lodash';
import type { ReactElement } from 'react';
import React, { useCallback, useEffect, useState } from 'react';
import useLatest from 'react-use/lib/useLatest';
import { ResizableLayoutDirection } from '../types';
import { getContainerSize, percentToPixels, pixelsToPercent } from './utils';

Expand Down Expand Up @@ -47,41 +54,30 @@ export const PanelsResizable = ({
onFixedPanelSizeChange?: (fixedPanelSize: number) => void;
setPanelSizes: (panelSizes: { fixedPanelSizePct: number; flexPanelSizePct: number }) => void;
}) => {
const { euiTheme } = useEuiTheme();
const fixedPanelId = useGeneratedHtmlId({ prefix: 'fixedPanel' });
const { height: containerHeight, width: containerWidth } = useResizeObserver(container);
const containerSize = getContainerSize(direction, containerWidth, containerHeight);

// EuiResizableContainer doesn't work properly when used with react-reverse-portal and
// will cancel the resize. To work around this we keep track of when resizes start and
// end to toggle the rendering of a transparent overlay which prevents the cancellation.
// EUI issue: https://github.com/elastic/eui/issues/6199
const [resizeWithPortalsHackIsResizing, setResizeWithPortalsHackIsResizing] = useState(false);
const enableResizeWithPortalsHack = useCallback(
() => setResizeWithPortalsHackIsResizing(true),
[]
);
const disableResizeWithPortalsHack = useCallback(
() => setResizeWithPortalsHackIsResizing(false),
[]
);
const baseButtonCss = css`
background-color: transparent !important;
gap: 0 !important;

&:not(:hover):not(:focus) {
&:before,
&:after {
width: 0;
}
}
// The resize overlay makes it so that other mouse events (e.g. tooltip hovers, etc)
// don't occur when mouse dragging
const [isResizing, setIsResizing] = useState(false);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@davismcphee I'm hoping you can approve/review these changes to the package/Discover layout since we've been discussing this last week! 🙏 Please do let me know if you find any issues or aren't a huge fan of the renames, etc.

Copy link
Contributor

Choose a reason for hiding this comment

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

The changes look and work great, thanks! The renames/cleanups look good too, and I like that the window listener approach means we no longer have to work around react-reverse-portal swallowing our events (but keeping the overlay for tooltips, etc. makes sense).


// Align the resizable button border to overlap exactly over existing panel/layout borders
const buttonBorderCss = css`
position: relative;
inset-${direction === 'horizontal' ? 'inline-start' : 'block-end'}: -${mathWithUnits(
euiTheme.border.width.thin,
(x) => x / 2
)};
`;
const defaultButtonCss = css`
z-index: 3;
`;
const resizeWithPortalsHackButtonCss = css`
const resizingButtonCss = css`
z-index: 4;
`;
const resizeWithPortalsHackOverlayCss = css`
const resizingOverlayCss = css`
position: absolute;
top: 0;
left: 0;
Expand Down Expand Up @@ -158,34 +154,35 @@ export const PanelsResizable = ({
setPanelSizes,
]);

const onResizeStart = useCallback(
(trigger: ResizeTrigger) => {
if (trigger !== 'pointer') {
return;
}

enableResizeWithPortalsHack();
},
[enableResizeWithPortalsHack]
);

const onResizeEnd = useCallback(() => {
if (!resizeWithPortalsHackIsResizing) {
const onResizeStart = useCallback((trigger: ResizeTrigger) => {
if (trigger !== 'pointer') {
return;
}
setIsResizing(true);
}, []);

// EUI will call an outdated version of this callback when the resize ends,
// so we need to make sure on our end that the latest version is called.
const onResizeEndStable = useLatest(() => {
setIsResizing((_isResizing) => {
// We don't want the resize button to retain focus after the resize is complete,
// but EuiResizableContainer will force focus it onClick. To work around this we
// use setTimeout to wait until after onClick has been called before blurring.
if (_isResizing) {
if (document.activeElement instanceof HTMLElement) {
const button = document.activeElement;
setTimeout(() => {
button.blur();
});
}
}
return false;
});
});

// We don't want the resize button to retain focus after the resize is complete,
// but EuiResizableContainer will force focus it onClick. To work around this we
// use setTimeout to wait until after onClick has been called before blurring.
if (document.activeElement instanceof HTMLElement) {
const button = document.activeElement;
setTimeout(() => {
button.blur();
});
}

disableResizeWithPortalsHack();
}, [disableResizeWithPortalsHack, resizeWithPortalsHackIsResizing]);
const onResizeEnd = useCallback(() => {
onResizeEndStable.current();
}, [onResizeEndStable]);

// Don't render EuiResizableContainer until we have have valid
// panel sizes or it can cause the resize functionality to break.
Expand Down Expand Up @@ -218,10 +215,8 @@ export const PanelsResizable = ({
</EuiResizablePanel>
<EuiResizableButton
className={resizeButtonClassName}
css={[
baseButtonCss,
resizeWithPortalsHackIsResizing ? resizeWithPortalsHackButtonCss : defaultButtonCss,
]}
indicator="border"
css={[buttonBorderCss, isResizing ? resizingButtonCss : defaultButtonCss]}
data-test-subj={`${dataTestSubj}ResizableButton`}
/>
<EuiResizablePanel
Expand All @@ -232,7 +227,7 @@ export const PanelsResizable = ({
>
{flexPanel}
</EuiResizablePanel>
{resizeWithPortalsHackIsResizing ? <div css={resizeWithPortalsHackOverlayCss} /> : <></>}
{isResizing ? <div css={resizingOverlayCss} /> : <></>}
</>
)}
</EuiResizableContainer>
Expand Down
2 changes: 1 addition & 1 deletion src/dev/license_checker/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ export const LICENSE_OVERRIDES = {
'[email protected]': ['Eclipse Distribution License - v 1.0'], // cf. https://github.com/bjornharrtell/jsts
'@mapbox/[email protected]': ['MIT'], // license in readme https://github.com/tmcw/jsonlint
'@elastic/[email protected]': ['Elastic License 2.0'],
'@elastic/eui@92.0.0-backport.0': ['SSPL-1.0 OR Elastic License 2.0'],
'@elastic/eui@92.1.1': ['SSPL-1.0 OR Elastic License 2.0'],
'[email protected]': ['CC-BY-4.0'], // retired ODC‑By license https://github.com/mattcg/language-subtag-registry
'[email protected]': ['MIT'], // license in importing module https://www.npmjs.com/package/binary
'@bufbuild/[email protected]': ['Apache-2.0'], // license (Apache-2.0 AND BSD-3-Clause)
Expand Down
4 changes: 4 additions & 0 deletions src/plugins/vis_default_editor/public/_default.scss
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@
* Resizer
*/

.visEditor__resizer {
height: auto;
}

.visEditor__resizer-isHidden {
display: none;
}
Expand Down
1 change: 1 addition & 0 deletions src/plugins/vis_default_editor/public/default_editor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ function DefaultEditor({
</EuiResizablePanel>

<EuiResizableButton
alignIndicator="start"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@elastic/kibana-visualizations I noticed while doing a quick grep through Kibana for <EuiResizableButton> usages that this button was slightly broken in production due to flex display issues. I fixed this with a height .scss change and added this indicator alignment prop to match production.

Before After
before after

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanx Cee! I really appreciate it

className={`visEditor__resizer ${isCollapsed ? 'visEditor__resizer-isHidden' : ''}`}
/>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ describe('DatatableComponent', () => {
/>
);

wrapper.find('[data-test-subj="dataGridRowCell"]').first().simulate('focus');
wrapper.find('[data-test-subj="dataGridRowCell"]').first().simulate('mouseEnter');

await waitForWrapperUpdate(wrapper);

Expand Down Expand Up @@ -248,7 +248,7 @@ describe('DatatableComponent', () => {
/>
);

wrapper.find('[data-test-subj="dataGridRowCell"]').at(1).simulate('focus');
wrapper.find('[data-test-subj="dataGridRowCell"]').at(1).simulate('mouseEnter');

await waitForWrapperUpdate(wrapper);

Expand Down Expand Up @@ -324,7 +324,7 @@ describe('DatatableComponent', () => {
/>
);

wrapper.find('[data-test-subj="dataGridRowCell"]').at(0).simulate('focus');
wrapper.find('[data-test-subj="dataGridRowCell"]').at(0).simulate('mouseEnter');

await waitForWrapperUpdate(wrapper);

Expand Down Expand Up @@ -365,7 +365,7 @@ describe('DatatableComponent', () => {
/>
);

wrapper.find('[data-test-subj="dataGridRowCell"]').first().simulate('focus');
wrapper.find('[data-test-subj="dataGridRowCell"]').first().simulate('mouseEnter');

await waitForWrapperUpdate(wrapper);

Expand Down
7 changes: 3 additions & 4 deletions x-pack/plugins/osquery/cypress/e2e/all/live_query.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,10 +58,9 @@ describe('ALL - Live Query', { tags: ['@ess', '@serverless'] }, () => {
expect(interception.response?.body.data.queries[0]).to.have.property('timeout', 890);
});
checkResults();
cy.get('[data-gridcell-column-index="0"][data-gridcell-row-index="0"]').should('exist');
cy.get(
'[data-gridcell-column-index="0"][data-gridcell-row-index="0"] [data-datagrid-interactable="true"]'
).click();
const firstCell = '[data-gridcell-column-index="0"][data-gridcell-row-index="0"]';
cy.get(firstCell).should('exist');
cy.get(firstCell).find('[data-euigrid-tab-managed="true"]').click();
cy.url().should('include', 'app/fleet/agents/');
});

Expand Down
21 changes: 11 additions & 10 deletions x-pack/plugins/osquery/cypress/tasks/saved_queries.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,15 +52,17 @@ export const getSavedQueriesComplexTest = () =>

// hidden columns
cy.getBySel(RESULTS_TABLE_COLUMNS_BUTTON).should('have.text', 'Columns35');
cy.getBySel('dataGridHeaderCell-osquery.cmdline').click();
cy.contains(/Hide column$/).click();
cy.getBySel('dataGridHeaderCell-osquery.cwd').click();

cy.contains(/Hide column$/).click();
cy.getBySel('dataGridHeaderCell-osquery.disk_bytes_written.number').click();

cy.contains(/Hide column$/).click();
cy.getBySel('dataGridColumnSelectorButton').click();
Copy link
Contributor

Choose a reason for hiding this comment

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

So the current behavior doesn't require clicking the header first then 'Hide column' button ? But instead using a specific selector?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, see above - for some reason trying to click the header cell actions was timing out Cypress (I could reproduce this locally, although I honestly have no idea why it was happening) and causing Chrome Renderer crashes. So I opted to use the selector toolbar button instead. EUI already has its own component Cypress tests for hiding columns via the header actions buttons, so hopefully we would catch any regressions around that UX ourselves before they ever reach Kibana.

cy.get('[data-popover-open="true"]').should('be.visible');
cy.getBySel('dataGridColumnSelectorToggleColumnVisibility-osquery.cmdline').click();
cy.getBySel('dataGridColumnSelectorToggleColumnVisibility-osquery.cwd').click();
cy.getBySel(
'dataGridColumnSelectorToggleColumnVisibility-osquery.disk_bytes_written.number'
).click();
cy.getBySel('dataGridColumnSelectorButton').click();
cy.get('[data-popover-open="true"]').should('not.exist');
cy.getBySel(RESULTS_TABLE_COLUMNS_BUTTON).should('have.text', 'Columns32/35');

// change pagination
cy.getBySel('pagination-button-next').click().wait(500).click();
cy.getBySel(RESULTS_TABLE_COLUMNS_BUTTON).should('have.text', 'Columns32/35');
Expand All @@ -72,8 +74,7 @@ export const getSavedQueriesComplexTest = () =>
cy.getBySel(RESULTS_TABLE_BUTTON).click();

// sorting
cy.getBySel('dataGridHeaderCell-osquery.egid').click();

cy.getBySel('dataGridHeaderCellActionButton-osquery.egid').click({ force: true });
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need force ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cypress was being really flaky about these clicks (I'm not totally sure why as they work just fine in production, I think it's either layout settling or rerenders due to how fast it "clicks" things). CI was consistently failing for me without the force: true. If you can get it working in the future with another approach (e.g. a wait or a real click?) feel free to refactor this in the future!

cy.contains(/Sort A-Z$/).click();
cy.getBySel(RESULTS_TABLE_COLUMNS_BUTTON).should('have.text', 'Columns32/35');
cy.getBySel(RESULTS_TABLE_BUTTON).trigger('mouseover');
Expand Down
5 changes: 3 additions & 2 deletions x-pack/test/security_solution_cypress/cypress/tasks/alerts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -143,8 +143,9 @@ export const expandFirstAlertActions = () => {
};

export const expandFirstAlert = () => {
cy.get(EXPAND_ALERT_BTN).should('be.visible');
cy.get(EXPAND_ALERT_BTN).first().click();
cy.get(EXPAND_ALERT_BTN).first().should('be.visible');
// Cypress is flaky on clicking this button despite production not having that issue
cy.get(EXPAND_ALERT_BTN).first().trigger('click');
};

export const hideMessageTooltip = () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,9 @@ import { scrollAlertTableColumnIntoView } from '../alerts';
* Find the first alert row in the alerts table then click on the host name to open the flyout
*/
export const expandFirstAlertHostFlyout = () => {
cy.get(OPEN_HOST_FLYOUT_BUTTON).first().click();
// Cypress is flaky on clicking this button despite production not having that issue
// eslint-disable-next-line cypress/no-force
cy.get(OPEN_HOST_FLYOUT_BUTTON).first().click({ force: true });
};

/**
Expand Down
8 changes: 4 additions & 4 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -1740,10 +1740,10 @@
resolved "https://registry.yarnpkg.com/@elastic/eslint-plugin-eui/-/eslint-plugin-eui-0.0.2.tgz#56b9ef03984a05cc213772ae3713ea8ef47b0314"
integrity sha512-IoxURM5zraoQ7C8f+mJb9HYSENiZGgRVcG4tLQxE61yHNNRDXtGDWTZh8N1KIHcsqN1CEPETjuzBXkJYF/fDiQ==

"@elastic/eui@92.0.0-backport.0":
version "92.0.0-backport.0"
resolved "https://registry.yarnpkg.com/@elastic/eui/-/eui-92.0.0-backport.0.tgz#201757bef89141dad6f8f719976fa2a0b52be86a"
integrity sha512-ZliSke0YehCbCuHvYkY0BIMg32QeqTMZy+qgGYsh+Bp0UZ4CZn9j5e7LWavfal3+t8HMFCHTk0RUFFXrcepplA==
"@elastic/eui@92.1.1":
version "92.1.1"
resolved "https://registry.yarnpkg.com/@elastic/eui/-/eui-92.1.1.tgz#f5713bfa12733a2c62df365e37f7983a7c08411e"
integrity sha512-kjUP+IewVcoJ59e84lHwtdy5OvGNSmlHvzn5uUmGTKnBGOxoYv5K9pyGujVWpxSCG9MNr3anMsNE7EX6usj37A==
dependencies:
"@hello-pangea/dnd" "^16.3.0"
"@types/lodash" "^4.14.198"
Expand Down