fix: Rigorizing the isStuck logic in container use-sticky-header (#3239)

cloudscape-design · Feb 6, 2025 · 6c6453d · 6c6453d
1 parent e8274c3
commit 6c6453d
Show file tree

Hide file tree

Showing 2 changed files with 122 additions and 4 deletions.
diff --git a/src/container/__tests__/sticky-header.test.tsx b/src/container/__tests__/sticky-header.test.tsx
@@ -2,6 +2,7 @@
 // SPDX-License-Identifier: Apache-2.0
 import { act, renderHook } from '../../__tests__/render-hook';
 import { useStickyHeader } from '../use-sticky-header';
+
 jest.mock('../../../lib/components/container/use-sticky-header', () => ({
   useStickyHeader: () => ({ isSticky: true }),
 }));
@@ -21,6 +22,101 @@ beforeEach(() => {
   jest.resetAllMocks();
 });
 
+test('should set isStuck to false when __stickyHeader is false', () => {
+  const rootRef = {
+    current: document.createElement('div'),
+  };
+  rootRef.current.getBoundingClientRect = jest.fn().mockReturnValue({ top: 100 });
+
+  const headerRef = {
+    current: document.createElement('div'),
+  };
+  headerRef.current.getBoundingClientRect = jest.fn().mockReturnValue({ top: -200 });
+
+  const { result } = renderHook(() => useStickyHeader(rootRef, headerRef, false, 0, 0, false));
+  act(() => {
+    window.dispatchEvent(new Event('scroll'));
+  });
+
+  expect(result.current.isStuck).toBe(false);
+});
+
+test('should set isStuck to false when __disableMobile is false', () => {
+  const rootRef = {
+    current: document.createElement('div'),
+  };
+  rootRef.current.getBoundingClientRect = jest.fn().mockReturnValue({ top: 100 });
+
+  const headerRef = {
+    current: document.createElement('div'),
+  };
+  headerRef.current.getBoundingClientRect = jest.fn().mockReturnValue({ top: -200 });
+
+  const { result } = renderHook(() => useStickyHeader(rootRef, headerRef, true, 0, 0, true));
+  act(() => {
+    window.dispatchEvent(new Event('scroll'));
+  });
+
+  expect(result.current.isStuck).toBe(false);
+});
+
+test('should set isStuck to false when rootTop headerTop are equal and headerTop is not 0', () => {
+  const rootRef = {
+    current: document.createElement('div'),
+  };
+  rootRef.current.getBoundingClientRect = jest.fn().mockReturnValue({ top: 100 });
+
+  const headerRef = {
+    current: document.createElement('div'),
+  };
+  headerRef.current.getBoundingClientRect = jest.fn().mockReturnValue({ top: 100 });
+
+  const { result } = renderHook(() => useStickyHeader(rootRef, headerRef, true, 0, 0, false));
+  act(() => {
+    window.dispatchEvent(new Event('scroll'));
+  });
+
+  expect(result.current.isStuck).toBe(false);
+});
+
+test('should set isStuck to true when rootTop less than a headerTop at 0', () => {
+  const rootRef = {
+    current: document.createElement('div'),
+  };
+  rootRef.current.getBoundingClientRect = jest.fn().mockReturnValue({ top: -100 });
+
+  const headerRef = {
+    current: document.createElement('div'),
+  };
+  headerRef.current.getBoundingClientRect = jest.fn().mockReturnValue({ top: 0 });
+
+  const { result } = renderHook(() => useStickyHeader(rootRef, headerRef, true, 0, 0, false));
+  act(() => {
+    window.dispatchEvent(new Event('scroll'));
+  });
+
+  expect(result.current.isStuck).toBe(true);
+});
+
+test('should set isStuck to false when rootTop and headerTop are equal at 0', () => {
+  const rootRef = {
+    current: document.createElement('div'),
+  };
+  rootRef.current.getBoundingClientRect = jest.fn().mockReturnValue({ top: 0 });
+
+  const headerRef = {
+    current: document.createElement('div'),
+  };
+  headerRef.current.getBoundingClientRect = jest.fn().mockReturnValue({ top: 0 });
+
+  const { result } = renderHook(() => useStickyHeader(rootRef, headerRef, true, 0, 0, false));
+  act(() => {
+    window.dispatchEvent(new Event('scroll'));
+  });
+
+  expect(result.current.isStuck).toBe(false);
+});
+
 test('should set isStuck to true when rootTop is less than headerTop', () => {
   const rootRef = {
     current: document.createElement('div'),
@@ -40,7 +136,7 @@ test('should set isStuck to true when rootTop is less than headerTop', () => {
   expect(result.current.isStuck).toBe(true);
 });
 
-test('should set isStuck to false when rootTop is larger than than headerTop', () => {
+test('should set isStuck to false when rootTop is larger than than nonZero headerTop', () => {
   const rootRef = {
     current: document.createElement('div'),
   };
@@ -59,6 +155,25 @@ test('should set isStuck to false when rootTop is larger than than headerTop', (
   expect(result.current.isStuck).toBe(false);
 });
 
+test('should set isStuck to false when rootTop is larger than than zero headerTop', () => {
+  const rootRef = {
+    current: document.createElement('div'),
+  };
+  rootRef.current.getBoundingClientRect = jest.fn().mockReturnValue({ top: 200 });
+
+  const headerRef = {
+    current: document.createElement('div'),
+  };
+  headerRef.current.getBoundingClientRect = jest.fn().mockReturnValue({ top: 0 });
+
+  const { result } = renderHook(() => useStickyHeader(rootRef, headerRef, true, 0, 0, false));
+  act(() => {
+    window.dispatchEvent(new Event('scroll'));
+  });
+
+  expect(result.current.isStuck).toBe(false);
+});
+
 test('should not set isStuck to true when rootTop has a border and is larger than than headerTop', () => {
   const rootRef = {
     current: document.createElement('div'),

diff --git a/src/container/use-sticky-header.ts b/src/container/use-sticky-header.ts
@@ -57,10 +57,12 @@ export const useStickyHeader = (
   // If it has overflow parents inside the app layout, we shouldn't apply a sticky offset.
   const [hasInnerOverflowParents, setHasInnerOverflowParents] = useState(false);
   const [isStuck, setIsStuck] = useState(false);
+
   useLayoutEffect(() => {
     if (rootRef.current) {
       const overflowParents = getOverflowParents(rootRef.current);
       const mainElement = findUpUntil(rootRef.current, elem => elem.tagName === 'MAIN');
+
       // In both versions of the app layout, the scrolling element for disableBodyScroll
       // is the <main>. If the closest overflow parent is also the closest <main> and we have
       // offset values, it's safe to assume that it's the app layout scroll root and we
@@ -95,9 +97,9 @@ export const useStickyHeader = (
       }
       if (rootRef.current && headerRef.current) {
         const rootTopBorderWidth = parseFloat(getComputedStyle(rootRef.current).borderTopWidth) || 0;
-        const rootTop = rootRef.current.getBoundingClientRect().top + rootTopBorderWidth;
-        const headerTop = headerRef.current.getBoundingClientRect().top;
-
+        // Using Math.round to adjust for rounding errors in floating-point arithmetic and timing issues
+        const rootTop = Math.round(rootRef.current.getBoundingClientRect().top + rootTopBorderWidth);
+        const headerTop = Math.round(headerRef.current.getBoundingClientRect().top);
         if (rootTop < headerTop) {
           setIsStuck(true);
         } else {
@@ -107,6 +109,7 @@ export const useStickyHeader = (
     },
     [rootRef, headerRef]
   );
+
   useEffect(() => {
     if (isSticky) {
       const controller = new AbortController();