Skip to content

Commit

Permalink
fix(tables)!: normalize tables checkbox, margins & height
Browse files Browse the repository at this point in the history
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 `<Table $withRowCheckbox />`.
- 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 `<TableWithSelectableRows.SortContainer />` since the flex display breaks its internal positioning.
  • Loading branch information
ivangabriele committed May 27, 2024
1 parent 176b161 commit 23afb70
Show file tree
Hide file tree
Showing 7 changed files with 174 additions and 75 deletions.
4 changes: 2 additions & 2 deletions .eslintrc.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
},
Expand All @@ -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
}
},
Expand Down
10 changes: 7 additions & 3 deletions src/fields/Checkbox.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -79,14 +81,15 @@ export function Checkbox({
<StyledRsuiteCheckbox
key={key}
$hasError={hasControlledError}
$isChecked={checked}
$isChecked={checked || indeterminate}
$isDisabled={disabled}
$isLight={isLight}
$isReadOnly={readOnly}
$isTransparent={isTransparent}
checked={checked}
disabled={disabled}
id={name}
indeterminate={indeterminate}
name={name}
onChange={handleChange}
readOnly={readOnly}
Expand All @@ -100,13 +103,14 @@ export function Checkbox({
)
}

const StyledRsuiteCheckbox = styled(RsuiteCheckbox)<CommonChoiceFieldStyleProps>`
export const StyledRsuiteCheckbox = styled(RsuiteCheckbox)<CommonChoiceFieldStyleProps>`
* {
${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;
Expand Down
11 changes: 10 additions & 1 deletion src/tables/SimpleTable/Td.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,17 @@ export const Td = styled.td.attrs<TdProps, TdProps>(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 <Tag /> component.
*/
> .Element-Tag {
align-self: unset;
vertical-align: bottom;
}
`
3 changes: 2 additions & 1 deletion src/tables/SimpleTable/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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;`}
Expand Down
84 changes: 60 additions & 24 deletions src/tables/TableWithSelectableRows/RowCheckbox.tsx
Original file line number Diff line number Diff line change
@@ -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<HTMLInputElement>) => 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<RsuiteCheckboxProps, 'onClick' | 'onChange'> & {
// TODO Maybe replace that with a `((isChecked: boolean) => Promisable<void>) | undefined` for consistency with other boolean fields?
onChange?: ((event: ChangeEvent<HTMLInputElement>) => void) | undefined
}
export function RowCheckbox({
className = '',
disabled = false,
isChecked = false,
isIndeterminate = false,
onChange = () => undefined
}: RowCheckboxProps & HTMLProps<HTMLInputElement>) {
export function RowCheckbox({ onChange, ...nativeProps }: RowCheckboxProps & HTMLProps<HTMLInputElement>) {
const handleOnChange = useCallback(
(_value: ValueType | undefined, _checked: boolean, event: ChangeEvent<HTMLInputElement>) => {
if (onChange) {
onChange(event)
}
},
[onChange]
)

return (
<RsuiteCheckbox
checked={isChecked}
className={`${className} cursor-pointer`}
disabled={disabled}
indeterminate={isIndeterminate}
// eslint-disable-next-line @typescript-eslint/naming-convention
onChange={(_, __, event) => onChange(event)}
<RestyledRsuiteCheckbox
$isChecked={nativeProps.checked || nativeProps.indeterminate}
$isDisabled={nativeProps.disabled}
$isReadOnly={nativeProps.readOnly}
{...nativeProps}
onChange={handleOnChange}
onClick={stopMouseEventPropagation}
/>
)
}

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 {
}
}
}
}
}
`
98 changes: 69 additions & 29 deletions src/tables/TableWithSelectableRows/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,59 +3,99 @@ 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)`
gap: 8px;
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 = {
Expand Down
Loading

0 comments on commit 23afb70

Please sign in to comment.