Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
40 changes: 40 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,46 @@ describe('EuiDataGrid', () => {
.should('be.greaterThan', firstHeight);
});
});

it.only('accounts for a horizontal scrollbar', () => {
Comment thread
chandlerprall marked this conversation as resolved.
Outdated
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
);
Comment thread
chandlerprall marked this conversation as resolved.
Outdated
});
});
});

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