refactor: list and table to be an actual table in dom (#4535)

* refactor: list and table to be an actual table in dom * refactor: list and table * fix: adding the column width elements * fix: improve accessibility * fix: table and list * fix: expandable content
scaleway · Dec 10, 2024 · 612316c · 612316c
1 parent 519b9b6
commit 612316c
Show file tree

Hide file tree

Showing 24 changed files with 9,445 additions and 9,311 deletions.
diff --git a/.changeset/poor-carpets-accept.md b/.changeset/poor-carpets-accept.md
@@ -0,0 +1,18 @@
+---
+"@ultraviolet/ui": minor
+---
+
+Refactoring of `<List />` and `<Table />` components:
+
+### List
+
+List used to be a bunch of `div` with different `role` to work like a table but it suffered from columns issues if not correctly set.
+It now uses a real `table` element and `thead` and `tbody` to correctly handle columns and rows.
+Each elements uses `display: table-*` to correctly handle columns and rows.
+The expandable is tricked, by adding a new `tr` on click and a `transform` to move it up under the clicked row.
+
+### Table
+
+Table was correctly using `table` element but the columns were not correctly handled.
+It now uses `display: table-*` for each elements of the table.
+The expandable is an added `tr` under the clicked row.
diff --git a/packages/ui/src/components/List/Body.tsx b/packages/ui/src/components/List/Body.tsx
diff --git a/packages/ui/src/components/List/Cell.tsx b/packages/ui/src/components/List/Cell.tsx
@@ -7,11 +7,11 @@ import type {
 } from 'react'
 import { forwardRef } from 'react'
 
-const StyledCell = styled.div`
-  display: flex;
-  justify-content: center;
-  flex-direction: column;
-  min-height: ${({ theme }) => theme.sizing['750']};
+const StyledCell = styled.td`
+  display: table-cell;
+  vertical-align: middle;
+  height: ${({ theme }) => theme.sizing['750']};
+  padding: 0 ${({ theme }) => theme.space['2']};
 `
 
 type CellProps = {
@@ -24,12 +24,19 @@ type CellProps = {
    * */
   preventClick?: boolean
   'data-testid'?: string
+  colSpan?: number
 }
 
 export const Cell = forwardRef(
   (
-    { children, className, preventClick, 'data-testid': dataTestid }: CellProps,
-    ref: ForwardedRef<HTMLDivElement>,
+    {
+      children,
+      className,
+      preventClick,
+      'data-testid': dataTestid,
+      colSpan,
+    }: CellProps,
+    ref: ForwardedRef<HTMLTableCellElement>,
   ) => {
     const handleClick: MouseEventHandler<HTMLDivElement> = event => {
       if (preventClick) {
@@ -46,11 +53,11 @@ export const Cell = forwardRef(
     return (
       <StyledCell
         ref={ref}
-        role="cell"
         className={className}
         onClick={handleClick}
         onKeyDown={handleKeyDown}
         data-testid={dataTestid}
+        colSpan={colSpan}
       >
         {children}
       </StyledCell>

diff --git a/packages/ui/src/components/List/HeaderCell.tsx b/packages/ui/src/components/List/HeaderCell.tsx
@@ -5,6 +5,7 @@ import {
   SouthShortIcon,
 } from '@ultraviolet/icons'
 import type { ReactNode } from 'react'
+import { Stack } from '../Stack'
 import { Tooltip } from '../Tooltip'
 
 const StyledSortIcon = styled(SouthShortIcon, {
@@ -21,16 +22,18 @@ const SortIcon = ({ order }: { order?: 'ascending' | 'descending' }) =>
     <SortIconUV sentiment="neutral" />
   )
 
-const StyledHeaderCell = styled.div`
-  display: flex;
+const StyledHeaderCell = styled('th', {
+  shouldForwardProp: prop => !['width', 'minWidth', 'maxWidth'].includes(prop),
+})<{ width?: string; minWidth?: string; maxWidth?: string }>`
+  display: table-cell;
   text-align: left;
-  flex-direction: row;
-  align-items: center;
+  vertical-align: middle;
   font-size: ${({ theme }) => theme.typography.bodySmall.fontSize};
   font-weight: ${({ theme }) => theme.typography.bodySmall.weight};
   font-family: ${({ theme }) => theme.typography.bodySmall.fontFamily};
   color: ${({ theme }) => theme.colors.neutral.text};
   gap: ${({ theme }) => theme.space['1']};
+  padding: 0 ${({ theme }) => theme.space['2']};
 
   &[role*='button'] {
     cursor: pointer;
@@ -40,6 +43,10 @@ const StyledHeaderCell = styled.div`
   &[aria-sort] {
     color: ${({ theme }) => theme.colors.primary.text};
   }
+
+  width: ${({ width }) => width};
+  min-width: ${({ minWidth }) => minWidth};
+  max-width: ${({ maxWidth }) => maxWidth};
 `
 
 type HeaderCellProps = {
@@ -49,6 +56,9 @@ type HeaderCellProps = {
   orderDirection?: 'asc' | 'desc' | 'none'
   onOrder?: (newOrder: 'asc' | 'desc') => void
   info?: string
+  width?: string
+  minWith?: string
+  maxWidth?: string
 }
 
 export const HeaderCell = ({
@@ -58,6 +68,9 @@ export const HeaderCell = ({
   onOrder,
   className,
   info,
+  width,
+  minWith,
+  maxWidth,
 }: HeaderCellProps) => {
   let order: undefined | 'ascending' | 'descending'
   if (isOrdered && orderDirection === 'asc') {
@@ -90,16 +103,25 @@ export const HeaderCell = ({
       }
       role={onOrder ? 'button columnheader' : undefined}
       tabIndex={handleOrder ? 0 : -1}
+      width={width}
+      maxWidth={maxWidth}
+      minWidth={minWith}
     >
-      {children}
-      {info ? (
-        <Tooltip text={info}>
-          <InformationIcon size="large" prominence="weak" sentiment="neutral" />
-        </Tooltip>
-      ) : null}
-      {orderDirection !== undefined && isOrdered !== undefined ? (
-        <SortIcon order={order} />
-      ) : null}
+      <Stack direction="row" gap={1} alignItems="center">
+        {children}
+        {info ? (
+          <Tooltip text={info}>
+            <InformationIcon
+              size="large"
+              prominence="weak"
+              sentiment="neutral"
+            />
+          </Tooltip>
+        ) : null}
+        {orderDirection !== undefined && isOrdered !== undefined ? (
+          <SortIcon order={order} />
+        ) : null}
+      </Stack>
     </StyledHeaderCell>
   )
 }
diff --git a/packages/ui/src/components/List/HeaderRow.tsx b/packages/ui/src/components/List/HeaderRow.tsx
@@ -1,13 +1,42 @@
+import { useTheme } from '@emotion/react'
 import styled from '@emotion/styled'
 import type { ReactNode } from 'react'
 import { Checkbox } from '../Checkbox'
+import {
+  EXPANDABLE_COLUMN_SIZE,
+  SELECTABLE_CHECKBOX_SIZE,
+} from '../Table/constants'
 import { HeaderCell } from './HeaderCell'
 import { useListContext } from './ListContext'
 
-const StyledHeaderRow = styled.div`
+const StyledHeaderRow = styled.tr`
   /* List itself also apply style about common templating between HeaderRow and other Rows */
-  column-gap: ${({ theme }) => theme.space['2']};
+  display: table-row;
+  vertical-align: middle;
   padding: 0 ${({ theme }) => theme.space['2']};
+
+  &:before {
+    content: "";
+    position: absolute;
+    top: 0; /* Adjust based on border width and spacing */
+    left: 0;
+    right: 0;
+    bottom: 0; /* Adjust based on border width and spacing */
+    border: 1px solid transparent;
+    border-radius: ${({ theme }) => theme.radii.default};
+    pointer-events: none;
+    transition:
+      box-shadow 200ms ease,
+      border-color 200ms ease;
+  }
+`
+
+const NoPaddingHeaderCell = styled(HeaderCell)`
+  padding: 0;
+
+  &:first-of-type {
+    padding-left: ${({ theme }) => theme.space['2']};
+  }
 `
 
 type RowProps = {
@@ -24,24 +53,32 @@ export const HeaderRow = ({ children, hasSelectAllColumn }: RowProps) => {
     expandButton,
   } = useListContext()
 
+  const theme = useTheme()
+
   const selectableRowCount = Object.keys(selectedRowIds).length
 
   return (
-    <StyledHeaderRow role="row">
-      {hasSelectAllColumn ? (
-        <HeaderCell>
-          <Checkbox
-            name="list-select-checkbox"
-            value="all"
-            aria-label="select all"
-            checked={allRowSelectValue}
-            onChange={allRowSelectValue === false ? selectAll : unselectAll}
-            disabled={selectableRowCount === 0}
-          />
-        </HeaderCell>
-      ) : null}
-      {expandButton ? <HeaderCell>{null}</HeaderCell> : null}
-      {children}
-    </StyledHeaderRow>
+    <thead>
+      <StyledHeaderRow>
+        {hasSelectAllColumn ? (
+          <NoPaddingHeaderCell width={theme.sizing[SELECTABLE_CHECKBOX_SIZE]}>
+            <Checkbox
+              name="list-select-checkbox"
+              value="all"
+              aria-label="select all"
+              checked={allRowSelectValue}
+              onChange={allRowSelectValue === false ? selectAll : unselectAll}
+              disabled={selectableRowCount === 0}
+            />
+          </NoPaddingHeaderCell>
+        ) : null}
+        {expandButton ? (
+          <NoPaddingHeaderCell width={theme.sizing[EXPANDABLE_COLUMN_SIZE]}>
+            {null}
+          </NoPaddingHeaderCell>
+        ) : null}
+        {children}
+      </StyledHeaderRow>
+    </thead>
   )
 }