Skip to content

Fix and reimplement: Expand cell height if there is only one row (Original #537, Revert #548) #554

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

Merged
merged 6 commits into from
Mar 3, 2025
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
23 changes: 21 additions & 2 deletions src/components/Grid/Cell.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { memo } from "react";
import { memo, useEffect, useRef } from "react";
import { GridChildComponentProps, areEqual } from "react-window";
import { ItemDataType } from "./types";
import { StyledCell } from "./StyledCell";
Expand All @@ -19,6 +19,8 @@ export const Cell = memo(
showHeader,
rowHeight,
rowStart,
rowAutoHeight,
updateRowHeight,
} = data;

const currentRowIndex = rowIndex + rowStart;
Expand Down Expand Up @@ -54,11 +56,27 @@ export const Cell = memo(
const selectionBorderLeft = rightOfSelectionBorder || rightOfFocus || isFocused;
const selectionBorderTop = belowSelectionBorder || belowFocus || isFocused;

const cellRef = useRef<HTMLDivElement>(null);

useEffect(() => {
if (!rowAutoHeight || !cellRef.current) {
return;
}
const height = cellRef.current.getBoundingClientRect().height;
updateRowHeight(rowIndex, height);
}, [updateRowHeight, rowIndex, rowAutoHeight]);

const styleWithHeight = {
...style,
height: "auto",
};

return (
<div
style={style}
style={styleWithHeight}
data-row={currentRowIndex}
data-column={columnIndex}
ref={cellRef}
>
<StyledCell
as={CellData}
Expand All @@ -79,6 +97,7 @@ export const Cell = memo(
data-grid-row={currentRowIndex}
data-grid-column={columnIndex}
$showBorder
$rowAutoHeight={rowAutoHeight}
{...props}
/>
</div>
Expand Down
2 changes: 2 additions & 0 deletions src/components/Grid/Grid.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ interface Props {
row: number;
column: number;
};
rowAutoHeight?: boolean;
}
const Grid = ({ columnCount, rowCount, focus: focusProp, ...props }: Props) => {
const [focus, setFocus] = useState(focusProp);
Expand Down Expand Up @@ -78,6 +79,7 @@ const Grid = ({ columnCount, rowCount, focus: focusProp, ...props }: Props) => {
});
}}
getMenuOptions={getMenuOptions}
rowAutoHeight={props.rowAutoHeight}
{...props}
/>
</div>
Expand Down
48 changes: 47 additions & 1 deletion src/components/Grid/Grid.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,29 @@ import { SelectionFocus } from "./types";
import { ReactNode } from "react";

const Cell: CellProps = ({ type, rowIndex, columnIndex, isScrolling, ...props }) => {
let content = `${rowIndex} ${columnIndex} - ${type}`;

if (rowIndex === 0 && columnIndex === 0) {
content = `CREATE TABLE random_user_events (
user_id UInt32,
event_time DateTime,
event_type Enum8('click' = 1, 'view' = 2, 'purchase' = 3),
item_id String,
price Decimal(10,2),
quantity UInt16
) ENGINE = MergeTree()
ORDER BY (user_id, event_time)
PARTITION BY toYYYYMM(event_time)
SETTINGS index_granularity = 8192;`;
}

return (
<div
data-scrolling={isScrolling}
{...props}
data-testid={`${type}-${rowIndex ?? "x"}-${columnIndex ?? "x"}`}
>
{rowIndex} {columnIndex} - {type}
{content}
</div>
);
};
Expand All @@ -31,6 +47,7 @@ interface Props
focus?: SelectionFocus;
onFocusChange?: (rowIndex: number, columnIndex: number) => void;
onColumnResize?: () => void;
rowAutoHeight?: boolean;
}
type AutoSizerModule = typeof import("react-virtualized-auto-sizer");

Expand Down Expand Up @@ -60,6 +77,7 @@ describe("Grid", () => {
onColumnResize,
focus,
onFocusChange,
rowAutoHeight,
...props
}: Props) =>
renderCUI(
Expand All @@ -72,6 +90,7 @@ describe("Grid", () => {
onColumnResize={onColumnResize ?? onColumnResizeTestFn}
onFocusChange={onFocusChange ?? onFocusChangeTestFn}
getMenuOptions={getMenuOptions}
rowAutoHeight={rowAutoHeight}
{...props}
/>
);
Expand Down Expand Up @@ -99,4 +118,31 @@ describe("Grid", () => {
cell && expect(cell.dataset.selected).toEqual("true");
cell && expect(cell.dataset.focused).toEqual("true");
});

it("should set row height to default (33px) when rowAutoHeight is false", async () => {
const { getByTestId } = renderGrid({
rowCount: 10,
columnCount: 10,
rowAutoHeight: false,
});

const cell = getByTestId("row-cell-0-0");

const computedHeight = window.getComputedStyle(cell).height;
const heightValue = parseFloat(computedHeight);
expect(heightValue).toBe(33);
});

it("should expand row height to 100% when rowAutoHeight is true", async () => {
const { getByTestId } = renderGrid({
rowCount: 1,
columnCount: 1,
rowAutoHeight: true,
});

const cell = getByTestId("row-cell-0-0");

const computedHeight = window.getComputedStyle(cell).height;
expect(computedHeight).toBe("100%");
});
});
45 changes: 43 additions & 2 deletions src/components/Grid/Grid.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,7 @@ export const Grid = forwardRef<HTMLDivElement, GridProps>(
onContextMenu: onContextMenuProp,
forwardedGridRef,
onItemsRendered: onItemsRenderedProp,
rowAutoHeight,
...props
},
forwardedRef
Expand Down Expand Up @@ -261,6 +262,36 @@ export const Grid = forwardRef<HTMLDivElement, GridProps>(
);
const resizingState = useResizingState();

const [rowHeights, setRowHeights] = useState<{ [key: number]: number }>({});

const getRowHeight = useCallback(
(index: number) => {
if (rowAutoHeight && rowHeights[index] !== undefined) {
return rowHeights[index] + rowHeight;
}
return rowHeight;
},
[rowHeight, rowAutoHeight, rowHeights]
);

const updateRowHeight = useCallback(
(rowIndex: number, height: number) => {
if (!rowAutoHeight) {
return;
}

setRowHeights(prevRowHeights => {
if (height > (prevRowHeights[rowIndex] || 0) && gridRef.current) {
const newRowHeights = { ...prevRowHeights, [rowIndex]: height };
gridRef.current.resetAfterRowIndex(rowIndex);
return newRowHeights;
}
return prevRowHeights;
});
},
[rowAutoHeight, gridRef]
);

const onFocusChange = useCallback(
(row: number, column: number) => {
setFocus(focus => ({
Expand Down Expand Up @@ -405,6 +436,9 @@ export const Grid = forwardRef<HTMLDivElement, GridProps>(
headerHeight,
rowNumberWidth,
rowStart,
rowAutoHeight,
updateRowHeight,
getRowHeight,
};

const InnerElementType = forwardRef<HTMLDivElement, InnerElementTypeTypes>(
Expand Down Expand Up @@ -435,6 +469,7 @@ export const Grid = forwardRef<HTMLDivElement, GridProps>(
showHeader={showHeader}
rowStart={rowStart}
showBorder={showBorder}
rowAutoHeight={rowAutoHeight}
/>
)}

Expand Down Expand Up @@ -755,7 +790,6 @@ export const Grid = forwardRef<HTMLDivElement, GridProps>(
const onItemsRendered = useCallback(
(props: GridOnItemsRenderedProps) => {
lastItemsRenderedProps.current = props;

return onItemsRenderedProp?.({
...props,
visibleRowStartIndex: props.visibleRowStartIndex + rowStart,
Expand Down Expand Up @@ -786,6 +820,13 @@ export const Grid = forwardRef<HTMLDivElement, GridProps>(
);
};

// Handles the case when rowCount/columnCount changes, rerenders styles
useEffect(() => {
if (gridRef.current) {
gridRef.current.resetAfterRowIndex(0);
}
}, [rowCount, columnCount]);

return (
<ContextMenu
modal={false}
Expand Down Expand Up @@ -820,7 +861,7 @@ export const Grid = forwardRef<HTMLDivElement, GridProps>(
height={height}
width={width}
columnCount={columnCount}
rowHeight={() => rowHeight}
rowHeight={getRowHeight}
useIsScrolling={useIsScrolling}
innerElementType={InnerElementType}
itemData={data}
Expand Down
1 change: 1 addition & 0 deletions src/components/Grid/Header.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ const RowColumnContainer = styled(HeaderCellContainer)<{
const RowColumn = styled(StyledCell)`
width: 100%;
text-align: right;
overflow: hidden;
`;

const Column = ({
Expand Down
14 changes: 12 additions & 2 deletions src/components/Grid/RowNumberColumn.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ const RowNumberColumnContainer = styled.div<{
$height: number;
$width: number;
$scrolledHorizontal: boolean;
$rowAutoHeight?: boolean;
}>`
position: sticky;
left: 0;
Expand All @@ -23,16 +24,17 @@ const RowNumberColumnContainer = styled.div<{
const RowNumberCell = styled.div<{
$height: number;
$rowNumber: number;
$rowAutoHeight?: boolean;
}>`
position: absolute;
left: 0;
overflow: hidden;
text-overflow: ellipsis;
white-space: nowrap;
width: 100%;
${({ $height, $rowNumber }) => `
${({ $height, $rowNumber, $rowAutoHeight }) => `
top: ${$height * $rowNumber}px;
height: ${$height}px;
height: ${$rowAutoHeight ? "100%" : `${$height}px`};
`}
`;
interface RowNumberColumnProps {
Expand All @@ -47,6 +49,7 @@ interface RowNumberColumnProps {
scrolledHorizontal: boolean;
rowStart: number;
showBorder: boolean;
rowAutoHeight?: boolean;
}
interface RowNumberProps
extends Pick<
Expand All @@ -56,6 +59,7 @@ interface RowNumberProps
rowIndex: number;
isLastRow: boolean;
isFirstRow: boolean;
rowAutoHeight?: boolean;
}
const RowNumber = ({
rowIndex,
Expand All @@ -65,6 +69,7 @@ const RowNumber = ({
isFirstRow,
showBorder,
rowStart,
rowAutoHeight,
}: RowNumberProps) => {
const currentRowIndex = rowIndex + rowStart;
const selectionType = getSelectionType({
Expand All @@ -84,6 +89,7 @@ const RowNumber = ({
<RowNumberCell
$rowNumber={rowIndex}
$height={rowHeight}
$rowAutoHeight={rowAutoHeight}
>
<StyledCell
$height={rowHeight}
Expand All @@ -96,6 +102,7 @@ const RowNumber = ({
$isLastRow={isLastRow}
$isSelectedLeft={isSelected}
$isSelectedTop={isSelectedTop}
$rowAutoHeight={rowAutoHeight}
data-selected={isSelected}
data-grid-row={currentRowIndex}
data-grid-column={-1}
Expand All @@ -121,12 +128,14 @@ const RowNumberColumn = ({
scrolledHorizontal,
rowStart = 0,
showBorder,
rowAutoHeight,
}: RowNumberColumnProps) => {
return (
<RowNumberColumnContainer
$height={headerHeight}
$width={rowWidth}
$scrolledHorizontal={scrolledHorizontal}
$rowAutoHeight={rowAutoHeight}
>
{Array.from({ length: maxRow - minRow + 1 }, (_, index) => minRow + index).map(
rowIndex => (
Expand All @@ -139,6 +148,7 @@ const RowNumberColumn = ({
isFirstRow={!showHeader && rowIndex === 0}
showBorder={showBorder}
rowStart={rowStart}
rowAutoHeight={rowAutoHeight}
/>
)
)}
Expand Down
12 changes: 10 additions & 2 deletions src/components/Grid/StyledCell.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ export const StyledCell = styled.div<{
$height: number;
$type?: "body" | "header";
$showBorder: boolean;
$rowAutoHeight?: boolean;
}>`
display: block;
text-align: left;
Expand All @@ -34,8 +35,11 @@ export const StyledCell = styled.div<{
$height,
$type = "body",
$showBorder,
$rowAutoHeight,
}) => `
height: ${$height}px;
height: ${$rowAutoHeight ? "100%" : `${$height}px`};
min-height: ${$rowAutoHeight ? "auto" : ""};
overflow-y: ${$rowAutoHeight ? "auto" : ""};
background: ${theme.click.grid[$type].cell.color.background[$selectionType]};
color: ${
$type === "header"
Expand Down Expand Up @@ -86,6 +90,7 @@ export const StyledCell = styled.div<{
`
: "border-right: none;"
}
${$rowAutoHeight && "border: none;"}
`}
${({
theme,
Expand All @@ -95,10 +100,12 @@ export const StyledCell = styled.div<{
$type = "body",
$isSelectedTop,
$isSelectedLeft,
$rowAutoHeight,
}) =>
$isSelectedTop ||
$isSelectedLeft ||
($selectionType === "selectDirect" && ($isLastRow || $isLastColumn))
($selectionType === "selectDirect" && ($isLastRow || $isLastColumn)) ||
$rowAutoHeight
? `
&::before {
content: "";
Expand Down Expand Up @@ -127,6 +134,7 @@ export const StyledCell = styled.div<{
? `border-right: 1px solid ${theme.click.grid[$type].cell.color.stroke.selectDirect};`
: ""
}
${$rowAutoHeight && "border: none;"}
}
`
: ""};
Expand Down
Loading