fix(Table): simplify sticky table hook

In regards to PR #1422 this fixes some issue and makes the sticky functionally more robust.
dnbexperience · Jun 9, 2022 · f1e7e3b · f1e7e3b
1 parent 20d2ea2
commit f1e7e3b
Show file tree

Hide file tree

Showing 7 changed files with 89 additions and 71 deletions.
diff --git a/packages/dnb-design-system-portal/src/docs/uilib/components/table/Examples.js b/packages/dnb-design-system-portal/src/docs/uilib/components/table/Examples.js
@@ -3,6 +3,8 @@
  *
  */
 
+import React from 'react'
+import { css, Global } from '@emotion/react'
 import ComponentBox from 'dnb-design-system-portal/src/shared/tags/ComponentBox'
 
 export const TableVariantBasic = () => (
@@ -156,9 +158,25 @@ export const TableLongHeader = () => (
 )
 
 export const TableSticky = () => (
-  <ComponentBox hideCode data-visual-test="table-sticky">
-    {
-      /* jsx */ `
+  <>
+    <Global
+      styles={css`
+        body {
+          .dnb-app-content {
+            /** Because of position: sticky; */
+            overflow: visible;
+          }
+
+          .dnb-tabbar {
+            /** Because the tabbar has an bottom border that will be whown on top of the side-menu */
+            overflow: hidden;
+          }
+        }
+      `}
+    />
+    <ComponentBox hideCode data-visual-test="table-sticky">
+      {
+        /* jsx */ `
 <Table sticky={true} sticky_offset="4rem" className="dnb-table--fixed">
   <caption className="dnb-sr-only">A Table Caption</caption>
   <thead>
@@ -220,6 +238,7 @@ export const TableSticky = () => (
   </tbody>
 </Table>
 `
-    }
-  </ComponentBox>
+      }
+    </ComponentBox>
+  </>
 )
diff --git a/packages/dnb-design-system-portal/src/docs/uilib/components/table/info.md b/packages/dnb-design-system-portal/src/docs/uilib/components/table/info.md
@@ -2,11 +2,6 @@
 showTabs: true
 ---
 
-import ComponentBox from 'dnb-design-system-portal/src/shared/tags/ComponentBox'
-import { css, Global } from '@emotion/react'
-
-<Global styles={typeof window !== 'undefined' && window.IS_TEST ?css`body{ .dnb-app-content { overflow: visible; } }`: css``} />
-
 # Tables
 
 The following table has a default style. But in future, there will be several extensions and styles to choose from.

diff --git a/packages/dnb-eufemia/src/components/table/TableStickyHeader.tsx b/packages/dnb-eufemia/src/components/table/TableStickyHeader.tsx
@@ -47,63 +47,50 @@ export const useStickyHeader = ({
 
       const tableElem = elementRef.current
 
-      let trElem: HTMLTableRowElement
-      let thElem: HTMLTableCellElement
-      let tdElem: HTMLTableCellElement
       let thHeight = 80
-      let tdHeight = 64
       let offsetTopPx = 0
 
       try {
-        trElem = tableElem.querySelector(
+        const trElem: HTMLTableRowElement = tableElem.querySelector(
           'thead > tr:first-of-type, thead > .dnb-table__tr:first-of-type'
         )
-        thElem = getThElement(tableElem)
-        tdElem = getTdElement(tableElem)
-
         offsetTopPx = parseFloat(String(stickyOffset || '0'))
 
         if (offsetTopPx > 0) {
           if (String(stickyOffset).includes('em')) {
             offsetTopPx = Math.round(offsetTopPx * 16)
-            trElem.style.top = String(stickyOffset)
+            trElem.style.setProperty('--table-top', String(stickyOffset))
           } else {
-            trElem.style.top = `${offsetTopPx / 16}rem`
+            trElem.style.setProperty(
+              '--table-top',
+              `${offsetTopPx / 16}rem`
+            )
           }
         }
 
+        const thElem = getThElement(tableElem)
         thHeight =
           (thElem && parseFloat(window.getComputedStyle(thElem).height)) ||
           thHeight
-        tdHeight =
-          (tdElem && parseFloat(window.getComputedStyle(tdElem).height)) ||
-          tdHeight
-      } catch (e) {
-        stickyWarning(e)
-      }
 
-      const marginTop = thHeight + tdHeight + offsetTopPx
-
-      intersectionObserver.current = new IntersectionObserver(
-        (entries) => {
-          const [entry] = entries
-          try {
-            if (entry.isIntersecting) {
-              trElem.classList.remove('show-shadow')
-            } else {
-              trElem.classList.add('show-shadow')
+        intersectionObserver.current = new IntersectionObserver(
+          (entries) => {
+            const [entry] = entries
+            try {
+              if (entry.isIntersecting) {
+                trElem.classList.remove('show-shadow')
+              } else {
+                trElem.classList.add('show-shadow')
+              }
+            } catch (e) {
+              stickyWarning(e)
             }
-          } catch (e) {
-            stickyWarning(e)
+          },
+          {
+            rootMargin: `-${thHeight + offsetTopPx}px 0px 0px 0px`,
           }
-        },
-        {
-          rootMargin: `-${marginTop}px 0px 0px 0px`,
-        }
-      )
+        )
 
-      try {
-        const trElem = tableElem.querySelector('thead > tr:first-of-type')
         trElem.classList.add('sticky')
         const tdElem =
           tableElem.querySelector(

diff --git a/packages/dnb-eufemia/src/components/table/__tests__/TableStickyHeader.test.tsx b/packages/dnb-eufemia/src/components/table/__tests__/TableStickyHeader.test.tsx
@@ -92,25 +92,28 @@ describe('useStickyHeader', () => {
     expect(getTrClasses()).toEqual(['dnb-table__tr', 'sticky'])
   })
 
-  it('should use default stickyOffset when not given', () => {
+  it('should use default header height in rootMargin when stickyOffset is not given', () => {
     render(
       <Table sticky>
         <BasicTable sticky />
       </Table>
     )
 
-    expect(screen.queryByRole('table').querySelector('tr').style.top).toBe(
-      ''
-    )
+    expect(
+      screen
+        .queryByRole('table')
+        .querySelector('tr')
+        .style.getPropertyValue('--table-top')
+    ).toBe('')
     expect(window.IntersectionObserver).toHaveBeenCalledTimes(1)
     expect(window.IntersectionObserver).toHaveBeenCalledWith(
       expect.any(Function),
-      // Formula: thHeight + tdHeight + offsetTopPx = -(sum)px
-      { rootMargin: '-144px 0px 0px 0px' }
+      // Formula: thHeight + offsetTopPx = -(sum)px
+      { rootMargin: '-80px 0px 0px 0px' } // we set a rootMargin to show/hide the shadow on a certain position ("show-shadow" class)
     )
   })
 
-  it('should support stickyOffset', () => {
+  it('should set correct rootMargin and --table-top with given stickyOffset', () => {
     const getTrElem = () => screen.queryByRole('table').querySelector('tr')
 
     const { rerender } = render(
@@ -119,27 +122,33 @@ describe('useStickyHeader', () => {
       </Table>
     )
 
-    expect(getTrElem().style.top).toEqual('4rem')
+    expect(getTrElem().style.getPropertyValue('--table-top')).toEqual(
+      '4rem'
+    )
     expect(window.IntersectionObserver).toHaveBeenCalledTimes(1)
     expect(window.IntersectionObserver).toHaveBeenNthCalledWith(
       1,
       expect.any(Function),
-      { rootMargin: '-208px 0px 0px 0px' }
+      // Formula: thHeight + offsetTopPx = -(sum)px
+      { rootMargin: '-144px 0px 0px 0px' } // we set a rootMargin to show/hide the shadow on a certain position ("show-shadow" class)
     )
 
-    // provide pixels
+    /** stickyOffset should support pixels as well */
     rerender(
       <Table sticky stickyOffset={64}>
         <BasicTable sticky />
       </Table>
     )
 
-    expect(getTrElem().style.top).toEqual('4rem')
+    expect(getTrElem().style.getPropertyValue('--table-top')).toEqual(
+      '4rem'
+    )
     expect(window.IntersectionObserver).toHaveBeenCalledTimes(2)
     expect(window.IntersectionObserver).toHaveBeenNthCalledWith(
       2,
       expect.any(Function),
-      { rootMargin: '-208px 0px 0px 0px' }
+      // Formula: thHeight + offsetTopPx = -(sum)px
+      { rootMargin: '-144px 0px 0px 0px' } // we set a rootMargin to show/hide the shadow on a certain position ("show-shadow" class)
     )
   })
 })

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
@@ -495,13 +495,11 @@ exports[`Table scss have to match snapshot 1`] = `
     font-size: var(--font-size-small);
     margin-top: var(--spacing-x-small);
     caption-side: bottom; }
-  .dnb-table tr.sticky {
-    top: 0; }
   .dnb-table tr.sticky th {
     position: sticky;
-    top: inherit;
+    top: var(--table-top, 0);
     z-index: 2; }
-    .dnb-table tr.sticky th::after {
+    .dnb-table tr.sticky th::before {
       content: '';
       position: absolute;
       opacity: 0;
@@ -512,9 +510,9 @@ exports[`Table scss have to match snapshot 1`] = `
       box-shadow: var(--shadow-default-x) -2px 12px 8px var(--shadow-default-color);
       clip-path: inset(6px 0 -48px 0);
       transition: opacity 300ms ease-out; }
-      [data-visual-test-wrapper] .dnb-table tr.sticky th::after {
+      [data-visual-test-wrapper] .dnb-table tr.sticky th::before {
         transition: none; }
-  .dnb-table tr.sticky.show-shadow th::after {
+  .dnb-table tr.sticky.show-shadow th::before {
     opacity: 1; }
   .dnb-table--small,
   .dnb-table--small > tr > th,

diff --git a/packages/dnb-eufemia/src/components/table/stories/Table.stories.tsx b/packages/dnb-eufemia/src/components/table/stories/Table.stories.tsx
@@ -19,10 +19,11 @@ export default {
 export const StickyBasicTable = () => {
   return (
     <Table
+      top="5rem"
       // skeleton // toggle
       sticky
       // className="dnb-table--fixed"
-      // stickyOffset="4rem"
+      stickyOffset="4rem"
     >
       <caption className="dnb-sr-only">A Table Caption</caption>
       <thead>
@@ -89,6 +90,18 @@ export const StickyBasicTable = () => {
           <td>Column 3</td>
           <td>Column 4</td>
         </tr>
+        <tr>
+          <td>Column 1</td>
+          <td>Column 2</td>
+          <td>Column 3</td>
+          <td>Column 4</td>
+        </tr>
+        <tr>
+          <td>Column 1</td>
+          <td>Column 2</td>
+          <td>Column 3</td>
+          <td>Column 4</td>
+        </tr>
       </tbody>
     </Table>
   )

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 @@
 *
 */
 
-
 .dnb-table {
   display: table;
   table-layout: auto;
@@ -45,16 +44,14 @@
     caption-side: bottom;
   }
 
-  tr.sticky {
-    top: 0;
-  }
   /* stylelint-disable-next-line */
+
   tr.sticky th {
     position: sticky;
-    top: inherit;
+    top: var(--table-top, 0); // is set by "stickyOffset" prop
     z-index: 2;
 
-    &::after {
+    &::before {
       content: '';
       position: absolute;
       opacity: 0;
@@ -74,7 +71,7 @@
       }
     }
   }
-  tr.sticky.show-shadow th::after {
+  tr.sticky.show-shadow th::before {
     opacity: 1;
   }