diff --git a/CHANGELOG.md b/CHANGELOG.md index ca1ce825062..3c68be64760 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,7 @@ ## [`main`](https://github.com/elastic/eui/tree/main) +- Updated `EuiDataGrid`s with scrolling content to always have a border around the grid body and any scrollbars ([#5563](https://github.com/elastic/eui/pull/5563)) + **Bug fixes** - Fixed EuiDataGrid height issue when in full-screen mode and with scrolling content ([#5557](https://github.com/elastic/eui/pull/5557)) diff --git a/src-docs/src/views/datagrid/container.js b/src-docs/src/views/datagrid/container.js index f1acb4d80d7..b75064fcbaa 100644 --- a/src-docs/src/views/datagrid/container.js +++ b/src-docs/src/views/datagrid/container.js @@ -52,7 +52,10 @@ export default () => { ); return ( - + { Sidebar - + = ( const outerGridRef = useRef(null); // container that becomes scrollable const innerGridRef = useRef(null); // container sized to fit all content + /** + * Scroll bars + */ + const { + scrollBarHeight, + hasVerticalScroll, + hasHorizontalScroll, + scrollBorderOverlay, + } = useScrollBars(outerGridRef, gridStyles.border); + /** * Widths */ @@ -364,7 +374,7 @@ export const EuiDataGridBody: FunctionComponent = ( useScroll({ gridRef, outerGridRef, - innerGridRef, + hasGridScrolling: hasVerticalScroll || hasHorizontalScroll, headerRowHeight, footerRowHeight, visibleRowCount, @@ -402,7 +412,7 @@ export const EuiDataGridBody: FunctionComponent = ( defaultRowHeight, headerRowHeight, footerRowHeight, - outerGridRef, + scrollBarHeight, innerGridRef, }); @@ -486,6 +496,7 @@ export const EuiDataGridBody: FunctionComponent = ( > {Cell} + {scrollBorderOverlay} ) : null; }; diff --git a/src/components/datagrid/controls/_data_grid_toolbar.scss b/src/components/datagrid/controls/_data_grid_toolbar.scss index c9144561a4f..656c307cfe8 100644 --- a/src/components/datagrid/controls/_data_grid_toolbar.scss +++ b/src/components/datagrid/controls/_data_grid_toolbar.scss @@ -1,7 +1,7 @@ .euiDataGrid__controls { background: $euiPageBackgroundColor; position: relative; - z-index: 3; // Needs to sit above the content blow that sits below it + z-index: 2; // Needs to sit above the content below it border: $euiBorderThin; padding: $euiSizeXS $euiSizeXS $euiSizeXS 0; display: flex; diff --git a/src/components/datagrid/utils/grid_height_width.ts b/src/components/datagrid/utils/grid_height_width.ts index a5c37a116b0..22946b6d136 100644 --- a/src/components/datagrid/utils/grid_height_width.ts +++ b/src/components/datagrid/utils/grid_height_width.ts @@ -72,7 +72,7 @@ export const useUnconstrainedHeight = ({ defaultRowHeight, headerRowHeight, footerRowHeight, - outerGridRef, + scrollBarHeight, innerGridRef, }: { rowHeightUtils: RowHeightUtils; @@ -82,7 +82,7 @@ export const useUnconstrainedHeight = ({ defaultRowHeight: number; headerRowHeight: number; footerRowHeight: number; - outerGridRef: React.MutableRefObject; + scrollBarHeight: number; innerGridRef: React.MutableRefObject; }) => { const { getCorrectRowIndex } = useContext(DataGridSortingContext); @@ -127,21 +127,12 @@ export const useUnconstrainedHeight = ({ ); useUpdateEffect(forceRender, [innerWidth]); - // https://stackoverflow.com/a/5038256 - const hasHorizontalScroll = - (outerGridRef.current?.scrollWidth ?? 0) > - (outerGridRef.current?.clientWidth ?? 0); - // https://stackoverflow.com/a/24797425 - const scrollbarHeight = hasHorizontalScroll - ? outerGridRef.current!.offsetHeight - outerGridRef.current!.clientHeight - : 0; - const unconstrainedHeight = defaultRowHeight * (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 - scrollbarHeight; // account for horizontal scrollbar + scrollBarHeight; // account for horizontal scrollbar return unconstrainedHeight; }; diff --git a/src/components/datagrid/utils/scrolling.test.ts b/src/components/datagrid/utils/scrolling.test.tsx similarity index 60% rename from src/components/datagrid/utils/scrolling.test.ts rename to src/components/datagrid/utils/scrolling.test.tsx index b11c65bb524..137f1060467 100644 --- a/src/components/datagrid/utils/scrolling.test.ts +++ b/src/components/datagrid/utils/scrolling.test.tsx @@ -6,8 +6,10 @@ * Side Public License, v 1. */ +import React from 'react'; +import { render } from 'enzyme'; import { testCustomHook } from '../../../test/test_custom_hook.test_helper'; -import { useScrollCellIntoView } from './scrolling'; +import { useScrollCellIntoView, useScrollBars } from './scrolling'; // see scrolling.spec.tsx for E2E useScroll tests @@ -38,12 +40,7 @@ describe('useScrollCellIntoView', () => { querySelector: getCell, } as any, }, - innerGridRef: { - current: { - offsetHeight: 800, - offsetWidth: 1000, - } as any, - }, + hasGridScrolling: true, headerRowHeight: 0, footerRowHeight: 0, visibleRowCount: 100, @@ -61,32 +58,17 @@ describe('useScrollCellIntoView', () => { ...args, gridRef: { current: null }, outerGridRef: { current: null }, - innerGridRef: { current: null }, }) ); scrollCellIntoView({ rowIndex: 0, colIndex: 0 }); expect(scrollTo).not.toHaveBeenCalled(); }); - it('does nothing if the grid does not scroll (inner and outer grid dimensions are the same)', () => { - const outerGrid = { - offsetHeight: 500, - offsetWidth: 500, - }; - const innerGrid = { - offsetHeight: 500, - offsetWidth: 500, - }; - + it('does nothing if the grid does not scroll', () => { const { scrollCellIntoView } = testCustomHook(() => useScrollCellIntoView({ ...args, - outerGridRef: { - current: { ...args.outerGridRef.current, ...outerGrid }, - }, - innerGridRef: { - current: { ...args.innerGridRef.current, ...innerGrid }, - }, + hasGridScrolling: false, }) ); scrollCellIntoView({ rowIndex: 0, colIndex: 0 }); @@ -339,3 +321,217 @@ describe('useScrollCellIntoView', () => { }); }); }); + +describe('useScrollBars', () => { + const mockOuterGrid = { + clientHeight: 0, + offsetHeight: 0, + scrollHeight: 0, + clientWidth: 0, + offsetWidth: 0, + scrollWidth: 0, + } as any; + + describe('scrollBarHeight', () => { + it("is derived by the difference between the grid's offsetHeight vs clientHeight", () => { + const { scrollBarHeight } = testCustomHook(() => + useScrollBars({ + current: { ...mockOuterGrid, clientHeight: 40, offsetHeight: 50 }, + }) + ); + + expect(scrollBarHeight).toEqual(10); + }); + }); + + describe('scrollBarWidth', () => { + it('is zero if there is no difference between offsetWidth and clientWidth', () => { + const { scrollBarWidth } = testCustomHook(() => + useScrollBars({ + current: { ...mockOuterGrid, clientWidth: 40, offsetWidth: 40 }, + }) + ); + + expect(scrollBarWidth).toEqual(0); + }); + }); + + describe('hasVerticalScroll', () => { + it("has scrolling overflow if the grid's scrollHeight exceeds its clientHeight", () => { + const { hasVerticalScroll } = testCustomHook(() => + useScrollBars({ + current: { ...mockOuterGrid, clientHeight: 50, scrollHeight: 100 }, + }) + ); + + expect(hasVerticalScroll).toEqual(true); + }); + + it("does not have scrolling overflow if the the grid's scrollHeight is the same as its clientHeight", () => { + const { hasVerticalScroll } = testCustomHook(() => + useScrollBars({ + current: { ...mockOuterGrid, clientHeight: 50, scrollHeight: 50 }, + }) + ); + + expect(hasVerticalScroll).toEqual(false); + }); + }); + + describe('hasHorizontalScroll', () => { + it("has scrolling overflow if the grid's scrollWidth exceeds its clientWidth", () => { + const { hasHorizontalScroll } = testCustomHook(() => + useScrollBars({ + current: { ...mockOuterGrid, clientWidth: 100, scrollWidth: 200 }, + }) + ); + + expect(hasHorizontalScroll).toEqual(true); + }); + + it("does not have scrolling overflow if the the grid's scrollWidth is the same as its clientWidth", () => { + const { hasHorizontalScroll } = testCustomHook(() => + useScrollBars({ + current: { ...mockOuterGrid, clientWidth: 200, scrollWidth: 200 }, + }) + ); + + expect(hasHorizontalScroll).toEqual(false); + }); + }); + + describe('scrollBorderOverlay', () => { + describe('if the grid does not scroll', () => { + it('does not render anything', () => { + const { scrollBorderOverlay } = testCustomHook(() => + useScrollBars({ + current: { + ...mockOuterGrid, + clientHeight: 100, + scrollHeight: 100, + clientWidth: 200, + scrollWidth: 200, + }, + }) + ); + + expect(scrollBorderOverlay).toEqual(null); + }); + }); + + describe('if the grid does not display borders', () => { + it('does not render anything', () => { + const { scrollBorderOverlay } = testCustomHook(() => + useScrollBars( + { + current: { + ...mockOuterGrid, + clientHeight: 50, + scrollHeight: 100, + }, + }, + 'none' + ) + ); + + expect(scrollBorderOverlay).toEqual(null); + }); + }); + + describe('if the grid scrolls but has inline scrollbars & no scrollbar width/height', () => { + it('renders a single overlay with borders for the outermost grid', () => { + const { scrollBorderOverlay } = testCustomHook(() => + useScrollBars({ + current: { + ...mockOuterGrid, + clientHeight: 50, + offsetHeight: 50, + scrollHeight: 100, + clientWidth: 100, + offsetWidth: 100, + scrollWidth: 200, + }, + }) + ); + const component = render(<>{scrollBorderOverlay}); + + expect(component).toMatchInlineSnapshot(` +