Skip to content

[EuiDataGrid] Fix grid height when exiting full screen mode #5580

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.

Sign up for GitHub

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

Merged
cee-chen merged 2 commits into from
Feb 1, 2022
Merged

[EuiDataGrid] Fix grid height when exiting full screen mode #5580

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e354384
Track full screen grid height separately
cee-chen Jan 31, 2022
e905397
Fix nested ternary eslint warning
cee-chen Jan 31, 2022
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/components/datagrid/body/data_grid_body.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ describe('EuiDataGridBody', () => {
schemaDetectors,
popoverContents: providedPopoverContents,
rowHeightUtils: mockRowHeightUtils,
isFullScreen: false,
gridStyles: {},
gridWidth: 300,
gridRef: {
Expand Down
2 changes: 2 additions & 0 deletions src/components/datagrid/body/data_grid_body.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,7 @@ export const EuiDataGridBody: FunctionComponent<EuiDataGridBodyProps> = (
onColumnResize,
rowHeightsOptions,
virtualizationOptions,
isFullScreen,
gridStyles,
gridWidth,
gridRef,
Expand Down Expand Up @@ -424,6 +425,7 @@ export const EuiDataGridBody: FunctionComponent<EuiDataGridBodyProps> = (
unconstrainedWidth: 0, // unable to determine this until the container's size is known
wrapperDimensions,
wrapperRef,
isFullScreen,
rowCount,
});

Expand Down
1 change: 1 addition & 0 deletions src/components/datagrid/data_grid.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -441,6 +441,7 @@ export const EuiDataGrid: FunctionComponent<EuiDataGridProps> = (props) => {
interactiveCellId={interactiveCellId}
rowHeightsOptions={rowHeightsOptions}
virtualizationOptions={virtualizationOptions || {}}
isFullScreen={isFullScreen}
gridStyles={gridStyles}
gridWidth={gridWidth}
gridRef={gridRef}
Expand Down
1 change: 1 addition & 0 deletions src/components/datagrid/data_grid_types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -345,6 +345,7 @@ export interface EuiDataGridBodyProps {
onColumnResize?: EuiDataGridOnColumnResizeHandler;
virtualizationOptions?: Partial<VariableSizeGridProps>;
rowHeightsOptions?: EuiDataGridRowHeightsOptions;
isFullScreen: boolean;
gridStyles: EuiDataGridStyle;
gridWidth: number;
gridRef: MutableRefObject<Grid | null>;
Expand Down
28 changes: 20 additions & 8 deletions src/components/datagrid/utils/grid_height_width.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,46 +19,58 @@ export const useFinalGridDimensions = ({
unconstrainedWidth,
wrapperDimensions,
wrapperRef,
isFullScreen,
rowCount,
}: {
unconstrainedHeight: number;
unconstrainedWidth: number;
wrapperDimensions: { width: number; height: number };
wrapperRef: MutableRefObject<HTMLDivElement | null>;
isFullScreen: boolean;
rowCount: number;
}) => {
// Used if the grid needs to scroll
const [height, setHeight] = useState<number | undefined>(undefined);
const [width, setWidth] = useState<number | undefined>(undefined);
// Tracking full screen height separately is necessary to correctly restore the grid back to non-full-screen height
const [fullScreenHeight, setFullScreenHeight] = useState(0);

// Set the wrapper height on load, whenever the grid wrapper resizes, and whenever rowCount changes
useEffect(() => {
const boundingRect = wrapperRef.current!.getBoundingClientRect();

if (boundingRect.height !== unconstrainedHeight) {
setHeight(boundingRect.height);
if (isFullScreen) {
setFullScreenHeight(boundingRect.height);
} else {
if (boundingRect.height !== unconstrainedHeight) {
setHeight(boundingRect.height);
}
}
if (boundingRect.width !== unconstrainedWidth) {
setWidth(boundingRect.width);
}
}, [
// Effects that should cause recalculations
rowCount,
isFullScreen,
wrapperDimensions,
// Dependencies
wrapperRef,
unconstrainedHeight,
unconstrainedWidth,
]);

const finalHeight = IS_JEST_ENVIRONMENT
? Number.MAX_SAFE_INTEGER
const finalHeight = isFullScreen
? fullScreenHeight
: height || unconstrainedHeight;
const finalWidth = IS_JEST_ENVIRONMENT
? Number.MAX_SAFE_INTEGER
: width || unconstrainedWidth;
const finalWidth = width || unconstrainedWidth;

return { finalHeight, finalWidth };
return IS_JEST_ENVIRONMENT
? {
Comment on lines +68 to +69
Copy link
Contributor Author

@cee-chen cee-chen Jan 31, 2022

Choose a reason for hiding this comment

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

@chandlerprall Raising this because we just chatted about IS_JEST_ENVIRONMENT - is there any realistic way of unit testing this entire helper in Jest without running into this conditional? 🤔

Copy link
Contributor

@chandlerprall chandlerprall Feb 1, 2022

Choose a reason for hiding this comment

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

Only thing that comes to mind is to rewrite relevant tests in Cypress, but ship a mockenv version for downstream test environments. That gets weird with the other two functions in this file, and we'd probably want to split this function out into its own file to mock specifically. So, no, I don't have any great ideas ☹️

Copy link
Contributor Author

@cee-chen cee-chen Feb 1, 2022

Choose a reason for hiding this comment

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

Ooh, I totally forgot about .testenv files - I believe I can import the actual functions for the other 2 fns that don't have mocks and export only a fake fn for the one we want to target. Perfect! I'll look into that as a follow-up PR

finalHeight: Number.MAX_SAFE_INTEGER,
finalWidth: Number.MAX_SAFE_INTEGER,
}
: { finalHeight, finalWidth };
};

/**
Expand Down