Merge pull request #1434 from dnbexperience/fix/align-table

fix(Table): align HTML/CSS classes and types
dnbexperience · Jun 9, 2022 · 20d2ea2 · 20d2ea2
2 parents e28ae07 + ff0be8d
commit 20d2ea2
Show file tree

Hide file tree

Showing 9 changed files with 128 additions and 106 deletions.
diff --git a/packages/dnb-eufemia/src/components/table/Table.tsx b/packages/dnb-eufemia/src/components/table/Table.tsx
@@ -26,19 +26,16 @@ export { StickyHelper }
 export interface TableProps extends StickyTableHeaderProps {
   /**
    * The content of the component.
-   * Default: null
    */
-  children?: React.ReactNode
+  children: React.ReactNode
 
   /**
    * Custom className on the component root
-   * Default: null
    */
   className?: string
 
   /**
    * Skeleton should be applied when loading content
-   * Default: null
    */
   skeleton?: SkeletonShow
 
@@ -60,7 +57,11 @@ export const defaultProps = {
   variant: 'basis',
 }
 
-const Table = (componentProps: TableProps & ISpacingProps) => {
+const Table = (
+  componentProps: TableProps &
+    React.TableHTMLAttributes<HTMLTableElement> &
+    ISpacingProps
+) => {
   const context = React.useContext(Context)
 
   const allProps = usePropsWithContext(
@@ -91,33 +92,25 @@ const Table = (componentProps: TableProps & ISpacingProps) => {
 
   validateDOMAttributes(allProps, props)
 
-  const Element = () => (
-    <table
-      className={classnames(
-        'dnb-table',
-        `dnb-table--${variant || 'basis'}`,
-        `dnb-table--size-${size || 'large'}`,
-        sticky && `dnb-table--sticky`,
-        spacingClasses,
-        skeletonClasses,
-        className
-      )}
-      ref={elementRef}
-      {...props}
-    >
-      {children}
-    </table>
+  return (
+    <Provider skeleton={Boolean(skeleton)}>
+      <table
+        className={classnames(
+          'dnb-table',
+          `dnb-table__variant--${variant || 'basis'}`,
+          `dnb-table__size--${size || 'large'}`,
+          sticky && `dnb-table--sticky`,
+          spacingClasses,
+          skeletonClasses,
+          className
+        )}
+        ref={elementRef}
+        {...props}
+      >
+        {children}
+      </table>
+    </Provider>
   )
-
-  if (skeleton) {
-    return (
-      <Provider skeleton>
-        <Element />
-      </Provider>
-    )
-  }
-
-  return <Element />
 }
 
 export default Table

diff --git a/packages/dnb-eufemia/src/components/table/TableTd.tsx b/packages/dnb-eufemia/src/components/table/TableTd.tsx
@@ -1,7 +1,7 @@
 import React from 'react'
 import classnames from 'classnames'
 
-type TableTdProps = {
+export type TableTdProps = {
   /**
    * The content of the component.
    * Default: null
@@ -13,11 +13,12 @@ type TableTdProps = {
    * Default: null
    */
   className?: string
-} & React.TdHTMLAttributes<HTMLTableCellElement>
-
-export const defaultProps = {}
+}
 
-const Td = (componentProps: TableTdProps) => {
+const Td = (
+  componentProps: TableTdProps &
+    React.TdHTMLAttributes<HTMLTableCellElement>
+) => {
   const {
     className,
     children,
@@ -26,8 +27,11 @@ const Td = (componentProps: TableTdProps) => {
   } = componentProps
 
   return (
-    <td className={classnames('dnb-td', className)} {...props}>
-      {children}
+    <td
+      role="cell"
+      className={classnames('dnb-table__td', className)}
+      {...props}
+    >
     </td>
   )
 }

diff --git a/packages/dnb-eufemia/src/components/table/TableTh.tsx b/packages/dnb-eufemia/src/components/table/TableTh.tsx
@@ -1,23 +1,23 @@
 import React from 'react'
 import classnames from 'classnames'
 
-type TableThProps = {
+export type TableThProps = {
   /**
-   * The content of the component.
-   * Default: null
+   * The content of the table header given as Tr.
    */
-  children?: React.ReactNode
+  children: React.ReactNode
 
   /**
    * Custom className on the component root
    * Default: null
    */
   className?: string
-} & React.ThHTMLAttributes<HTMLTableCellElement>
-
-export const defaultProps = {}
+}
 
-const Th = (componentProps: TableThProps) => {
+const Th = (
+  componentProps: TableThProps &
+    React.ThHTMLAttributes<HTMLTableCellElement>
+) => {
   const {
     className,
     children,
@@ -26,7 +26,14 @@ const Th = (componentProps: TableThProps) => {
   } = componentProps
 
   return (
-    <th className={classnames('dnb-th', className)} {...props}>
+    <th
+      role="columnheader"
+      className={classnames(
+        'dnb-table__th',
+        className
+      )}
+      {...props}
+    >
       {children}
     </th>
   )

diff --git a/packages/dnb-eufemia/src/components/table/TableTr.tsx b/packages/dnb-eufemia/src/components/table/TableTr.tsx
@@ -1,23 +1,24 @@
 import React from 'react'
 import classnames from 'classnames'
+import { TableTdProps } from './TableTd'
 
-type TableTrProps = {
+export type TableTrProps = {
   /**
    * The content of the component.
-   * Default: null
    */
-  children?: React.ReactNode
+  children: Array<React.ReactElement<TableTdProps>>
 
   /**
    * Custom className on the component root
    * Default: null
    */
   className?: string
-} & React.TableHTMLAttributes<HTMLTableRowElement>
-
-export const defaultProps = {}
+}
 
-const Tr = (componentProps: TableTrProps) => {
+const Tr = (
+  componentProps: TableTrProps &
+    React.TableHTMLAttributes<HTMLTableRowElement>
+) => {
   const {
     className,
     children,
@@ -26,7 +27,7 @@ const Tr = (componentProps: TableTrProps) => {
   } = componentProps
 
   return (
-    <tr className={classnames('dnb-tr', className)} {...props}>
+    <tr className={classnames('dnb-table__tr', className)} {...props}>
       {children}
     </tr>
   )

diff --git a/packages/dnb-eufemia/src/components/table/__tests__/Table.test.tsx b/packages/dnb-eufemia/src/components/table/__tests__/Table.test.tsx
@@ -39,8 +39,8 @@ describe('Table', () => {
 
     expect(Array.from(screen.queryByRole('table').classList)).toEqual([
       'dnb-table',
-      'dnb-table--basis',
-      'dnb-table--size-large',
+      'dnb-table__variant--basis',
+      'dnb-table__size--large',
     ])
   })
 
@@ -53,8 +53,8 @@ describe('Table', () => {
 
     expect(Array.from(screen.queryByRole('table').classList)).toEqual([
       'dnb-table',
-      'dnb-table--not-defined-yet',
-      'dnb-table--size-large',
+      'dnb-table__variant--not-defined-yet',
+      'dnb-table__size--large',
     ])
   })
 
@@ -67,8 +67,8 @@ describe('Table', () => {
 
     expect(Array.from(screen.queryByRole('table').classList)).toEqual([
       'dnb-table',
-      'dnb-table--basis',
-      'dnb-table--size-medium',
+      'dnb-table__variant--basis',
+      'dnb-table__size--medium',
     ])
   })
 
@@ -81,8 +81,8 @@ describe('Table', () => {
 
     expect(Array.from(screen.queryByRole('table').classList)).toEqual([
       'dnb-table',
-      'dnb-table--basis',
-      'dnb-table--size-large',
+      'dnb-table__variant--basis',
+      'dnb-table__size--large',
       'dnb-table--fixed',
     ])
 
@@ -106,8 +106,8 @@ describe('Table', () => {
     expect(attributes).toEqual(['class'])
     expect(Array.from(element.classList)).toEqual([
       'dnb-table',
-      'dnb-table--basis',
-      'dnb-table--size-large',
+      'dnb-table__variant--basis',
+      'dnb-table__size--large',
       'dnb-space__top--large',
     ])
   })
@@ -127,8 +127,8 @@ describe('Table', () => {
     expect(attributes).toEqual(['class'])
     expect(Array.from(element.classList)).toEqual([
       'dnb-table',
-      'dnb-table--basis',
-      'dnb-table--size-large',
+      'dnb-table__variant--basis',
+      'dnb-table__size--large',
       'dnb-skeleton',
       'dnb-skeleton--font',
     ])

diff --git a/packages/dnb-eufemia/src/components/table/__tests__/TableStickyHeader.test.tsx b/packages/dnb-eufemia/src/components/table/__tests__/TableStickyHeader.test.tsx
@@ -61,8 +61,8 @@ describe('useStickyHeader', () => {
     ])
     expect(Array.from(screen.queryByRole('table').classList)).toEqual([
       'dnb-table',
-      'dnb-table--basis',
-      'dnb-table--size-large',
+      'dnb-table__variant--basis',
+      'dnb-table__size--large',
       'dnb-table--sticky',
     ])
   })
@@ -77,15 +77,19 @@ describe('useStickyHeader', () => {
       </Table>
     )
 
-    expect(getTrClasses()).toEqual(['dnb-tr', 'sticky'])
+    expect(getTrClasses()).toEqual(['dnb-table__tr', 'sticky'])
 
     simulateEntry([{ isIntersecting: false }])
 
-    expect(getTrClasses()).toEqual(['dnb-tr', 'sticky', 'show-shadow'])
+    expect(getTrClasses()).toEqual([
+      'dnb-table__tr',
+      'sticky',
+      'show-shadow',
+    ])
 
     simulateEntry([{ isIntersecting: true }])
 
-    expect(getTrClasses()).toEqual(['dnb-tr', 'sticky'])
+    expect(getTrClasses()).toEqual(['dnb-table__tr', 'sticky'])
   })
 
   it('should use default stickyOffset when not given', () => {

diff --git a/packages/dnb-eufemia/src/components/table/__tests__/__snapshots__/Table.test.tsx.snap b/packages/dnb-eufemia/src/components/table/__tests__/__snapshots__/Table.test.tsx.snap
@@ -23,52 +23,53 @@ exports[`Table scss have to match default theme snapshot 1`] = `
   .dnb-table > tr > th,
   .dnb-table > tr > td,
   .dnb-table > thead > tr > th,
-  .dnb-table > tbody > tr > td,
-  .dnb-table .dnb-table__th,
-  .dnb-table .dnb-table__td {
+  .dnb-table > tbody > tr > td, .dnb-table__th, .dnb-table__td {
     border-bottom: 1px solid transparent;
     font-size: var(--font-size-basis);
     line-height: var(--line-height-basis);
     border-spacing: 0;
     word-break: keep-all; }
   .dnb-table > tr > th,
-  .dnb-table > thead > tr > th,
-  .dnb-table .dnb-table__th {
+  .dnb-table > thead > tr > th, .dnb-table__th {
     padding: var(--spacing-x-large) var(--spacing-small) var(--spacing-x-small);
     font-weight: var(--font-weight-medium);
     color: var(--color-emerald-green);
     vertical-align: bottom;
     background-color: var(--color-mint-green-50);
     border-bottom: 1px solid var(--color-mint-green); }
     .dnb-table > tr > th:not([align]):not(.dnb-table--left):not(.dnb-table--right):not(.dnb-table--center),
-    .dnb-table > thead > tr > th:not([align]):not(.dnb-table--left):not(.dnb-table--right):not(.dnb-table--center),
-    .dnb-table .dnb-table__th:not([align]):not(.dnb-table--left):not(.dnb-table--right):not(.dnb-table--center) {
+    .dnb-table > thead > tr > th:not([align]):not(.dnb-table--left):not(.dnb-table--right):not(.dnb-table--center), .dnb-table__th:not([align]):not(.dnb-table--left):not(.dnb-table--right):not(.dnb-table--center) {
       text-align: inherit; }
     @supports (-moz-appearance: meterbar) and (background-blend-mode: difference, normal) {
       .dnb-table > tr > th,
-      .dnb-table > thead > tr > th,
-      .dnb-table .dnb-table__th {
+      .dnb-table > thead > tr > th, .dnb-table__th {
         padding-bottom: calc(var(--spacing-x-small) - 0.5px); } }
   .dnb-table > tr > td,
-  .dnb-table > tbody > tr > td,
-  .dnb-table .dnb-table__td,
+  .dnb-table > tbody > tr > td, .dnb-table__td,
   .dnb-table > tr > th[scope='row'] {
     padding: var(--spacing-small);
     padding-top: calc(var(--spacing-small) * 1.25);
     padding-bottom: calc(var(--spacing-small) * 1.188);
     /* 19/16 */
     color: var(--theme-color-black-80, currentColor); }
-  .dnb-table > tbody > tr:not(.dnb-table--ignore),
-  .dnb-table .dnb-table__tr:not(.dnb-table--ignore),
-  .dnb-table .dnb-table--odd:not(.dnb-table--ignore) {
+  .dnb-table > tbody > tr,
+  .dnb-table .dnb-table__tr,
+  .dnb-table .dnb-table__tr--odd {
     background-color: var(--color-white); }
-  .dnb-table > tbody > tr:not(.dnb-table--odd):nth-of-type(2n):not(.dnb-table--ignore),
-  .dnb-table .dnb-table__tr:not(.dnb-table--odd):nth-of-type(2n):not(.dnb-table--ignore),
-  .dnb-table .dnb-table--even:not(.dnb-table--ignore) {
+  .dnb-table > tbody > tr:not(.dnb-table__tr--odd):nth-of-type(2n),
+  .dnb-table .dnb-table__tr:not(.dnb-table__tr--odd):nth-of-type(2n),
+  .dnb-table .dnb-table__tr--even {
     background-color: var(--color-mint-green-12); }
-  .dnb-table > tbody > tr:last-of-type > td,
-  .dnb-table .dnb-table__tr:last-of-type .dnb-table__td {
-    border-bottom: 1px solid var(--color-mint-green-50); }
+  .dnb-table > tbody > tr:last-of-type::after,
+  .dnb-table > tbody > .dnb-table__tr:last-of-type::after {
+    content: '';
+    position: absolute;
+    left: 0;
+    right: 0;
+    bottom: 0;
+    z-index: 2;
+    height: 1px;
+    background-color: var(--color-mint-green-50); }
   .dnb-table > thead > tr > th.dnb-table--sortable,
   .dnb-table .dnb-table__th.dnb-table--sortable {
     color: var(--color-emerald-green); }

diff --git a/packages/dnb-eufemia/src/components/table/style/_table.scss b/packages/dnb-eufemia/src/components/table/style/_table.scss
@@ -3,7 +3,6 @@
 *
 */
 
-// @import '../../button/style/themes/button-mixins.scss';
 
 .dnb-table {
   display: table;
-Original file line number
+Diff line change
@@ Expand Up / @@ -3,7 +3,6 @@ @@
     *
     */
-    // @import '../../button/style/themes/button-mixins.scss';
     .dnb-table {
       display: table;
@@ Expand Down @@