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

[EuiDataGrid] Account for horizontal scrollbar when determining height #5478

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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 `43.1.1`.
**Bug fixes**

- Fixed a `EuiDataGrid` sizing bug which didn't account for a horizontal scrollbar ([#5478](https://github.com/elastic/eui/pull/5478))

## [`43.1.1`](https://github.com/elastic/eui/tree/v43.1.1)

Expand Down
39 changes: 33 additions & 6 deletions src/components/datagrid/body/data_grid_body.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ import {
} from '../data_grid_types';
import { makeRowManager } from './data_grid_row_manager';
import { useForceRender } from '../../../services/hooks/useForceRender';
import { useUpdateEffect } from '../../../services';
Copy link
Contributor

Choose a reason for hiding this comment

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

Yay, glad this is already coming in handy! 🎉


export const VIRTUALIZED_CONTAINER_CLASS = 'euiDataGrid__virtualized';

Expand Down Expand Up @@ -265,6 +266,8 @@ const useUnconstrainedHeight = ({
defaultHeight,
headerRowHeight,
footerRowHeight,
outerGridRef,
innerGridRef,
}: {
rowHeightUtils: RowHeightUtils;
startRow: number;
Expand All @@ -274,6 +277,8 @@ const useUnconstrainedHeight = ({
defaultHeight: number;
headerRowHeight: number;
footerRowHeight: number;
outerGridRef: React.MutableRefObject<HTMLDivElement | null>;
innerGridRef: React.MutableRefObject<HTMLDivElement | null>;
}) => {
// when a row height is updated, force a re-render of the grid body to update the unconstrained height
const forceRender = useForceRender();
Expand Down Expand Up @@ -307,11 +312,29 @@ const useUnconstrainedHeight = ({
// how many rows to provide space for on the screen
const rowCountToAffordFor = endRow - startRow;

// watch the inner element for a change to its width
// which may cause the horizontal scrollbar to be added or removed
const { width: innerWidth } = useResizeObserver(
innerGridRef.current,
'width'
);
useUpdateEffect(forceRender, [innerWidth]);

// https://stackoverflow.com/a/5038256
const hasHorizontalScroll =
(outerGridRef.current?.scrollWidth ?? 0) >
(outerGridRef.current?.clientWidth ?? 0);
// https://stackoverflow.com/a/24797425
Comment on lines +323 to +327
Copy link
Contributor

Choose a reason for hiding this comment

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

Haha, I appreciate the extra context / honesty of adding stack overflow links 😆

const scrollbarHeight = hasHorizontalScroll
? outerGridRef.current!.offsetHeight - outerGridRef.current!.clientHeight
: 0;

const unconstrainedHeight =
defaultHeight * (rowCountToAffordFor - knownRowCount) + // guess how much space is required for unknown rows
knownHeight + // computed pixel height of the known rows
headerRowHeight + // account for header
footerRowHeight; // account for footer
footerRowHeight + // account for footer
scrollbarHeight; // account for horizontal scrollbar

return unconstrainedHeight;
};
Expand Down Expand Up @@ -636,6 +659,12 @@ export const EuiDataGridBody: FunctionComponent<EuiDataGridBodyProps> = (
}
}, [getRowHeight]);

const wrapperRef = useRef<HTMLDivElement | null>(null);
const wrapperDimensions = useResizeObserver(wrapperRef.current);

const outerGridRef = useRef<HTMLDivElement | null>(null); // container that becomes scrollable
const innerGridRef = useRef<HTMLDivElement | null>(null); // container sized to fit all content
Comment on lines +665 to +666
Copy link
Contributor

Choose a reason for hiding this comment

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

Also appreciate this extra comment context + reorganization


const unconstrainedHeight = useUnconstrainedHeight({
rowHeightUtils,
startRow,
Expand All @@ -645,6 +674,8 @@ export const EuiDataGridBody: FunctionComponent<EuiDataGridBodyProps> = (
defaultHeight,
headerRowHeight,
footerRowHeight,
outerGridRef,
innerGridRef,
});

// unable to determine this until the container's size is known anyway
Expand All @@ -653,11 +684,6 @@ export const EuiDataGridBody: FunctionComponent<EuiDataGridBodyProps> = (
const [height, setHeight] = useState<number | undefined>(undefined);
const [width, setWidth] = useState<number | undefined>(undefined);

const wrapperRef = useRef<HTMLDivElement | null>(null);
const wrapperDimensions = useResizeObserver(wrapperRef.current);

const innerGridRef = useRef<HTMLDivElement | null>(null);

// useState instead of useMemo as React reserves the right to drop memoized
// values in the future, and that would be very bad here
const [rowManager] = useState<EuiDataGridRowManager>(() =>
Expand Down Expand Up @@ -738,6 +764,7 @@ export const EuiDataGridBody: FunctionComponent<EuiDataGridBodyProps> = (
{...(virtualizationOptions ? virtualizationOptions : {})}
ref={setGridRef}
innerElementType={InnerElement}
outerRef={outerGridRef}
innerRef={innerGridRef}
className={VIRTUALIZED_CONTAINER_CLASS}
columnCount={
Expand Down
38 changes: 38 additions & 0 deletions src/components/datagrid/data_grid.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,44 @@ describe('EuiDataGrid', () => {
.should('be.greaterThan', firstHeight);
});
});

it('accounts for a horizontal scrollbar', () => {
const columns: EuiDataGridColumn[] = [];
for (let i = 0; i < 100; i++) {
columns.push({ id: `column ${i}` });
}
const columnVisibility = {
visibleColumns: columns.map(({ id }) => id),
setVisibleColumns: () => {},
};
cy.mount(
<EuiDataGrid
{...baseProps}
columns={columns}
columnVisibility={columnVisibility}
renderFooterCellValue={undefined}
/>
);

getGridData();

const virtualizedContainer = cy
.get('[data-test-subj=euiDataGridBody]')
.children()
.first();

// make sure the horizontal scrollbar is present
virtualizedContainer.then(([outerContainer]: [HTMLDivElement]) => {
expect(outerContainer.offsetHeight).to.be.greaterThan(
outerContainer.clientHeight
);
});

// make sure the vertical scrollbar is gone
virtualizedContainer.then(([outerContainer]: [HTMLDivElement]) => {
expect(outerContainer.offsetWidth).to.equal(outerContainer.clientWidth);
});
});
});

describe('focus management', () => {
Expand Down