From 144dbb0f99bb6da14a8b398bee5bdb39c2a5f868 Mon Sep 17 00:00:00 2001 From: Mike Perrotti Date: Mon, 6 Mar 2023 15:05:17 -0500 Subject: [PATCH 1/9] adds styles and feature stories --- src/DataTable/DataTable.features.stories.tsx | 451 +++++++++++++++++++ src/DataTable/DataTable.tsx | 9 +- src/DataTable/Table.tsx | 35 +- src/DataTable/column.ts | 29 ++ src/DataTable/useTable.ts | 48 ++ 5 files changed, 568 insertions(+), 4 deletions(-) diff --git a/src/DataTable/DataTable.features.stories.tsx b/src/DataTable/DataTable.features.stories.tsx index 1da0ced058f..9c400ed0e19 100644 --- a/src/DataTable/DataTable.features.stories.tsx +++ b/src/DataTable/DataTable.features.stories.tsx @@ -577,3 +577,454 @@ export const WithActionsOnly = () => ( ) + +export const MixedColumnWidths = () => ( + + + Repositories + + { + return + }, + width: 'shrink', + maxWidth: '100px', + }, + { + header: 'auto', + field: 'updatedAt', + renderCell: row => { + return + }, + width: 'auto', + }, + { + header: '200px', + field: 'securityFeatures.dependabot', + renderCell: row => { + return row.securityFeatures.dependabot.length > 0 ? ( + + {row.securityFeatures.dependabot.map(feature => { + return + })} + + ) : null + }, + width: '200px', + }, + { + header: 'undefined', + field: 'securityFeatures.codeScanning', + renderCell: row => { + return row.securityFeatures.codeScanning.length > 0 ? ( + + {row.securityFeatures.codeScanning.map(feature => { + return + })} + + ) : null + }, + }, + ]} + /> + +) + +export const AllColumnWidthsGrow = () => ( + + + Repositories + + { + return + }, + }, + { + header: 'Updated', + field: 'updatedAt', + renderCell: row => { + return + }, + }, + { + header: 'Dependabot', + field: 'securityFeatures.dependabot', + renderCell: row => { + return row.securityFeatures.dependabot.length > 0 ? ( + + {row.securityFeatures.dependabot.map(feature => { + return + })} + + ) : null + }, + }, + { + header: 'Code scanning', + field: 'securityFeatures.codeScanning', + renderCell: row => { + return row.securityFeatures.codeScanning.length > 0 ? ( + + {row.securityFeatures.codeScanning.map(feature => { + return + })} + + ) : null + }, + }, + ]} + /> + +) + +export const AllColumnWidthsShrink = () => ( + + + Repositories + + { + return + }, + width: 'shrink', + }, + { + header: 'Updated', + field: 'updatedAt', + renderCell: row => { + return + }, + width: 'shrink', + }, + { + header: 'Dependabot', + field: 'securityFeatures.dependabot', + renderCell: row => { + return row.securityFeatures.dependabot.length > 0 ? ( + + {row.securityFeatures.dependabot.map(feature => { + return + })} + + ) : null + }, + width: 'shrink', + }, + { + header: 'Code scanning', + field: 'securityFeatures.codeScanning', + renderCell: row => { + return row.securityFeatures.codeScanning.length > 0 ? ( + + {row.securityFeatures.codeScanning.map(feature => { + return + })} + + ) : null + }, + width: 'shrink', + }, + ]} + /> + +) + +export const AllColumnWidthsAuto = () => ( + + + Repositories + + { + return + }, + width: 'auto', + }, + { + header: 'Updated', + field: 'updatedAt', + renderCell: row => { + return + }, + width: 'auto', + }, + { + header: 'Dependabot', + field: 'securityFeatures.dependabot', + renderCell: row => { + return row.securityFeatures.dependabot.length > 0 ? ( + + {row.securityFeatures.dependabot.map(feature => { + return + })} + + ) : null + }, + width: 'auto', + }, + { + header: 'Code scanning', + field: 'securityFeatures.codeScanning', + renderCell: row => { + return row.securityFeatures.codeScanning.length > 0 ? ( + + {row.securityFeatures.codeScanning.map(feature => { + return + })} + + ) : null + }, + width: 'auto', + }, + ]} + /> + +) + +export const AllColumnWidthsExplicit = () => ( + + + Repositories + + { + return + }, + width: '200px', + }, + { + header: 'Updated', + field: 'updatedAt', + renderCell: row => { + return + }, + width: '200px', + }, + { + header: 'Dependabot', + field: 'securityFeatures.dependabot', + renderCell: row => { + return row.securityFeatures.dependabot.length > 0 ? ( + + {row.securityFeatures.dependabot.map(feature => { + return + })} + + ) : null + }, + width: '200px', + }, + { + header: 'Code scanning', + field: 'securityFeatures.codeScanning', + renderCell: row => { + return row.securityFeatures.codeScanning.length > 0 ? ( + + {row.securityFeatures.codeScanning.map(feature => { + return + })} + + ) : null + }, + width: '200px', + }, + ]} + /> + +) + +export const AllColumnWidthsGrowWithMaxWidth = () => ( + + + Repositories + + { + return + }, + maxWidth: '200px', + }, + { + header: 'Updated', + field: 'updatedAt', + renderCell: row => { + return + }, + maxWidth: '200px', + }, + { + header: 'Dependabot', + field: 'securityFeatures.dependabot', + renderCell: row => { + return row.securityFeatures.dependabot.length > 0 ? ( + + {row.securityFeatures.dependabot.map(feature => { + return + })} + + ) : null + }, + maxWidth: '200px', + }, + { + header: 'Code scanning', + field: 'securityFeatures.codeScanning', + renderCell: row => { + return row.securityFeatures.codeScanning.length > 0 ? ( + + {row.securityFeatures.codeScanning.map(feature => { + return + })} + + ) : null + }, + maxWidth: '200px', + }, + ]} + /> + +) + +export const AllColumnWidthsGrowWithMinWidth = () => ( + + + Repositories + + { + return + }, + minWidth: '200px', + }, + { + header: 'Updated', + field: 'updatedAt', + renderCell: row => { + return + }, + minWidth: '200px', + }, + { + header: 'Dependabot', + field: 'securityFeatures.dependabot', + renderCell: row => { + return row.securityFeatures.dependabot.length > 0 ? ( + + {row.securityFeatures.dependabot.map(feature => { + return + })} + + ) : null + }, + minWidth: '200px', + }, + { + header: 'Code scanning', + field: 'securityFeatures.codeScanning', + renderCell: row => { + return row.securityFeatures.codeScanning.length > 0 ? ( + + {row.securityFeatures.codeScanning.map(feature => { + return + })} + + ) : null + }, + minWidth: '200px', + }, + ]} + /> + +) diff --git a/src/DataTable/DataTable.tsx b/src/DataTable/DataTable.tsx index 0ae159cc9a7..2efe86f50a8 100644 --- a/src/DataTable/DataTable.tsx +++ b/src/DataTable/DataTable.tsx @@ -61,14 +61,19 @@ function DataTable({ initialSortColumn, initialSortDirection, }: DataTableProps) { - const {headers, rows, actions} = useTable({ + const {headers, rows, actions, gridTemplateColumns} = useTable({ data, columns, initialSortColumn, initialSortDirection, }) return ( - +
{headers.map(header => { diff --git a/src/DataTable/Table.tsx b/src/DataTable/Table.tsx index 40330bfcf59..91cd8422eca 100644 --- a/src/DataTable/Table.tsx +++ b/src/DataTable/Table.tsx @@ -19,7 +19,9 @@ const StyledTable = styled.table>` background-color: ${get('colors.canvas.default')}; border-spacing: 0; border-collapse: separate; + display: grid; font-size: var(--table-font-size); + grid-template-columns: var(--grid-template-columns); line-height: calc(20 / var(--table-font-size)); width: 100%; overflow-x: auto; @@ -138,6 +140,19 @@ const StyledTable = styled.table>` font-weight: 600; text-align: start; } + + /* Grid layout */ + .TableHead, + .TableBody, + .TableRow { + display: contents; + } + + @supports (grid-template-columns: subgrid) { + display: grid; + grid-template-columns: subgrid; + grid-column: -1 /1; + } ` export type TableProps = React.ComponentPropsWithoutRef<'table'> & { @@ -156,10 +171,26 @@ export type TableProps = React.ComponentPropsWithoutRef<'table'> & { * a cell */ cellPadding?: 'condensed' | 'normal' | 'spacious' | undefined + + /** + * Column width definitions + */ + gridTemplateColumns?: React.CSSProperties['gridTemplateColumns'] | undefined } -const Table = React.forwardRef(function Table({cellPadding = 'normal', ...rest}, ref) { - return +const Table = React.forwardRef(function Table( + {cellPadding = 'normal', gridTemplateColumns, ...rest}, + ref, +) { + return ( + + ) }) // ---------------------------------------------------------------------------- diff --git a/src/DataTable/column.ts b/src/DataTable/column.ts index 1fe540b865d..0ab82cdf638 100644 --- a/src/DataTable/column.ts +++ b/src/DataTable/column.ts @@ -1,7 +1,10 @@ import {ObjectPaths} from './utils' import {UniqueRow} from './row' import {SortStrategies} from './sorting' +// TODO: uncomment ResponsiveValue when I'm ready to implement the responsive part +// import {ResponsiveValue} from '../hooks/useResponsiveValue' +export type ColumnWidth = 'grow' | 'shrink' | 'auto' | React.CSSProperties['width'] export interface Column { id?: string | undefined @@ -21,6 +24,18 @@ export interface Column { */ field: ObjectPaths + /** + * The minimum width the column can shrink to + */ + // TODO: uncomment ResponsiveValue when I'm ready to implement the responsive part + maxWidth?: React.CSSProperties['maxWidth'] /*| ResponsiveValue*/ | undefined + + /** + * The maximum width the column can grow to + */ + // TODO: uncomment ResponsiveValue when I'm ready to implement the responsive part + minWidth?: React.CSSProperties['minWidth'] /*| ResponsiveValue*/ | undefined + /** * Provide a custom component or render prop to render the data for this * column in a row @@ -38,6 +53,20 @@ export interface Column { * specific sort strategy or custom sort strategy */ sortBy?: boolean | SortStrategies + + /** + * Controls the width of the column. + * - 'grow': Stretch to fill available space, and min width is the width of the widest cell in the column + * - 'shrink': Stretch to fill available space or shrink to fit in the available space. Allows the column to shrink smaller than the cell content's width. + * - 'auto': The column is the width of it’s widest cell. Not intended for use with columns who’s content length varies a lot because a layout shift will occur when the content changes + * - explicit width: Will be exactly that width and will not grow or shrink to fill the parent + * @default 'grow' + */ + width?: + | ColumnWidth + // TODO: uncomment ResponsiveValue when I'm ready to implement the responsive part + // | ResponsiveValue + | undefined } export function createColumnHelper() { diff --git a/src/DataTable/useTable.ts b/src/DataTable/useTable.ts index 295c5b15f11..3149f03e464 100644 --- a/src/DataTable/useTable.ts +++ b/src/DataTable/useTable.ts @@ -17,6 +17,7 @@ interface Table { actions: { sortBy: (header: Header) => void } + gridTemplateColumns: React.CSSProperties['gridTemplateColumns'] } interface Header { @@ -179,6 +180,52 @@ export function useTable({ }) } + const gridColumnWidths = columns.map(column => { + const columnWidth = column.width ?? 'grow' + let minWidth = 'auto' + let maxWidth = '1fr' + + if (columnWidth === 'auto') { + maxWidth = 'auto' + + // If the column is sized to auto and there's no min-width, we don't need to use `minmax()`, + // so we can just return 'auto'. + if (!column.minWidth) { + return 'auto' + } + } + + // Setting a min-width of 'max-content' ensures that the column will grow to fit the widest cell's content. + // However, If the column has a max width, we can't set the min width to `max-content` because + // the widest cell's content might overflow the container. + if (columnWidth === 'grow' && !column.maxWidth) { + minWidth = 'max-content' + } + + // Column widths set to "shrink" don't need a min width unless one is explicitly provided. + if (columnWidth === 'shrink') { + minWidth = '0' + } + + // If a consumer passes `minWidth` or `maxWidth`, we need to override whatever we set above. + if (column.minWidth) { + minWidth = typeof column.minWidth === 'number' ? `${column.minWidth}px` : column.minWidth + } + + if (column.maxWidth) { + maxWidth = typeof column.maxWidth === 'number' ? `${column.maxWidth}px` : column.maxWidth + } + + // If a consumer is passing one of the shorthand widths or doesn't pass a width at all, we use the + // min and max width calculated above to create a minmax() column template value. + if (typeof columnWidth !== 'number' && ['grow', 'shrink', 'auto'].includes(columnWidth)) { + return `minmax(${minWidth}, ${maxWidth})` + } + + // If we reach this point, the consumer is passing an explicit width value. + return typeof columnWidth === 'number' ? `${columnWidth}px` : columnWidth + }) + return { headers, rows: rowOrder.map(row => { @@ -204,6 +251,7 @@ export function useTable({ actions: { sortBy, }, + gridTemplateColumns: gridColumnWidths.join(' '), } } From 2c21c81cedf8a58fc7b409485db67fc1ebbdf502 Mon Sep 17 00:00:00 2001 From: Mike Perrotti Date: Mon, 6 Mar 2023 15:13:11 -0500 Subject: [PATCH 2/9] explicitly adds roles to table elements --- src/DataTable/Table.tsx | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/src/DataTable/Table.tsx b/src/DataTable/Table.tsx index 91cd8422eca..54530d88763 100644 --- a/src/DataTable/Table.tsx +++ b/src/DataTable/Table.tsx @@ -188,6 +188,7 @@ const Table = React.forwardRef(function Table( data-cell-padding={cellPadding} style={{'--grid-template-columns': gridTemplateColumns}} className="Table" + role="table" ref={ref} /> ) @@ -200,7 +201,14 @@ const Table = React.forwardRef(function Table( export type TableHeadProps = React.ComponentPropsWithoutRef<'thead'> function TableHead({children}: TableHeadProps) { - return {children} + return ( + // We need to explicitly pass this role because some ATs and browsers drop table semantics + // when we use `display: contents` or `display: grid` in the table + // eslint-disable-next-line jsx-a11y/no-redundant-roles + + {children} + + ) } // ---------------------------------------------------------------------------- @@ -210,7 +218,14 @@ function TableHead({children}: TableHeadProps) { export type TableBodyProps = React.ComponentPropsWithoutRef<'tbody'> function TableBody({children}: TableBodyProps) { - return {children} + return ( + // We need to explicitly pass this role because some ATs and browsers drop table semantics + // when we use `display: contents` or `display: grid` in the table + // eslint-disable-next-line jsx-a11y/no-redundant-roles + + {children} + + ) } // ---------------------------------------------------------------------------- From 0835bdfd06adeda2660d4ba0774ad5504cab6dc7 Mon Sep 17 00:00:00 2001 From: Mike Perrotti Date: Mon, 6 Mar 2023 17:26:07 -0500 Subject: [PATCH 3/9] adds column widths to Playground story --- src/DataTable/DataTable.stories.tsx | 31 +++++++++++++++-- src/DataTable/storyHelpers.ts | 52 +++++++++++++++++++++++++++++ 2 files changed, 80 insertions(+), 3 deletions(-) create mode 100644 src/DataTable/storyHelpers.ts diff --git a/src/DataTable/DataTable.stories.tsx b/src/DataTable/DataTable.stories.tsx index 36f8a5dda8f..5459f55d1bc 100644 --- a/src/DataTable/DataTable.stories.tsx +++ b/src/DataTable/DataTable.stories.tsx @@ -1,13 +1,16 @@ -import {Meta, ComponentStory} from '@storybook/react' +import {Meta} from '@storybook/react' import React from 'react' -import {DataTable, Table} from '../DataTable' +import {DataTable, DataTableProps, Table} from '../DataTable' import Label from '../Label' import LabelGroup from '../LabelGroup' import RelativeTime from '../RelativeTime' +import {UniqueRow} from './row' +import {getColumnWidthArgTypes, ColWidthArgTypes} from './storyHelpers' export default { title: 'Drafts/Components/DataTable', component: DataTable, + argTypes: getColumnWidthArgTypes(5), } as Meta const now = Date.now() @@ -179,7 +182,14 @@ export const Default = () => ( ) -export const Playground: ComponentStory = args => { +export const Playground = (args: DataTableProps & ColWidthArgTypes) => { + const getColWidth = (colIndex: number) => { + return args[`colWidth${colIndex}`] !== 'explicit width' + ? args[`colWidth${colIndex}`] + : args[`explicitColWidth${colIndex}`] + ? args[`explicitColWidth${colIndex}`] + : 'grow' + } return ( @@ -198,6 +208,9 @@ export const Playground: ComponentStory = args => { header: 'Repository', field: 'name', rowHeader: true, + width: getColWidth(0), + minWidth: args.minColWidth0, + maxWidth: args.maxColWidth0, }, { header: 'Type', @@ -205,6 +218,9 @@ export const Playground: ComponentStory = args => { renderCell: row => { return }, + width: getColWidth(1), + minWidth: args.minColWidth1, + maxWidth: args.maxColWidth1, }, { header: 'Updated', @@ -212,6 +228,9 @@ export const Playground: ComponentStory = args => { renderCell: row => { return }, + width: getColWidth(2), + minWidth: args.minColWidth2, + maxWidth: args.maxColWidth2, }, { header: 'Dependabot', @@ -225,6 +244,9 @@ export const Playground: ComponentStory = args => { ) : null }, + width: getColWidth(3), + minWidth: args.minColWidth3, + maxWidth: args.maxColWidth3, }, { header: 'Code scanning', @@ -238,6 +260,9 @@ export const Playground: ComponentStory = args => { ) : null }, + width: getColWidth(4), + minWidth: args.minColWidth4, + maxWidth: args.maxColWidth4, }, ]} /> diff --git a/src/DataTable/storyHelpers.ts b/src/DataTable/storyHelpers.ts new file mode 100644 index 00000000000..0781b255b0e --- /dev/null +++ b/src/DataTable/storyHelpers.ts @@ -0,0 +1,52 @@ +import {InputType} from '@storybook/csf' + +// Keeping this generic because we can't know how many columns there will be, +// so we can't know all the possible width keys (for example. 'colWidth1') +export type ColWidthArgTypes = Record + +export const getColumnWidthArgTypes = (colCount: number) => { + const argTypes: InputType = {} + for (let i = 0; i <= colCount - 1; i++) { + argTypes[`colWidth${i}`] = { + name: `column ${i + 1} width`, + control: { + type: 'radio', + }, + defaultValue: 'grow', + options: ['grow', 'shrink', 'auto', 'explicit width'], + table: { + category: 'Column widths', + }, + } + argTypes[`explicitColWidth${i}`] = { + name: `column ${i + 1} explicit width`, + control: { + type: 'text', + }, + defaultValue: '200px', + if: {arg: `colWidth${i}`, eq: 'explicit width'}, + table: { + category: 'Column widths', + }, + } + argTypes[`minColWidth${i}`] = { + name: `column ${i + 1} min width`, + control: { + type: 'text', + }, + table: { + category: 'Column widths', + }, + } + argTypes[`maxColWidth${i}`] = { + name: `column ${i + 1} max width`, + control: { + type: 'text', + }, + table: { + category: 'Column widths', + }, + } + } + return argTypes +} From 7638f77c0301c6dad0eb48492de196a2da1a1bb0 Mon Sep 17 00:00:00 2001 From: Mike Perrotti Date: Tue, 7 Mar 2023 17:55:31 -0500 Subject: [PATCH 4/9] adds tests fixes max width with auto --- src/DataTable/__tests__/DataTable.test.tsx | 173 ++++++++++++++++++++- src/DataTable/column.ts | 12 +- src/DataTable/useTable.ts | 90 +++++------ 3 files changed, 220 insertions(+), 55 deletions(-) diff --git a/src/DataTable/__tests__/DataTable.test.tsx b/src/DataTable/__tests__/DataTable.test.tsx index cc26d83fe0d..427e9a3fbb8 100644 --- a/src/DataTable/__tests__/DataTable.test.tsx +++ b/src/DataTable/__tests__/DataTable.test.tsx @@ -2,7 +2,8 @@ import userEvent from '@testing-library/user-event' import {render, screen, getByRole, queryByRole, queryAllByRole} from '@testing-library/react' import React from 'react' import {DataTable, Table} from '../../DataTable' -import {createColumnHelper} from '../column' +import {Column, createColumnHelper} from '../column' +import {getGridTemplateFromColumns} from '../useTable' describe('DataTable', () => { it('should render a semantic
through `data` and `columns`', () => { @@ -621,4 +622,174 @@ describe('DataTable', () => { ]) }) }) + + describe('column widths', () => { + it('correctly sets the column width to "grow" when width is undefined', () => { + const columnHelper = createColumnHelper<{id: number; name: string}>() + const columns = [ + columnHelper.column({ + header: 'Name', + field: 'name', + }), + ] + + expect(getGridTemplateFromColumns(columns)).toEqual(['minmax(max-content, 1fr)']) + }) + it('correctly sets the column width when width === "grow"', () => { + const columnHelper = createColumnHelper<{id: number; name: string}>() + const columns = [ + columnHelper.column({ + header: 'Name', + field: 'name', + width: 'grow', + }), + ] + + expect(getGridTemplateFromColumns(columns)).toEqual(['minmax(max-content, 1fr)']) + }) + it('correctly sets the column width when width === "shrink"', () => { + const columnHelper = createColumnHelper<{id: number; name: string}>() + const columns = [ + columnHelper.column({ + header: 'Name', + field: 'name', + width: 'shrink', + }), + ] + + expect(getGridTemplateFromColumns(columns)).toEqual(['minmax(0, 1fr)']) + }) + it('correctly sets the column width when width === "auto"', () => { + const columnHelper = createColumnHelper<{id: number; name: string}>() + const columns = [ + columnHelper.column({ + header: 'Name', + field: 'name', + width: 'auto', + }), + ] + + expect(getGridTemplateFromColumns(columns)).toEqual(['auto']) + }) + it('correctly sets the column width when width is a CSS width string', () => { + const columnHelper = createColumnHelper<{id: number; name: string}>() + const columns = [ + columnHelper.column({ + header: 'Name', + field: 'name', + width: '42ch', + }), + ] + + expect(getGridTemplateFromColumns(columns)).toEqual(['42ch']) + }) + it('correctly sets the column width when width is a number', () => { + const columnHelper = createColumnHelper<{id: number; name: string}>() + const columns = [ + columnHelper.column({ + header: 'Name', + field: 'name', + width: 200, + }), + ] + + expect(getGridTemplateFromColumns(columns)).toEqual(['200px']) + }) + it('correctly sets min-widths for the column', () => { + const columnHelper = createColumnHelper<{id: number; name: string}>() + const columns: Record[]> = { + grow: [ + columnHelper.column({ + header: 'Name', + field: 'name', + width: 'grow', + minWidth: '42ch', + }), + ], + shrink: [ + columnHelper.column({ + header: 'Name', + field: 'name', + width: 'shrink', + minWidth: '42ch', + }), + ], + auto: [ + columnHelper.column({ + header: 'Name', + field: 'name', + width: 'auto', + minWidth: '42ch', + }), + ], + } + const expectedWidths: Record = { + grow: 'minmax(42ch, 1fr)', + shrink: 'minmax(42ch, 1fr)', + auto: 'minmax(42ch, auto)', + } + + for (const widthOpt in columns) { + expect(getGridTemplateFromColumns(columns[widthOpt])).toEqual([expectedWidths[widthOpt]]) + } + }) + it('correctly sets max-widths for the column', () => { + const columnHelper = createColumnHelper<{id: number; name: string}>() + const columns: Record[]> = { + grow: [ + columnHelper.column({ + header: 'Name', + field: 'name', + width: 'grow', + maxWidth: '42ch', + }), + ], + shrink: [ + columnHelper.column({ + header: 'Name', + field: 'name', + width: 'shrink', + maxWidth: '42ch', + }), + ], + auto: [ + columnHelper.column({ + header: 'Name', + field: 'name', + width: 'auto', + maxWidth: '42ch', + }), + ], + } + const expectedWidths: Record = { + grow: 'minmax(auto, 42ch)', + shrink: 'minmax(0, 42ch)', + auto: 'minmax(auto, 42ch)', + } + + for (const widthOpt in columns) { + expect(getGridTemplateFromColumns(columns[widthOpt])).toEqual([expectedWidths[widthOpt]]) + } + }) + it('sets a custom property style to define the column grid template', () => { + const columnHelper = createColumnHelper<{id: number; name: string}>() + const columns = [ + columnHelper.column({ + header: 'Name', + field: 'name', + }), + ] + const data = [ + { + id: 1, + name: 'one', + }, + ] + render() + + expect(screen.getByRole('table')).toHaveStyle({ + '--grid-template-columns': 'minmax(max-content, 1fr)', + }) + }) + }) }) diff --git a/src/DataTable/column.ts b/src/DataTable/column.ts index 4779b89f161..d267a10b473 100644 --- a/src/DataTable/column.ts +++ b/src/DataTable/column.ts @@ -28,13 +28,13 @@ export interface Column { * The minimum width the column can shrink to */ // TODO: uncomment ResponsiveValue when I'm ready to implement the responsive part - maxWidth?: React.CSSProperties['maxWidth'] /*| ResponsiveValue*/ | undefined + maxWidth?: React.CSSProperties['maxWidth'] /*| ResponsiveValue*/ /** * The maximum width the column can grow to */ // TODO: uncomment ResponsiveValue when I'm ready to implement the responsive part - minWidth?: React.CSSProperties['minWidth'] /*| ResponsiveValue*/ | undefined + minWidth?: React.CSSProperties['minWidth'] /*| ResponsiveValue*/ /** * Provide a custom component or render prop to render the data for this @@ -62,11 +62,9 @@ export interface Column { * - explicit width: Will be exactly that width and will not grow or shrink to fill the parent * @default 'grow' */ - width?: - | ColumnWidth - // TODO: uncomment ResponsiveValue when I'm ready to implement the responsive part - // | ResponsiveValue - | undefined + width?: ColumnWidth + // TODO: uncomment ResponsiveValue when I'm ready to implement the responsive part + // | ResponsiveValue } export function createColumnHelper() { diff --git a/src/DataTable/useTable.ts b/src/DataTable/useTable.ts index 3a5e902efa5..1e88168bad4 100644 --- a/src/DataTable/useTable.ts +++ b/src/DataTable/useTable.ts @@ -42,6 +42,48 @@ interface Cell { type ColumnSortState = {id: string; direction: Exclude} | null +export function getGridTemplateFromColumns(columns: Array>): string[] { + return columns.map(column => { + const columnWidth = column.width ?? 'grow' + let minWidth = 'auto' + let maxWidth = '1fr' + + if (columnWidth === 'auto') { + maxWidth = 'auto' + } + + // Setting a min-width of 'max-content' ensures that the column will grow to fit the widest cell's content. + // However, If the column has a max width, we can't set the min width to `max-content` because + // the widest cell's content might overflow the container. + if (columnWidth === 'grow' && !column.maxWidth) { + minWidth = 'max-content' + } + + // Column widths set to "shrink" don't need a min width unless one is explicitly provided. + if (columnWidth === 'shrink') { + minWidth = '0' + } + + // If a consumer passes `minWidth` or `maxWidth`, we need to override whatever we set above. + if (column.minWidth) { + minWidth = typeof column.minWidth === 'number' ? `${column.minWidth}px` : column.minWidth + } + + if (column.maxWidth) { + maxWidth = typeof column.maxWidth === 'number' ? `${column.maxWidth}px` : column.maxWidth + } + + // If a consumer is passing one of the shorthand widths or doesn't pass a width at all, we use the + // min and max width calculated above to create a minmax() column template value. + if (typeof columnWidth !== 'number' && ['grow', 'shrink', 'auto'].includes(columnWidth)) { + return minWidth === maxWidth ? minWidth : `minmax(${minWidth}, ${maxWidth})` + } + + // If we reach this point, the consumer is passing an explicit width value. + return typeof columnWidth === 'number' ? `${columnWidth}px` : columnWidth + }) +} + export function useTable({ columns, data, @@ -180,52 +222,6 @@ export function useTable({ }) } - const gridColumnWidths = columns.map(column => { - const columnWidth = column.width ?? 'grow' - let minWidth = 'auto' - let maxWidth = '1fr' - - if (columnWidth === 'auto') { - maxWidth = 'auto' - - // If the column is sized to auto and there's no min-width, we don't need to use `minmax()`, - // so we can just return 'auto'. - if (!column.minWidth) { - return 'auto' - } - } - - // Setting a min-width of 'max-content' ensures that the column will grow to fit the widest cell's content. - // However, If the column has a max width, we can't set the min width to `max-content` because - // the widest cell's content might overflow the container. - if (columnWidth === 'grow' && !column.maxWidth) { - minWidth = 'max-content' - } - - // Column widths set to "shrink" don't need a min width unless one is explicitly provided. - if (columnWidth === 'shrink') { - minWidth = '0' - } - - // If a consumer passes `minWidth` or `maxWidth`, we need to override whatever we set above. - if (column.minWidth) { - minWidth = typeof column.minWidth === 'number' ? `${column.minWidth}px` : column.minWidth - } - - if (column.maxWidth) { - maxWidth = typeof column.maxWidth === 'number' ? `${column.maxWidth}px` : column.maxWidth - } - - // If a consumer is passing one of the shorthand widths or doesn't pass a width at all, we use the - // min and max width calculated above to create a minmax() column template value. - if (typeof columnWidth !== 'number' && ['grow', 'shrink', 'auto'].includes(columnWidth)) { - return `minmax(${minWidth}, ${maxWidth})` - } - - // If we reach this point, the consumer is passing an explicit width value. - return typeof columnWidth === 'number' ? `${columnWidth}px` : columnWidth - }) - return { headers, rows: rowOrder.map(row => { @@ -251,7 +247,7 @@ export function useTable({ actions: { sortBy, }, - gridTemplateColumns: gridColumnWidths.join(' '), + gridTemplateColumns: getGridTemplateFromColumns(columns).join(' '), } } From 700d7864bef0ada393a773c87296a00226f09e03 Mon Sep 17 00:00:00 2001 From: Mike Perrotti Date: Tue, 7 Mar 2023 18:32:55 -0500 Subject: [PATCH 5/9] cleans up feature stories --- src/DataTable/DataTable.features.stories.tsx | 395 +------------------ 1 file changed, 5 insertions(+), 390 deletions(-) diff --git a/src/DataTable/DataTable.features.stories.tsx b/src/DataTable/DataTable.features.stories.tsx index 9c400ed0e19..c55deeaf151 100644 --- a/src/DataTable/DataTable.features.stories.tsx +++ b/src/DataTable/DataTable.features.stories.tsx @@ -589,20 +589,20 @@ export const MixedColumnWidths = () => ( data={data} columns={[ { - header: 'grow w/ 200px min', + header: 'grow w/ 200px max', field: 'name', rowHeader: true, width: 'grow', - minWidth: '200px', + maxWidth: '200px', }, { - header: 'shrink w/ 100px max', + header: 'shrink w/ 100px min', field: 'type', renderCell: row => { return }, width: 'shrink', - maxWidth: '100px', + minWidth: '100px', }, { header: 'auto', @@ -627,391 +627,7 @@ export const MixedColumnWidths = () => ( width: '200px', }, { - header: 'undefined', - field: 'securityFeatures.codeScanning', - renderCell: row => { - return row.securityFeatures.codeScanning.length > 0 ? ( - - {row.securityFeatures.codeScanning.map(feature => { - return - })} - - ) : null - }, - }, - ]} - /> - -) - -export const AllColumnWidthsGrow = () => ( - - - Repositories - - { - return - }, - }, - { - header: 'Updated', - field: 'updatedAt', - renderCell: row => { - return - }, - }, - { - header: 'Dependabot', - field: 'securityFeatures.dependabot', - renderCell: row => { - return row.securityFeatures.dependabot.length > 0 ? ( - - {row.securityFeatures.dependabot.map(feature => { - return - })} - - ) : null - }, - }, - { - header: 'Code scanning', - field: 'securityFeatures.codeScanning', - renderCell: row => { - return row.securityFeatures.codeScanning.length > 0 ? ( - - {row.securityFeatures.codeScanning.map(feature => { - return - })} - - ) : null - }, - }, - ]} - /> - -) - -export const AllColumnWidthsShrink = () => ( - - - Repositories - - { - return - }, - width: 'shrink', - }, - { - header: 'Updated', - field: 'updatedAt', - renderCell: row => { - return - }, - width: 'shrink', - }, - { - header: 'Dependabot', - field: 'securityFeatures.dependabot', - renderCell: row => { - return row.securityFeatures.dependabot.length > 0 ? ( - - {row.securityFeatures.dependabot.map(feature => { - return - })} - - ) : null - }, - width: 'shrink', - }, - { - header: 'Code scanning', - field: 'securityFeatures.codeScanning', - renderCell: row => { - return row.securityFeatures.codeScanning.length > 0 ? ( - - {row.securityFeatures.codeScanning.map(feature => { - return - })} - - ) : null - }, - width: 'shrink', - }, - ]} - /> - -) - -export const AllColumnWidthsAuto = () => ( - - - Repositories - - { - return - }, - width: 'auto', - }, - { - header: 'Updated', - field: 'updatedAt', - renderCell: row => { - return - }, - width: 'auto', - }, - { - header: 'Dependabot', - field: 'securityFeatures.dependabot', - renderCell: row => { - return row.securityFeatures.dependabot.length > 0 ? ( - - {row.securityFeatures.dependabot.map(feature => { - return - })} - - ) : null - }, - width: 'auto', - }, - { - header: 'Code scanning', - field: 'securityFeatures.codeScanning', - renderCell: row => { - return row.securityFeatures.codeScanning.length > 0 ? ( - - {row.securityFeatures.codeScanning.map(feature => { - return - })} - - ) : null - }, - width: 'auto', - }, - ]} - /> - -) - -export const AllColumnWidthsExplicit = () => ( - - - Repositories - - { - return - }, - width: '200px', - }, - { - header: 'Updated', - field: 'updatedAt', - renderCell: row => { - return - }, - width: '200px', - }, - { - header: 'Dependabot', - field: 'securityFeatures.dependabot', - renderCell: row => { - return row.securityFeatures.dependabot.length > 0 ? ( - - {row.securityFeatures.dependabot.map(feature => { - return - })} - - ) : null - }, - width: '200px', - }, - { - header: 'Code scanning', - field: 'securityFeatures.codeScanning', - renderCell: row => { - return row.securityFeatures.codeScanning.length > 0 ? ( - - {row.securityFeatures.codeScanning.map(feature => { - return - })} - - ) : null - }, - width: '200px', - }, - ]} - /> - -) - -export const AllColumnWidthsGrowWithMaxWidth = () => ( - - - Repositories - - { - return - }, - maxWidth: '200px', - }, - { - header: 'Updated', - field: 'updatedAt', - renderCell: row => { - return - }, - maxWidth: '200px', - }, - { - header: 'Dependabot', - field: 'securityFeatures.dependabot', - renderCell: row => { - return row.securityFeatures.dependabot.length > 0 ? ( - - {row.securityFeatures.dependabot.map(feature => { - return - })} - - ) : null - }, - maxWidth: '200px', - }, - { - header: 'Code scanning', - field: 'securityFeatures.codeScanning', - renderCell: row => { - return row.securityFeatures.codeScanning.length > 0 ? ( - - {row.securityFeatures.codeScanning.map(feature => { - return - })} - - ) : null - }, - maxWidth: '200px', - }, - ]} - /> - -) - -export const AllColumnWidthsGrowWithMinWidth = () => ( - - - Repositories - - { - return - }, - minWidth: '200px', - }, - { - header: 'Updated', - field: 'updatedAt', - renderCell: row => { - return - }, - minWidth: '200px', - }, - { - header: 'Dependabot', - field: 'securityFeatures.dependabot', - renderCell: row => { - return row.securityFeatures.dependabot.length > 0 ? ( - - {row.securityFeatures.dependabot.map(feature => { - return - })} - - ) : null - }, - minWidth: '200px', - }, - { - header: 'Code scanning', + header: 'undefined (defaults to grow)', field: 'securityFeatures.codeScanning', renderCell: row => { return row.securityFeatures.codeScanning.length > 0 ? ( @@ -1022,7 +638,6 @@ export const AllColumnWidthsGrowWithMinWidth = () => ( ) : null }, - minWidth: '200px', }, ]} /> From f84bf64d276d21e3e87bab6056719f91c7b9ee37 Mon Sep 17 00:00:00 2001 From: Mike Perrotti Date: Tue, 7 Mar 2023 18:41:50 -0500 Subject: [PATCH 6/9] Create .changeset/flat-owls-grin.md --- .changeset/flat-owls-grin.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/flat-owls-grin.md diff --git a/.changeset/flat-owls-grin.md b/.changeset/flat-owls-grin.md new file mode 100644 index 00000000000..3bfa3bced66 --- /dev/null +++ b/.changeset/flat-owls-grin.md @@ -0,0 +1,5 @@ +--- +"@primer/react": patch +--- + +Implements column width features for the DataTable From c2781357ea6e531d3bf7b7e3ad8cfaffb6bd53c1 Mon Sep 17 00:00:00 2001 From: Mike Perrotti Date: Fri, 10 Mar 2023 13:16:54 -0500 Subject: [PATCH 7/9] corrects scope of subgrid styles --- src/DataTable/Table.tsx | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/DataTable/Table.tsx b/src/DataTable/Table.tsx index b42faaa3c96..2d96993ccb1 100644 --- a/src/DataTable/Table.tsx +++ b/src/DataTable/Table.tsx @@ -149,9 +149,12 @@ const StyledTable = styled.table>` } @supports (grid-template-columns: subgrid) { - display: grid; - grid-template-columns: subgrid; - grid-column: -1 /1; + .TableHead, + .TableBody, + .TableRow { + display: grid; + grid-template-columns: subgrid; + grid-column: -1 /1; } ` From 535df574ae1cbd2c0fbb4bea2a13a3631604bd0b Mon Sep 17 00:00:00 2001 From: Mike Perrotti Date: Wed, 15 Mar 2023 16:47:25 -0400 Subject: [PATCH 8/9] fixes type issue caused by CSS custom properties --- src/DataTable/Table.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/DataTable/Table.tsx b/src/DataTable/Table.tsx index 2d96993ccb1..7f197d0983f 100644 --- a/src/DataTable/Table.tsx +++ b/src/DataTable/Table.tsx @@ -189,7 +189,7 @@ const Table = React.forwardRef(function Table( Date: Wed, 15 Mar 2023 16:54:17 -0400 Subject: [PATCH 9/9] manually merge row action stories --- src/DataTable/DataTable.features.stories.tsx | 333 +++++++++++++++++++ 1 file changed, 333 insertions(+) diff --git a/src/DataTable/DataTable.features.stories.tsx b/src/DataTable/DataTable.features.stories.tsx index ab69a3e683c..db8b1b4edd9 100644 --- a/src/DataTable/DataTable.features.stories.tsx +++ b/src/DataTable/DataTable.features.stories.tsx @@ -578,6 +578,339 @@ export const WithActionsOnly = () => ( ) +export const WithRowAction = () => ( + + + Repositories + + + A subtitle could appear here to give extra context to the data. + + { + return + }, + }, + { + header: 'Updated', + field: 'updatedAt', + renderCell: row => { + return + }, + }, + { + header: 'Dependabot', + field: 'securityFeatures.dependabot', + renderCell: row => { + return row.securityFeatures.dependabot.length > 0 ? ( + + {row.securityFeatures.dependabot.map(feature => { + return + })} + + ) : null + }, + }, + { + header: 'Code scanning', + field: 'securityFeatures.codeScanning', + renderCell: row => { + return row.securityFeatures.codeScanning.length > 0 ? ( + + {row.securityFeatures.codeScanning.map(feature => { + return + })} + + ) : null + }, + }, + { + id: 'actions', + header: () => Actions, + renderCell: row => { + return ( + { + action('Download')(row) + }} + /> + ) + }, + }, + ]} + /> + +) + +export const WithRowActions = () => ( + + + Repositories + + + A subtitle could appear here to give extra context to the data. + + { + return + }, + }, + { + header: 'Updated', + field: 'updatedAt', + renderCell: row => { + return + }, + }, + { + header: 'Dependabot', + field: 'securityFeatures.dependabot', + renderCell: row => { + return row.securityFeatures.dependabot.length > 0 ? ( + + {row.securityFeatures.dependabot.map(feature => { + return + })} + + ) : null + }, + }, + { + header: 'Code scanning', + field: 'securityFeatures.codeScanning', + renderCell: row => { + return row.securityFeatures.codeScanning.length > 0 ? ( + + {row.securityFeatures.codeScanning.map(feature => { + return + })} + + ) : null + }, + }, + { + id: 'actions', + header: () => Actions, + renderCell: row => { + return ( + <> + { + action('Edit')(row) + }} + /> + { + action('Delete')(row) + }} + /> + + ) + }, + }, + ]} + /> + +) + +export const WithRowActionMenu = () => ( + + + Repositories + + + A subtitle could appear here to give extra context to the data. + + { + return + }, + }, + { + header: 'Updated', + field: 'updatedAt', + renderCell: row => { + return + }, + }, + { + header: 'Dependabot', + field: 'securityFeatures.dependabot', + renderCell: row => { + return row.securityFeatures.dependabot.length > 0 ? ( + + {row.securityFeatures.dependabot.map(feature => { + return + })} + + ) : null + }, + }, + { + header: 'Code scanning', + field: 'securityFeatures.codeScanning', + renderCell: row => { + return row.securityFeatures.codeScanning.length > 0 ? ( + + {row.securityFeatures.codeScanning.map(feature => { + return + })} + + ) : null + }, + }, + { + id: 'actions', + header: () => Actions, + renderCell: row => { + return ( + + + + + + + { + action('Copy')(row) + }} + > + Copy row + + Edit row + Export row as CSV + + Delete row + + + + ) + }, + }, + ]} + /> + +) + +export const WithCustomHeading = () => ( + <> + + Security coverage + +

+ Organization members can only see data for the most recently-updated repositories. To see all repositories, talk + to your organization administrator about becoming a security manager. +

+ + { + return + }, + }, + { + header: 'Updated', + field: 'updatedAt', + renderCell: row => { + return + }, + }, + { + header: 'Dependabot', + field: 'securityFeatures.dependabot', + renderCell: row => { + return row.securityFeatures.dependabot.length > 0 ? ( + + {row.securityFeatures.dependabot.map(feature => { + return + })} + + ) : null + }, + }, + { + header: 'Code scanning', + field: 'securityFeatures.codeScanning', + renderCell: row => { + return row.securityFeatures.codeScanning.length > 0 ? ( + + {row.securityFeatures.codeScanning.map(feature => { + return + })} + + ) : null + }, + }, + ]} + /> + + +) + export const MixedColumnWidths = () => (