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.

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

fix: Better support for min-width on Table columns #1019

Merged
merged 2 commits into from
May 10, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
46 changes: 43 additions & 3 deletions src/components/Table/GridTable.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
condensedStyle,
dateColumn,
defaultStyle,
dragHandleColumn,
emptyCell,
GridCellAlignment,
GridColumn,
Expand All @@ -21,15 +22,14 @@ import {
GridTable,
Icon,
IconButton,
insertAtIndex,
numericColumn,
recursivelyGetContainingRow,
RowStyles,
selectColumn,
simpleHeader,
SimpleHeaderAndData,
useGridTableApi,
insertAtIndex,
dragHandleColumn,
recursivelyGetContainingRow,
} from "src/components/index";
import { Css, Palette } from "src/Css";
import { useComputed } from "src/hooks";
Expand Down Expand Up @@ -2065,3 +2065,43 @@ export function DraggableCardRows() {
/>
);
}

export function MinColumnWidths() {
const nameColumn: GridColumn<Row> = {
header: "Name",
data: ({ name }) => ({
content: name === "group" ? "Col-spans across all!" : `Width: 2fr; Min-width: 300px`,
colspan: name === "group" ? 3 : 1,
}),
w: 2,
mw: "300px",
};
const valueColumn: GridColumn<Row> = {
id: "value",
header: "Value",
data: ({ value }) => `Width: 1fr; Min-width: 200px`,
w: 1,
mw: "200px",
};
const actionColumn: GridColumn<Row> = {
header: "Action",
data: () => `Width: 1fr; Min-width: 150px`,
w: 1,
mw: "150px",
};
return (
<div css={Css.mx2.$}>
<GridTable
columns={[nameColumn, valueColumn, actionColumn]}
style={{ bordered: true, allWhite: true }}
rows={[
simpleHeader,
{ kind: "data", id: "group", data: { name: "group", value: 0 } },
{ kind: "data", id: "1", data: { name: "c", value: 1 } },
{ kind: "data", id: "2", data: { name: "B", value: 2 } },
{ kind: "data", id: "3", data: { name: "a", value: 3 } },
]}
/>
</div>
);
}
15 changes: 2 additions & 13 deletions src/components/Table/GridTable.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -953,7 +953,7 @@ describe("GridTable", () => {
});
});

it("throws error if column min-width definition is set with a non-px/percentage value", async () => {
it("throws error if column min-width definition is set with a non-px value", async () => {
// Given a column with an invalid `mw` value, then the render will fail
await expect(
render(
Expand All @@ -962,7 +962,7 @@ describe("GridTable", () => {
rows={[simpleHeader, { kind: "data", id: "1", data: { name: "a", value: 3 } }]}
/>,
),
).rejects.toThrow("Beam Table column min-width definition only supports px or percentage values");
).rejects.toThrow("Beam Table column minWidth definition only supports pixel units");
});

it("accepts pixel values for column min-width definition", async () => {
Expand All @@ -976,17 +976,6 @@ describe("GridTable", () => {
expect(r.gridTable).toBeTruthy();
});

it("accepts percentage values for column min-width definition", async () => {
// Given a column with an valid `mw` percentage value, then the render will succeed.
const r = await render(
<GridTable
columns={[{ header: () => "Name", data: "Test", mw: "100%" }]}
rows={[simpleHeader, { kind: "data", id: "1", data: { name: "a", value: 3 } }]}
/>,
);
expect(r.gridTable).toBeTruthy();
});

it("can handle onClick for rows", async () => {
const onClick = jest.fn();
const rowStyles: RowStyles<Row> = { header: {}, data: { onClick } };
Expand Down
10 changes: 1 addition & 9 deletions src/components/Table/components/Row.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { observer } from "mobx-react";
import { ReactElement, useContext, useRef, useCallback, RefObject } from "react";
import { ReactElement, useCallback, useContext, useRef } from "react";
import {
defaultRenderFn,
headerRenderFn,
Expand Down Expand Up @@ -176,13 +176,6 @@ function RowImpl<R extends Kinded, S>(props: RowProps<R>): ReactElement {
const applyFirstContentColumnStyles = !isHeader && isFirstContentColumn;
foundFirstContentColumn ||= applyFirstContentColumnStyles;

if (column.mw) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Makes a ton of sense to have this handled in the same place as the fr / size handling; great change!

// Validate the column's minWidth definition if set.
if (!column.mw.endsWith("px") && !column.mw.endsWith("%")) {
throw new Error("Beam Table column min-width definition only supports px or percentage values");
}
}

// When using the variation of the table with an EXPANDABLE_HEADER, then our HEADER and TOTAL rows have special border styling
// Keep track of the when we get to the last expanded column so we can apply this styling properly.
if (hasExpandableHeader && (isHeader || isTotals)) {
Expand Down Expand Up @@ -333,7 +326,6 @@ function RowImpl<R extends Kinded, S>(props: RowProps<R>): ReactElement {
width: `calc(${columnSizes.slice(columnIndex, columnIndex + currentColspan).join(" + ")}${
applyFirstContentColumnStyles && levelIndent ? ` - ${levelIndent}px` : ""
})`,
...(typeof column.mw === "string" ? Css.mw(column.mw).$ : {}),
};

const cellClassNames = revealOnRowHover ? revealOnRowHoverClass : undefined;
Expand Down
2 changes: 1 addition & 1 deletion src/components/Table/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ export type GridColumn<R extends Kinded> = {
* Example: Collapsed state shows number of books. Expanded state shows titles of books.
*/
expandedWidth?: number | string;
/** The minimum width the column can shrink to */
/** The minimum width the column can shrink to. This must be defined in pixels */
mw?: string;
/** The column's default alignment for each cell. */
align?: GridCellAlignment;
Expand Down
53 changes: 46 additions & 7 deletions src/components/Table/utils/columns.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { GridColumn, GridColumnWithId, Kinded, nonKindGridColumnKeys } from "src
import { DragData, emptyCell } from "src/components/Table/utils/utils";
import { isFunction, newMethodMissingProxy } from "src/utils";
import { Icon } from "src";
import { Css, Palette } from "src/Css";
import { Css } from "src/Css";

/** Provides default styling for a GridColumn representing a Date. */
export function column<T extends Kinded>(columnDef: GridColumn<T>): GridColumn<T> {
Expand Down Expand Up @@ -135,32 +135,71 @@ export function calcColumnSizes<R extends Kinded>(
{ claimedPercentages: 0, claimedPixels: 0, totalFr: 0 },
);

function getFrUnit(w: string | number | undefined): number | undefined {
bdow marked this conversation as resolved.
Show resolved Hide resolved
return typeof w === "number"
? w
: typeof w === "undefined"
? 1
: typeof w === "string" && w.endsWith("fr")
? Number(w.replace("fr", ""))
: undefined;
}

// In the event a column defines a fractional unit (fr) as the `w` value and a `mw` value in pixels,
// it is possible that the min-width value will kick in and throw off our claimedPixel and totalFr calculations.
// Once a `tableWidth` is defined, then we can adjust the claimedPixels and totalFr based on minWidth being present for any columns
let adjustedClaimedPixels = claimedPixels;
let adjustedTotalFr = totalFr;
if (tableWidth) {
columns.forEach(({ w, mw }) => {
const frUnit = getFrUnit(w);
if (mw === undefined || frUnit === undefined) return;

const mwPx = Number(mw.replace("px", ""));
const calcedWidth =
(tableWidth - (claimedPercentages / 100) * tableWidth - adjustedClaimedPixels) * (frUnit / adjustedTotalFr);
// If the calculated width is less than the minWidth, then adjust the claimedPixels and totalFr accordingly
bdow marked this conversation as resolved.
Show resolved Hide resolved
if (calcedWidth < mwPx) {
adjustedClaimedPixels += mwPx;
adjustedTotalFr -= frUnit;
}
});
}

// This is our "fake but for some reason it lines up better" fr calc
function fr(myFr: number): string {
function fr(myFr: number, mw: number): string {
// If the tableWidth, then return a pixel value
if (tableWidth) {
const widthBasis = Math.max(tableWidth, tableMinWidthPx);
return `(${(widthBasis - (claimedPercentages / 100) * widthBasis - claimedPixels) * (myFr / totalFr)}px)`;
const calcedWidth =
(widthBasis - (claimedPercentages / 100) * widthBasis - adjustedClaimedPixels) * (myFr / adjustedTotalFr);

return `${Math.max(calcedWidth, mw)}px`;
}
// Otherwise return the `calc()` value
return `((100% - ${claimedPercentages}% - ${claimedPixels}px) * (${myFr} / ${totalFr}))`;
}

const sizes = columns.map(({ id, expandedWidth, w: _w }) => {
const sizes = columns.map(({ id, expandedWidth, w: _w, mw: _mw }) => {
// Ensure `mw` is a pixel value if defined
if (_mw !== undefined && !_mw.endsWith("px")) {
throw new Error("Beam Table column minWidth definition only supports pixel units");
}
const mw = _mw ? Number(_mw.replace("px", "")) : 0;
const w = expandedColumnIds.includes(id) && expandedWidth !== undefined ? expandedWidth : _w;

if (typeof w === "undefined") {
return fr(1);
return fr(1, mw);
} else if (typeof w === "string") {
if (w.endsWith("%") || w.endsWith("px")) {
return w;
} else if (w.endsWith("fr")) {
return fr(Number(w.replace("fr", "")));
return fr(Number(w.replace("fr", "")), mw);
} else {
throw new Error("Beam Table column width definition only supports px, percentage, or fr units");
}
} else {
return fr(w);
return fr(w, mw);
}
});

Expand Down
Loading