From 23afb702e79e44788d82b6abeb0e2e6ba4deb4d3 Mon Sep 17 00:00:00 2001 From: Ivan Gabriele Date: Thu, 23 May 2024 20:12:57 +0200 Subject: [PATCH] fix(tables)!: normalize tables checkbox, margins & height BREAKING CHANGE: - Remove `isChecked` prop in `TableWithSelectableRows.RowCheckbox`, replaced by native `checked` prop. - Remove `isIndeterminate` prop in `TableWithSelectableRows.RowCheckbox`, replaced by original Rsuite `indeterminate` prop. - Remove `$isHighlighted` prop in `TableWithSelectableRows.Td`, replaced by the same prop on `TableWithSelectableRows.BodyTr`. - In order to work properly with `RowCheckbox`, `TableWithSelectableRows` now requires ``. - Following the `SimpleTable` & `TableWithSelectableRows` margins/heights normalization, including a few hacks, it may break some UI widths & heights. - Please check `TableWithSelectableRows.stories.tsx` to see a full example on how to use/update it. - As shown in the story, be careful **NOT** to wrap the checkbox table header cell within `` since the flex display breaks its internal positioning. --- .eslintrc.cjs | 4 +- src/fields/Checkbox.tsx | 10 +- src/tables/SimpleTable/Td.tsx | 11 ++- src/tables/SimpleTable/index.tsx | 3 +- .../TableWithSelectableRows/RowCheckbox.tsx | 84 +++++++++++----- src/tables/TableWithSelectableRows/index.tsx | 98 +++++++++++++------ .../TableWithSelectableRows.stories.tsx | 39 +++++--- 7 files changed, 174 insertions(+), 75 deletions(-) diff --git a/.eslintrc.cjs b/.eslintrc.cjs index b17067900..e3e179187 100644 --- a/.eslintrc.cjs +++ b/.eslintrc.cjs @@ -71,7 +71,7 @@ module.exports = { format: ['camelCase', 'PascalCase'], prefix: BOOLEAN_CAMEL_PREFIXES, filter: { - regex: '^(checked|disabled|readOnly|searchable|value)$', + regex: '^(_.*|checked|disabled|readOnly|searchable|value)$', match: false } }, @@ -81,7 +81,7 @@ module.exports = { format: ['camelCase', 'PascalCase'], prefix: BOOLEAN_CAMEL_PREFIXES, filter: { - regex: '^(checked|disabled|readOnly|searchable|value)$', + regex: '^(_.*|checked|disabled|readOnly|searchable|value)$', match: false } }, diff --git a/src/fields/Checkbox.tsx b/src/fields/Checkbox.tsx index 625ea79b5..9b0fa57da 100644 --- a/src/fields/Checkbox.tsx +++ b/src/fields/Checkbox.tsx @@ -45,6 +45,8 @@ export function Checkbox({ disabled = false, error, hasError = false, + // eslint-disable-next-line @typescript-eslint/naming-convention + indeterminate = false, isErrorMessageHidden = false, isLight = false, isTransparent = false, @@ -79,7 +81,7 @@ export function Checkbox({ ` +export const StyledRsuiteCheckbox = styled(RsuiteCheckbox)` * { ${p => p.$isReadOnly && `cursor: default !important;`} user-select: none; } - > .rs-checkbox-checker { + > .rs-checkbox-checker, + &.rs-checkbox-indeterminate > .rs-checkbox-checker { min-height: unset; padding: 0 0 0 24px; diff --git a/src/tables/SimpleTable/Td.tsx b/src/tables/SimpleTable/Td.tsx index 909ba25e1..74f040aa2 100644 --- a/src/tables/SimpleTable/Td.tsx +++ b/src/tables/SimpleTable/Td.tsx @@ -19,8 +19,17 @@ export const Td = styled.td.attrs(props => ({ line-height: 22px; max-width: 0; overflow: hidden; - padding: 10px; + padding: 9px 10px; text-align: ${p => (p.$isCenter ? 'center' : 'left')}; text-overflow: ellipsis; white-space: nowrap; + + /** + Dirty hack to prevent 'display: inline-flex;' from breaking row height. + This may be fixable by internal alignment cleanup in component. + */ + > .Element-Tag { + align-self: unset; + vertical-align: bottom; + } ` diff --git a/src/tables/SimpleTable/index.tsx b/src/tables/SimpleTable/index.tsx index 99dc0ed7e..2e0021c2f 100644 --- a/src/tables/SimpleTable/index.tsx +++ b/src/tables/SimpleTable/index.tsx @@ -34,7 +34,8 @@ const Th = styled.th<{ color: ${p => p.theme.color.slateGray}; font-size: 13px; font-weight: 500; - padding: 10px; + line-height: 22px; + padding: 9px 10px; overflow: hidden; text-overflow: ellipsis; ${p => !!p.$width && `width: ${p.$width}px;`} diff --git a/src/tables/TableWithSelectableRows/RowCheckbox.tsx b/src/tables/TableWithSelectableRows/RowCheckbox.tsx index 8650e3efa..dc10c65c5 100644 --- a/src/tables/TableWithSelectableRows/RowCheckbox.tsx +++ b/src/tables/TableWithSelectableRows/RowCheckbox.tsx @@ -1,31 +1,67 @@ +import { StyledRsuiteCheckbox } from '@fields/Checkbox' import { stopMouseEventPropagation } from '@utils/stopMouseEventPropagation' -import { type HTMLProps } from 'react' -import { Checkbox as RsuiteCheckbox } from 'rsuite' - -export type RowCheckboxProps = { - className?: string - disabled?: boolean - isChecked?: boolean - // handle the case where some child checkboxes are checked to display an indeterminate style (-) - isIndeterminate?: boolean - onChange?: (event: React.ChangeEvent) => void +import { useCallback, type ChangeEvent, type HTMLProps } from 'react' +import { type CheckboxProps as RsuiteCheckboxProps } from 'rsuite' +import styled from 'styled-components' + +import type { ValueType } from 'rsuite/esm/Checkbox' + +export type RowCheckboxProps = Omit & { + // TODO Maybe replace that with a `((isChecked: boolean) => Promisable) | undefined` for consistency with other boolean fields? + onChange?: ((event: ChangeEvent) => void) | undefined } -export function RowCheckbox({ - className = '', - disabled = false, - isChecked = false, - isIndeterminate = false, - onChange = () => undefined -}: RowCheckboxProps & HTMLProps) { +export function RowCheckbox({ onChange, ...nativeProps }: RowCheckboxProps & HTMLProps) { + const handleOnChange = useCallback( + (_value: ValueType | undefined, _checked: boolean, event: ChangeEvent) => { + if (onChange) { + onChange(event) + } + }, + [onChange] + ) + return ( - onChange(event)} + ) } + +const RestyledRsuiteCheckbox = styled(StyledRsuiteCheckbox)` + vertical-align: top; + + > .rs-checkbox-checker, + &.rs-checkbox-indeterminate > .rs-checkbox-checker { + padding: 0; + + > label { + > .rs-checkbox-wrapper { + bottom: 0; + top: 3px; + + &:before { + } + &:after { + bottom: 0; + left: 0; + right: 0; + top: 0; + } + + > .rs-checkbox-inner { + &:before { + } + + /* Checkmark */ + &:after { + } + } + } + } + } +` diff --git a/src/tables/TableWithSelectableRows/index.tsx b/src/tables/TableWithSelectableRows/index.tsx index a3d567f48..4566d3c6f 100644 --- a/src/tables/TableWithSelectableRows/index.tsx +++ b/src/tables/TableWithSelectableRows/index.tsx @@ -3,24 +3,58 @@ import styled from 'styled-components' import { RowCheckbox } from './RowCheckbox' import { SimpleTable } from '../SimpleTable' -const Table = styled(SimpleTable.Table)` +const Table = styled(SimpleTable.Table)<{ + $withRowCheckbox?: boolean +}>` border-collapse: separate; border-spacing: 0 5px; table-layout: fixed; + + ${p => + !!p.$withRowCheckbox && + ` + > thead > tr > th:first-child { + padding: 0 0 0 8px; + + > .rs-checkbox { + > .rs-checkbox-checker { + > label { + > .rs-checkbox-wrapper { + left: -8px; + } + } + } + } + } + + > tbody > tr > td:first-child { + padding: 0 0 0 8px; + } + `} ` const Head = styled(SimpleTable.Head)` - > th:last-child { - border-right: 1px solid ${p => p.theme.color.lightGray}; + > tr { + > th { + border-bottom: 1px solid ${p => p.theme.color.lightGray}; + border-right: none; + border-top: 1px solid ${p => p.theme.color.lightGray}; + + &:first-child { + border-left: 1px solid ${p => p.theme.color.lightGray}; + } + + &:last-child { + border-right: 1px solid ${p => p.theme.color.lightGray}; + } + } } ` const Th = styled(SimpleTable.Th)` background-color: ${p => p.theme.color.white}; - border-top: 1px solid ${p => p.theme.color.lightGray}; - border-bottom: 1px solid ${p => p.theme.color.lightGray}; - border-right: none; - padding: 2px 16px; + line-height: 22px; + padding: 9px 16px; ` const SortContainer = styled(SimpleTable.SortContainer)` @@ -28,34 +62,40 @@ const SortContainer = styled(SimpleTable.SortContainer)` justify-content: start; ` -const BodyTr = styled(SimpleTable.BodyTr)<{ $isHighlighted?: boolean }>` - > td:first-child { - border-left: ${p => - p.$isHighlighted ? `2px solid ${p.theme.color.blueGray}` : `1px solid ${p.theme.color.lightGray}`}; - } - > td:last-child { - border-right: ${p => - p.$isHighlighted ? `2px solid ${p.theme.color.blueGray}` : `1px solid ${p.theme.color.lightGray}`}; - overflow: visible; +const BodyTr = styled(SimpleTable.BodyTr)<{ + $isHighlighted?: boolean +}>` + > td { + border-bottom: 1px solid ${p => (p.$isHighlighted ? p.theme.color.blueGray : p.theme.color.lightGray)}; + border-right: none; + border-top: 1px solid ${p => (p.$isHighlighted ? p.theme.color.blueGray : p.theme.color.lightGray)}; + ${p => + !!p.$isHighlighted && + `box-shadow: 0 -1px 0 ${p.theme.color.blueGray} inset, 0 1px 0 ${p.theme.color.blueGray} inset;`} + + &:first-child { + border-left: 1px solid ${p => (p.$isHighlighted ? p.theme.color.blueGray : p.theme.color.lightGray)}; + ${p => + !!p.$isHighlighted && + `box-shadow: 0 -1px 0 ${p.theme.color.blueGray} inset, 0 1px 0 ${p.theme.color.blueGray} inset, 1px 0 0 ${p.theme.color.blueGray} inset;`} + } + + &:last-child { + border-right: 1px solid ${p => (p.$isHighlighted ? p.theme.color.blueGray : p.theme.color.lightGray)}; + ${p => + !!p.$isHighlighted && + `box-shadow: 0 -1px 0 ${p.theme.color.blueGray} inset, 0 1px 0 ${p.theme.color.blueGray} inset, -1px 0 0 ${p.theme.color.blueGray} inset;`} + overflow: visible; + } } ` const Td = styled(SimpleTable.Td)<{ - $hasRightBorder: boolean - $isHighlighted?: boolean - // TODO This should be removed, a table column width should only be set via its `th` width. - /** @deprecated Will be removed in the next major version. Use `Td.$width` instead to set columns width. */ - $width?: number | undefined + $hasRightBorder?: boolean }>` background-color: ${p => p.theme.color.cultured}; - border-bottom: ${p => - p.$isHighlighted ? `2px solid ${p.theme.color.blueGray}` : `1px solid ${p.theme.color.lightGray}`}; - border-right: none; - border-right: ${p => (p.$hasRightBorder ? `1px solid ${p.theme.color.lightGray}` : '')}; - border-top: ${p => - p.$isHighlighted ? `2px solid ${p.theme.color.blueGray}` : `1px solid ${p.theme.color.lightGray}`}; - padding: 4px 16px; - width: ${p => p.$width}px; + ${p => !!p.$hasRightBorder && `border-right: 1px solid ${p.theme.color.lightGray};`} + padding: 9px 16px; ` export const TableWithSelectableRows = { diff --git a/stories/tables/TableWithSelectableRows.stories.tsx b/stories/tables/TableWithSelectableRows.stories.tsx index facbbf368..6da5c4fda 100644 --- a/stories/tables/TableWithSelectableRows.stories.tsx +++ b/stories/tables/TableWithSelectableRows.stories.tsx @@ -11,7 +11,7 @@ import { Link } from '../../src/icons' import type { Meta } from '@storybook/react' -const fakeData1 = Array(100).fill({ +const fakeData1 = Array(5).fill({ actionTaken: null, controlUnitId: null, createdAt: '2023-08-04T15:13:43.296Z', @@ -48,7 +48,7 @@ const fakeData1 = Array(100).fill({ validityTime: 1, vehicleType: null }) -const fakeData2 = Array(100).fill({ +const fakeData2 = Array(5).fill({ actionTaken: 'ACTION TAKEN', controlUnitId: null, createdAt: '2023-08-01T15:13:01.073587Z', @@ -127,21 +127,26 @@ export function _TableWithSelectableRows() { accessorFn: row => row.reportingId, cell: ({ row }) => ( ), enableSorting: false, header: ({ table }) => ( { + console.log('indeterminate', table.getIsSomeRowsSelected()) + + return table.getIsSomeRowsSelected() + })()} onChange={table.getToggleAllRowsSelectedHandler()} /> ), id: 'select', - size: 25 + + size: 25 // 24px + 1px to avoid cutting the right border }, { accessorFn: row => row.reportingId, @@ -221,7 +226,7 @@ export function _TableWithSelectableRows() { enableSorting: false, header: () => '', id: 'missionId', - size: 85 + size: 120 }, { accessorFn: row => row.geom, @@ -238,12 +243,17 @@ export function _TableWithSelectableRows() { enableSorting: false, header: () => '', id: 'actionStatus', - size: 85 + size: 112 }, { accessorFn: row => row.geom, cell: info => ( - console.log(info.getValue())} /> + console.log(info.getValue())} + /> ), enableSorting: false, header: () => '', @@ -321,13 +331,14 @@ export function _TableWithSelectableRows() {
- + {table.getHeaderGroups().map(headerGroup => (
{headerGroup.headers.map(header => ( - {header.isPlaceholder ? undefined : ( + {header.id === 'select' && flexRender(header.column.columnDef.header, header.getContext())} + {header.id !== 'select' && !header.isPlaceholder && ( ▲, - desc:
▼
+ asc: , + desc: }[header.column.getIsSorted() as string] ?? )} )} @@ -361,8 +372,6 @@ export function _TableWithSelectableRows() { key={cell.id} $hasRightBorder={!!(cell.column.id === 'geom')} $isCenter={!!(cell.column.id === 'geom' || cell.column.id === 'id')} - $isHighlighted={index % 2 === 0} - $width={cell.column.getSize()} > {flexRender(cell.column.columnDef.cell, cell.getContext())}