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.

Sign up for GitHub

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

Update to React 18.3.1 and fix two React warnings in Table #922

Merged
merged 2 commits into from
Oct 17, 2024
Merged

Update to React 18.3.1 and fix two React warnings in Table #922

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
427d3cd
Fix React warning about spreading key in props
gabro Oct 16, 2024
72730e9
Replace useLayoutEffect with useEffect in Table
gabro Oct 16, 2024
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: 2 additions & 2 deletions packages/bento-design-system/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,8 +190,8 @@
"playroom": "0.32.1",
"postcss": "8.4.47",
"prettier": "2.8.8",
"react": "18.2.0",
"react-dom": "18.2.0",
"react": "18.3.1",
"react-dom": "18.3.1",
"storybook": "8.0.8",
"style-loader": "3.3.4",
"ts-loader": "9.4.4",
Expand Down
43 changes: 24 additions & 19 deletions packages/bento-design-system/src/Table/Table.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ import {
GridWidth,
Row as RowType,
} from "./types";
import { useLayoutEffect, useMemo, useState, CSSProperties, useEffect, useRef } from "react";
import { useMemo, useState, CSSProperties, useEffect, useRef } from "react";
import { match, __ } from "ts-pattern";
import { useBentoConfig } from "../BentoConfigContext";
import { assignInlineVars } from "@vanilla-extract/dynamic";
Expand Down Expand Up @@ -233,12 +233,12 @@ export function Table<
.map((c) => c.id ?? c.accessor)
.indexOf(stickyLeftColumnsIds[stickyLeftColumnsIds.length - 1]);

// Keep a style object for each sticky column, which will be updated by the useLayoutEffect below
// Keep a style object for each sticky column, which will be updated by the useEffect below
const [stickyLeftColumnStyle, setStickyLeftColumnStyle] = useState(
{} as Record<string, CSSProperties>
);

// Keep a state for the height of the first row of headers, which will be updated by the useLayoutEffect below
// Keep a state for the height of the first row of headers, which will be updated by the useEffect below
const [stickyHeaderHeight, setStickyHeaderHeight] = useState(0);

const tableContainerRef = useRef<HTMLDivElement>(null);
Expand All @@ -249,7 +249,7 @@ export function Table<
/** Get the width of each sticky column (using the header width as reference) and use it to set the `left` of each sticky column.
* Each sticky column must have as `left` the total width of the previous sticky columns.
*/
useLayoutEffect(() => {
useEffect(() => {
// Make this computation only if we have any data, because headers are not rendered when there are no rows
// and we need them to get the column width.
if (data.length > 0) {
Expand Down Expand Up @@ -375,20 +375,24 @@ export function Table<
rowIndex: number,
interactiveRow: boolean
) {
return cells.map((cell, index) => (
<CellContainer
{...cell.getCellProps()}
index={rowIndex}
lastLeftSticky={index === lastStickyColumnIndex}
style={stickyLeftColumnStyle[cell.column.id]}
first={index === 0}
last={(index + 1) % flatColumns.length === 0}
interactiveRow={interactiveRow}
withDividers={withDividers}
>
{cell.render("Cell")}
</CellContainer>
));
return cells.map((cell, index) => {
const { key, ...cellProps } = cell.getCellProps();
return (
<CellContainer
key={key}
{...cellProps}
index={rowIndex}
lastLeftSticky={index === lastStickyColumnIndex}
style={stickyLeftColumnStyle[cell.column.id]}
first={index === 0}
last={(index + 1) % flatColumns.length === 0}
interactiveRow={interactiveRow}
withDividers={withDividers}
>
{cell.render("Cell")}
</CellContainer>
);
});
}

const rowsToRender = virtualizeRows
Expand Down Expand Up @@ -581,6 +585,7 @@ function ColumnHeader<D extends Record<string, unknown>>({
) : null;

const hasHeaderContent = column.Header || hint || sortIcon;
const { key: _key, ...headerProps } = column.getHeaderProps(column.getSortByToggleProps());

return (
<Box
Expand All @@ -598,7 +603,7 @@ function ColumnHeader<D extends Record<string, unknown>>({
className={[columnHeader({ withDividers, first, lastLeftSticky })]}
background={config.headerBackgroundColor}
color={config.headerForegroundColor}
{...column.getHeaderProps(column.getSortByToggleProps())}
{...headerProps}
textAlign={column.align}
{...config.padding.header}
>
Expand Down
Loading
Loading