Skip to content

Commit

Permalink
[EuiDataGrid] Fix incorrect full screen height (elastic#5557)
Browse files Browse the repository at this point in the history
* Fix incorrect scrolling height on full screen grids

- There's no need for a separate isFullScreen branch in this logic: EuiDataGrid's wrapper will update its dimensions on full screen toggle, and react-window should simply use that wrapper's height & width, the same way it does in non-full-screen mode

* Remove unused props

* Remove now-unnecessary isFullScreen prop from EuiDataGridBody

* Remove now unnecessary toolbar ref/height observer

* Add changelog entry

* Workaround for failing spec / toolbar hiding itself on non-interactive headers

* Swap a useRef for a useState to trigger a re-render after mount

Co-authored-by: Chandler Prall <chandler.prall@gmail.com>
  • Loading branch information
2 people authored and cee-chen committed Feb 11, 2022
1 parent 65bdf28 commit 0fea550
Show file tree
Hide file tree
Showing 8 changed files with 13 additions and 45 deletions.
4 changes: 3 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
## [`main`](https://github.com/elastic/eui/tree/main)

No public interface changes since `46.1.0`.
**Bug fixes**

- Fixed EuiDataGrid height issue when in full-screen mode and with scrolling content ([#5557](https://github.com/elastic/eui/pull/5557))

## [`46.1.0`](https://github.com/elastic/eui/tree/v46.1.0)

Expand Down
2 changes: 0 additions & 2 deletions src/components/datagrid/body/data_grid_body.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,9 @@ import { EuiDataGridBody, Cell } from './data_grid_body';

describe('EuiDataGridBody', () => {
const requiredProps = {
isFullScreen: false,
headerIsInteractive: true,
rowCount: 1,
visibleRows: { startRow: 0, endRow: 1, visibleRowCount: 1 },
toolbarHeight: 10,
columnWidths: { columnA: 20 },
columns: [
{ id: 'columnA', schema: 'boolean' },
Expand Down
6 changes: 0 additions & 6 deletions src/components/datagrid/body/data_grid_body.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,6 @@ export const EuiDataGridBody: FunctionComponent<EuiDataGridBodyProps> = (
props
) => {
const {
isFullScreen,
leadingControlColumns,
trailingControlColumns,
columns,
Expand All @@ -238,7 +237,6 @@ export const EuiDataGridBody: FunctionComponent<EuiDataGridBodyProps> = (
setVisibleColumns,
switchColumnPos,
onColumnResize,
toolbarHeight,
rowHeightsOptions,
virtualizationOptions,
gridStyles,
Expand Down Expand Up @@ -416,11 +414,7 @@ export const EuiDataGridBody: FunctionComponent<EuiDataGridBodyProps> = (
unconstrainedWidth: 0, // unable to determine this until the container's size is known
wrapperDimensions,
wrapperRef,
toolbarHeight,
headerRowHeight,
footerRowHeight,
rowCount,
isFullScreen,
});

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ describe('EuiDataGridToolbar', () => {
controlBtnClasses: '',
columnSelector: <div>mock column selector</div>,
columnSorting: <div>mock column sorting</div>,
setRef: jest.fn(),
setIsFullScreen: jest.fn(),
};

Expand Down
7 changes: 1 addition & 6 deletions src/components/datagrid/controls/data_grid_toolbar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ export const EuiDataGridToolbar = ({
displaySelector,
columnSelector,
columnSorting,
setRef,
setIsFullScreen,
}: EuiDataGridToolbarProps) => {
const [fullScreenButton, fullScreenButtonActive] = useEuiI18n(
Expand Down Expand Up @@ -86,11 +85,7 @@ export const EuiDataGridToolbar = ({
);

return (
<div
ref={setRef}
className="euiDataGrid__controls"
data-test-sub="dataGridControls"
>
<div className="euiDataGrid__controls" data-test-sub="dataGridControls">
{hasRoomForGridControls && (
<div className="euiDataGrid__leftControls">
{renderAdditionalControls(toolbarVisibility, 'left.prepend')}
Expand Down
13 changes: 5 additions & 8 deletions src/components/datagrid/data_grid.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -162,8 +162,10 @@ export const EuiDataGrid: FunctionComponent<EuiDataGridProps> = (props) => {
* Grid refs & observers
*/
// Outermost wrapper div
const resizeRef = useRef<HTMLDivElement | null>(null);
const { width: gridWidth } = useResizeObserver(resizeRef.current, 'width');
// this ref needs to be managed by a state, to cause a re-render after mount
// and passing the mounted element to the resize observer
const [resizeRef, setResizeRef] = useState<HTMLDivElement | null>(null);
const { width: gridWidth } = useResizeObserver(resizeRef, 'width');

// Wrapper div around EuiDataGridBody
const contentRef = useRef<HTMLDivElement | null>(null);
Expand Down Expand Up @@ -270,8 +272,6 @@ export const EuiDataGrid: FunctionComponent<EuiDataGridProps> = (props) => {
* Toolbar & full-screen
*/
const showToolbar = !!toolbarVisibility;
const [toolbarRef, setToolbarRef] = useState<HTMLDivElement | null>(null);
const { height: toolbarHeight } = useResizeObserver(toolbarRef, 'height');

const [isFullScreen, setIsFullScreen] = useState(false);
const handleGridKeyDown = (event: KeyboardEvent<HTMLDivElement>) => {
Expand Down Expand Up @@ -368,12 +368,11 @@ export const EuiDataGrid: FunctionComponent<EuiDataGridProps> = (props) => {
className={classes}
onKeyDown={handleGridKeyDown}
style={isFullScreen ? undefined : { width, height }}
ref={resizeRef}
ref={setResizeRef}
{...rest}
>
{showToolbar && (
<EuiDataGridToolbar
setRef={setToolbarRef}
gridWidth={gridWidth}
minSizeForControls={minSizeForControls}
toolbarVisibility={toolbarVisibility}
Expand Down Expand Up @@ -422,10 +421,8 @@ export const EuiDataGrid: FunctionComponent<EuiDataGridProps> = (props) => {
{...gridAriaProps}
>
<EuiDataGridBody
isFullScreen={isFullScreen}
columns={orderedVisibleColumns}
visibleColCount={visibleColCount}
toolbarHeight={toolbarHeight}
leadingControlColumns={leadingControlColumns}
schema={mergedSchema}
trailingControlColumns={trailingControlColumns}
Expand Down
4 changes: 1 addition & 3 deletions src/components/datagrid/data_grid_types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,9 @@ export interface EuiDataGridToolbarProps {
controlBtnClasses: string;
columnSelector: ReactNode;
columnSorting: ReactNode;
setRef: RefCallback<HTMLDivElement | null>;
setIsFullScreen: Dispatch<SetStateAction<boolean>>;
}

export interface EuiDataGridPaginationRendererProps
extends EuiDataGridPaginationProps {
rowCount: number;
Expand Down Expand Up @@ -327,7 +327,6 @@ export interface EuiDataGridColumnSortingDraggableProps {
display: string;
}
export interface EuiDataGridBodyProps {
isFullScreen: boolean;
leadingControlColumns: EuiDataGridControlColumn[];
trailingControlColumns: EuiDataGridControlColumn[];
columns: EuiDataGridColumn[];
Expand All @@ -346,7 +345,6 @@ export interface EuiDataGridBodyProps {
setVisibleColumns: EuiDataGridHeaderRowProps['setVisibleColumns'];
switchColumnPos: EuiDataGridHeaderRowProps['switchColumnPos'];
onColumnResize?: EuiDataGridOnColumnResizeHandler;
toolbarHeight: number;
virtualizationOptions?: Partial<VariableSizeGridProps>;
rowHeightsOptions?: EuiDataGridRowHeightsOptions;
gridStyles: EuiDataGridStyle;
Expand Down
21 changes: 3 additions & 18 deletions src/components/datagrid/utils/grid_height_width.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,21 +19,13 @@ export const useFinalGridDimensions = ({
unconstrainedWidth,
wrapperDimensions,
wrapperRef,
toolbarHeight,
headerRowHeight,
footerRowHeight,
rowCount,
isFullScreen,
}: {
unconstrainedHeight: number;
unconstrainedWidth: number;
wrapperDimensions: { width: number; height: number };
wrapperRef: MutableRefObject<HTMLDivElement | null>;
toolbarHeight: number;
headerRowHeight: number;
footerRowHeight: number;
rowCount: number;
isFullScreen: boolean;
}) => {
// Used if the grid needs to scroll
const [height, setHeight] = useState<number | undefined>(undefined);
Expand All @@ -43,7 +35,7 @@ export const useFinalGridDimensions = ({
useEffect(() => {
const boundingRect = wrapperRef.current!.getBoundingClientRect();

if (boundingRect.height !== unconstrainedHeight && !isFullScreen) {
if (boundingRect.height !== unconstrainedHeight) {
setHeight(boundingRect.height);
}
if (boundingRect.width !== unconstrainedWidth) {
Expand All @@ -57,22 +49,15 @@ export const useFinalGridDimensions = ({
wrapperRef,
unconstrainedHeight,
unconstrainedWidth,
isFullScreen,
]);

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

if (isFullScreen) {
finalHeight =
window.innerHeight - toolbarHeight - headerRowHeight - footerRowHeight;
finalWidth = window.innerWidth;
}

return { finalHeight, finalWidth };
};

Expand Down

0 comments on commit 0fea550

Please sign in to comment.