Skip to content

Commit

Permalink
abandon fixed row height virtualization because its buggy
Browse files Browse the repository at this point in the history
  • Loading branch information
KevinVandy committed Jan 1, 2024
1 parent 5a3de30 commit 8da153f
Show file tree
Hide file tree
Showing 13 changed files with 1,099 additions and 1,141 deletions.
20 changes: 10 additions & 10 deletions apps/material-react-table-docs/package.json
Expand Up @@ -14,19 +14,19 @@
},
"dependencies": {
"@docsearch/js": "3.5.2",
"@emotion/react": "^11.11.1",
"@emotion/react": "^11.11.3",
"@emotion/styled": "^11.11.0",
"@faker-js/faker": "^8.3.1",
"@fortawesome/fontawesome-svg-core": "^6.5.1",
"@fortawesome/free-solid-svg-icons": "^6.5.1",
"@fortawesome/react-fontawesome": "^0.2.0",
"@mdx-js/loader": "^3.0.0",
"@mdx-js/react": "^3.0.0",
"@mui/icons-material": "^5.15.1",
"@mui/material": "^5.15.1",
"@mui/x-date-pickers": "^6.18.5",
"@mui/icons-material": "^5.15.2",
"@mui/material": "^5.15.2",
"@mui/x-date-pickers": "^6.18.6",
"@next/mdx": "^14.0.4",
"@tanstack/react-query": "^5.14.2",
"@tanstack/react-query": "^5.17.0",
"@types/mdx": "^2.0.10",
"dayjs": "^1.11.10",
"export-to-csv": "^1.2.2",
Expand All @@ -40,12 +40,12 @@
"react-dom": "18.2.0"
},
"devDependencies": {
"@tanstack/eslint-plugin-query": "^5.12.1",
"@types/node": "^20.10.5",
"@types/react": "^18.2.45",
"@tanstack/eslint-plugin-query": "^5.14.6",
"@types/node": "^20.10.6",
"@types/react": "^18.2.46",
"@types/react-dom": "^18.2.18",
"@typescript-eslint/eslint-plugin": "^6.15.0",
"@typescript-eslint/parser": "^6.15.0",
"@typescript-eslint/eslint-plugin": "^6.16.0",
"@typescript-eslint/parser": "^6.16.0",
"eslint": "8.56.0",
"eslint-config-next": "14.0.4",
"next-plausible": "^3.12.0",
Expand Down
12 changes: 6 additions & 6 deletions apps/test-cra/package.json
Expand Up @@ -3,14 +3,14 @@
"version": "0.1.0",
"private": true,
"dependencies": {
"@emotion/react": "^11.11.1",
"@emotion/react": "^11.11.3",
"@emotion/styled": "^11.11.0",
"@mui/icons-material": "^5.15.1",
"@mui/material": "^5.15.1",
"@mui/x-date-pickers": "^6.18.5",
"@testing-library/jest-dom": "^6.1.5",
"@mui/icons-material": "^5.15.2",
"@mui/material": "^5.15.2",
"@mui/x-date-pickers": "^6.18.6",
"@testing-library/jest-dom": "^6.1.6",
"@testing-library/react": "^14.1.2",
"@testing-library/user-event": "^14.5.1",
"@testing-library/user-event": "^14.5.2",
"material-react-table": "workspace:*",
"react": "^18.2.0",
"react-dom": "^18.2.0",
Expand Down
22 changes: 11 additions & 11 deletions apps/test-remix/package.json
Expand Up @@ -10,24 +10,24 @@
"typecheck": "tsc"
},
"dependencies": {
"@emotion/react": "^11.11.1",
"@emotion/react": "^11.11.3",
"@emotion/styled": "^11.11.0",
"@mui/icons-material": "^5.15.1",
"@mui/material": "^5.15.1",
"@mui/x-date-pickers": "^6.18.5",
"@remix-run/css-bundle": "^2.4.0",
"@remix-run/node": "^2.4.0",
"@remix-run/react": "^2.4.0",
"@remix-run/serve": "^2.4.0",
"@mui/icons-material": "^5.15.2",
"@mui/material": "^5.15.2",
"@mui/x-date-pickers": "^6.18.6",
"@remix-run/css-bundle": "^2.4.1",
"@remix-run/node": "^2.4.1",
"@remix-run/react": "^2.4.1",
"@remix-run/serve": "^2.4.1",
"isbot": "^4.1.0",
"material-react-table": "workspace:*",
"react": "^18.2.0",
"react-dom": "^18.2.0"
},
"devDependencies": {
"@remix-run/dev": "^2.4.0",
"@remix-run/eslint-config": "^2.4.0",
"@types/react": "^18.2.45",
"@remix-run/dev": "^2.4.1",
"@remix-run/eslint-config": "^2.4.1",
"@types/react": "^18.2.46",
"@types/react-dom": "^18.2.18",
"eslint": "^8.56.0",
"typescript": "^5.3.3"
Expand Down
14 changes: 7 additions & 7 deletions apps/test-vite/package.json
Expand Up @@ -10,20 +10,20 @@
"preview": "vite preview"
},
"dependencies": {
"@emotion/react": "^11.11.1",
"@emotion/react": "^11.11.3",
"@emotion/styled": "^11.11.0",
"@mui/icons-material": "^5.15.1",
"@mui/material": "^5.15.1",
"@mui/x-date-pickers": "^6.18.5",
"@mui/icons-material": "^5.15.2",
"@mui/material": "^5.15.2",
"@mui/x-date-pickers": "^6.18.6",
"material-react-table": "workspace:*",
"react": "^18.2.0",
"react-dom": "^18.2.0"
},
"devDependencies": {
"@types/react": "^18.2.45",
"@types/react": "^18.2.46",
"@types/react-dom": "^18.2.18",
"@typescript-eslint/eslint-plugin": "^6.15.0",
"@typescript-eslint/parser": "^6.15.0",
"@typescript-eslint/eslint-plugin": "^6.16.0",
"@typescript-eslint/parser": "^6.16.0",
"@vitejs/plugin-react": "^4.2.1",
"eslint": "^8.56.0",
"eslint-plugin-react-hooks": "^4.6.0",
Expand Down
18 changes: 9 additions & 9 deletions packages/material-react-table/package.json
Expand Up @@ -61,14 +61,14 @@
"storybook:dev": "storybook dev -p 6006"
},
"devDependencies": {
"@babel/core": "^7.23.6",
"@babel/core": "^7.23.7",
"@babel/preset-react": "^7.23.3",
"@emotion/react": "^11.11.1",
"@emotion/react": "^11.11.3",
"@emotion/styled": "^11.11.0",
"@faker-js/faker": "^8.3.1",
"@mui/icons-material": "^5.15.1",
"@mui/material": "^5.15.1",
"@mui/x-date-pickers": "^6.18.5",
"@mui/icons-material": "^5.15.2",
"@mui/material": "^5.15.2",
"@mui/x-date-pickers": "^6.18.6",
"@rollup/plugin-typescript": "^11.1.5",
"@size-limit/preset-small-lib": "^11.0.1",
"@storybook/addon-a11y": "^7.6.6",
Expand All @@ -80,11 +80,11 @@
"@storybook/react": "^7.6.6",
"@storybook/react-vite": "^7.6.6",
"@storybook/testing-library": "^0.2.2",
"@types/node": "^20.10.5",
"@types/react": "^18.2.45",
"@types/node": "^20.10.6",
"@types/react": "^18.2.46",
"@types/react-dom": "^18.2.18",
"@typescript-eslint/eslint-plugin": "^6.15.0",
"@typescript-eslint/parser": "^6.15.0",
"@typescript-eslint/eslint-plugin": "^6.16.0",
"@typescript-eslint/parser": "^6.16.0",
"@vitejs/plugin-react": "^4.2.1",
"eslint": "^8.56.0",
"eslint-plugin-mui-path-imports": "^0.0.15",
Expand Down
3 changes: 1 addition & 2 deletions packages/material-react-table/src/body/MRT_TableBody.tsx
Expand Up @@ -45,7 +45,6 @@ export const MRT_TableBody = <TData extends MRT_RowData>({
muiTableBodyProps,
renderEmptyRowsFallback,
rowPinningDisplayMode,
rowVirtualizationDisplayMode,
},
refs: { tableFooterRef, tableHeadRef, tablePaperRef },
} = table;
Expand Down Expand Up @@ -120,7 +119,7 @@ export const MRT_TableBody = <TData extends MRT_RowData>({
sx={(theme) => ({
display: layoutMode?.startsWith('grid') ? 'grid' : undefined,
height:
rowVirtualizer && rowVirtualizationDisplayMode === 'dynamic'
rowVirtualizer
? `${rowVirtualizer.getTotalSize()}px`
: undefined,
minHeight: !rows.length ? '100px' : undefined,
Expand Down
25 changes: 5 additions & 20 deletions packages/material-react-table/src/body/MRT_TableBodyRow.tsx
Expand Up @@ -60,7 +60,6 @@ export const MRT_TableBodyRow = <TData extends MRT_RowData>({
muiTableBodyRowProps,
renderDetailPanel,
rowPinningDisplayMode,
rowVirtualizationDisplayMode,
},
refs: { tableFooterRef, tableHeadRef },
setHoveredRow,
Expand Down Expand Up @@ -116,9 +115,6 @@ export const MRT_TableBodyRow = <TData extends MRT_RowData>({

const rowHeight = customRowHeight || defaultRowHeight;

const isDynamicVirtualRow =
virtualRow && rowVirtualizationDisplayMode === 'dynamic';

const handleDragEnter = (_e: DragEvent) => {
if (enableRowOrdering && draggingRow) {
setHoveredRow(row);
Expand All @@ -143,20 +139,14 @@ export const MRT_TableBodyRow = <TData extends MRT_RowData>({
ref={(node: HTMLTableRowElement) => {
if (node) {
rowRef.current = node;
if (isDynamicVirtualRow) {
measureElement?.(node);
}
measureElement?.(node);
}
}}
selected={row.getIsSelected()}
{...tableRowProps}
style={{
transform: virtualRow
? `translateY(${
isDynamicVirtualRow
? virtualRow.start
: virtualRow.start - rowIndex * virtualRow.size
}px)`
? `translateY(${virtualRow.start}px)`
: undefined,
...tableRowProps?.style,
}}
Expand Down Expand Up @@ -186,7 +176,7 @@ export const MRT_TableBodyRow = <TData extends MRT_RowData>({
: draggingRow?.id === row.id || hoveredRow?.id === row.id
? 0.5
: 1,
position: isDynamicVirtualRow
position: virtualRow
? 'absolute'
: rowPinningDisplayMode?.includes('sticky') && isPinned
? 'sticky'
Expand All @@ -198,26 +188,21 @@ export const MRT_TableBodyRow = <TData extends MRT_RowData>({
? pinnedRowBackgroundColor
: undefined,
},
top: isDynamicVirtualRow
top: virtualRow
? 0
: topPinnedIndex !== undefined && isPinned
? `${
topPinnedIndex * rowHeight +
(enableStickyHeader || isFullScreen ? tableHeadHeight - 1 : 0)
}px`
: undefined,
transition: isDynamicVirtualRow ? 'none' : 'all 150ms ease-in-out',
transition: virtualRow ? 'none' : 'all 150ms ease-in-out',
width: '100%',
zIndex:
rowPinningDisplayMode?.includes('sticky') && isPinned
? 2
: undefined,
...(sx as any),
height:
(virtualRow && !isDynamicVirtualRow) ||
customRowHeight
? `${customRowHeight ?? virtualRow?.size}px`
: undefined,
})}
>
{virtualPaddingLeft ? (
Expand Down
Expand Up @@ -22,7 +22,6 @@ export const useMRT_RowVirtualizer = <
getState,
options: {
enableRowVirtualization,
rowVirtualizationDisplayMode,
rowVirtualizerInstanceRef,
rowVirtualizerOptions,
},
Expand All @@ -41,7 +40,6 @@ export const useMRT_RowVirtualizer = <
density === 'compact' ? 37 : density === 'comfortable' ? 58 : 73,
getScrollElement: () => tableContainerRef.current,
measureElement:
rowVirtualizationDisplayMode === 'dynamic' &&
typeof window !== 'undefined' &&
navigator.userAgent.indexOf('Firefox') === -1
? (element) => element?.getBoundingClientRect().height
Expand Down
Expand Up @@ -90,7 +90,6 @@ export const useMRT_TableOptions: <TData extends MRT_RowData>(
positionToolbarDropZone = 'top',
rowNumberDisplayMode = 'static',
rowPinningDisplayMode = 'sticky',
rowVirtualizationDisplayMode = 'dynamic',
selectAllMode = 'page',
sortingFns,
...rest
Expand Down Expand Up @@ -130,9 +129,7 @@ export const useMRT_TableOptions: <TData extends MRT_RowData>(
layoutMode || (enableColumnResizing ? 'grid-no-grow' : 'semantic');
if (
layoutMode === 'semantic' &&
((rest.enableRowVirtualization &&
rowVirtualizationDisplayMode === 'dynamic') ||
rest.enableColumnVirtualization)
(rest.enableRowVirtualization || rest.enableColumnVirtualization)
) {
layoutMode = 'grid';
}
Expand Down Expand Up @@ -207,7 +204,6 @@ export const useMRT_TableOptions: <TData extends MRT_RowData>(
positionToolbarDropZone,
rowNumberDisplayMode,
rowPinningDisplayMode,
rowVirtualizationDisplayMode,
selectAllMode,
sortingFns: _sortingFns,
...rest,
Expand Down
57 changes: 18 additions & 39 deletions packages/material-react-table/src/table/MRT_Table.tsx
@@ -1,5 +1,4 @@
import { type ReactNode, useMemo } from 'react';
import Box from '@mui/material/Box';
import { useMemo } from 'react';
import Table, { type TableProps } from '@mui/material/Table';
import { MRT_TableBody, Memo_MRT_TableBody } from '../body/MRT_TableBody';
import { parseFromValuesOrFunc } from '../column.utils';
Expand Down Expand Up @@ -29,7 +28,6 @@ export const MRT_Table = <TData extends MRT_RowData>({
layoutMode,
memoMode,
muiTableProps,
rowVirtualizationDisplayMode,
},
} = table;
const { columnSizing, columnSizingInfo, columnVisibility, isFullScreen } =
Expand Down Expand Up @@ -77,42 +75,23 @@ export const MRT_Table = <TData extends MRT_RowData>({
};

return (
<MRT_FixedVirtualTableContainer
height={
rowVirtualizer && rowVirtualizationDisplayMode === 'fixed'
? rowVirtualizer.getTotalSize()
: undefined
}
<Table
stickyHeader={enableStickyHeader || isFullScreen}
{...tableProps}
style={{ ...columnSizeVars, ...tableProps?.style }}
sx={(theme) => ({
borderCollapse: 'separate',
display: layoutMode?.startsWith('grid') ? 'grid' : undefined,
...(parseFromValuesOrFunc(tableProps?.sx, theme) as any),
})}
>
<Table
stickyHeader={enableStickyHeader || isFullScreen}
{...tableProps}
style={{ ...columnSizeVars, ...tableProps?.style }}
sx={(theme) => ({
borderCollapse: 'separate',
display: layoutMode?.startsWith('grid') ? 'grid' : undefined,
...(parseFromValuesOrFunc(tableProps?.sx, theme) as any),
})}
>
{enableTableHead && <MRT_TableHead {...commonTableGroupProps} />}
{memoMode === 'table-body' || columnSizingInfo.isResizingColumn ? (
<Memo_MRT_TableBody {...commonTableBodyProps} />
) : (
<MRT_TableBody {...commonTableBodyProps} />
)}
{enableTableFooter && <MRT_TableFooter {...commonTableGroupProps} />}
</Table>
</MRT_FixedVirtualTableContainer>
{enableTableHead && <MRT_TableHead {...commonTableGroupProps} />}
{memoMode === 'table-body' || columnSizingInfo.isResizingColumn ? (
<Memo_MRT_TableBody {...commonTableBodyProps} />
) : (
<MRT_TableBody {...commonTableBodyProps} />
)}
{enableTableFooter && <MRT_TableFooter {...commonTableGroupProps} />}
</Table>
);
};

export const MRT_FixedVirtualTableContainer = ({
children,
height,
}: {
children: ReactNode;
height?: number;
}) => {
if (!height) return children;
return <Box sx={{ height: `${height}px` }}>{children}</Box>;
};
1 change: 0 additions & 1 deletion packages/material-react-table/src/types.ts
Expand Up @@ -1128,7 +1128,6 @@ export type MRT_TableOptions<TData extends MRT_RowData> = Omit<
| 'sticky'
| 'top'
| 'top-and-bottom';
rowVirtualizationDisplayMode?: 'dynamic' | 'fixed';
rowVirtualizerInstanceRef?: MutableRefObject<Virtualizer<
HTMLDivElement,
HTMLTableRowElement
Expand Down

2 comments on commit 8da153f

@vercel
Copy link

@vercel vercel bot commented on 8da153f Jan 1, 2024

Choose a reason for hiding this comment

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

@vercel
Copy link

@vercel vercel bot commented on 8da153f Jan 1, 2024

Choose a reason for hiding this comment

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

Please sign in to comment.