Fix AnchorNav layout shift when sticky (#1124)

* add snapshots and unit test

* fix a test

* add changeset

Author: rezrah

.changeset/tame-boxes-shout.md

@@ -0,0 +1,7 @@
+---
+'@primer/react-brand': patch
+---
+
+Fixed page layout shift caused by the `AnchorNav` component in a sticky state.
+
+Previously the `AnchorNav` would remove its computed height from the underlying page in sticky state. Now that lost space is compensated for to create a smoother scrolling experience.

packages/react/src/AnchorNav/AnchorNav.features.stories.tsx

@@ -266,7 +266,8 @@ HideUntilSticky.storyName = 'Hide until sticky'
 export const AnchorNavPrimaryActions = () => {
   const data = ['link1', 'link2', 'link3']
   return (
-    <>
+    <div style={{backgroundColor: 'var(--brand-color-canvas-default)'}}>
+      <RedlineBackground height={75}></RedlineBackground>
       <AnchorNav>
         {data.map((link, index) => (
           <AnchorNav.Link href={link} key={index}>
@@ -302,15 +303,16 @@ export const AnchorNavPrimaryActions = () => {
           </RedlineBackground>
         ))}
       </Stack>
-    </>
+    </div>
   )
 }
 AnchorNavPrimaryActions.storyName = 'With optional primary CTA'
 
 export const AnchorNavLargerActions = () => {
   const data = ['link1', 'link2', 'link3']
   return (
-    <>
+    <div style={{backgroundColor: 'var(--brand-color-canvas-default)'}}>
+      <RedlineBackground height={75}></RedlineBackground>
       <AnchorNav>
         {data.map((link, index) => (
           <AnchorNav.Link href={link} key={index}>
@@ -349,7 +351,7 @@ export const AnchorNavLargerActions = () => {
           </RedlineBackground>
         ))}
       </Stack>
-    </>
+    </div>
   )
 }
 AnchorNavLargerActions.storyName = 'With larger CTAs'

packages/react/src/AnchorNav/AnchorNav.test.tsx

@@ -1,4 +1,4 @@
-import React, {render, cleanup, fireEvent} from '@testing-library/react'
+import React, {render, cleanup, fireEvent, act} from '@testing-library/react'
 import '@testing-library/jest-dom'
 import {axe, toHaveNoViolations} from 'jest-axe'
 
@@ -40,17 +40,99 @@ const MockAnchorNavFixture = ({data = mockData, withSecondAction = false, ...res
 }
 
 describe('AnchorNav', () => {
-  afterEach(cleanup)
+  let mockIntersectionObserver: jest.Mock
+  let addEventListenerSpy: jest.SpyInstance
+  let scrollListener: EventListener | null
+
+  const setScrollPosition = (position: number) => {
+    Object.defineProperty(window, 'pageYOffset', {
+      writable: true,
+      configurable: true,
+      value: position,
+    })
+  }
+
+  const triggerScrollEvent = async () => {
+    if (scrollListener) {
+      scrollListener(new Event('scroll'))
+    }
+  }
+
+  const simulateNavBecomingSticky = async (navYPosition: number, scrollToPosition: number) => {
+    await act(async () => {
+      triggerObserverByRootMargin('0px 0px -100%', {
+        boundingClientRect: mockRect(navYPosition),
+        isIntersecting: true,
+      })
+    })
+
+    await act(async () => {
+      setScrollPosition(scrollToPosition)
+      await triggerScrollEvent()
+    })
+  }
+
+  const mockRect = (y = 100): DOMRectReadOnly => ({
+    y,
+    top: y,
+    bottom: y + 50,
+    left: 0,
+    right: 0,
+    width: 0,
+    height: 50,
+    x: 0,
+    toJSON: () => ({}),
+  })
+
+  const triggerObserverByRootMargin = (rootMargin: string, entry: Partial<IntersectionObserverEntry>) => {
+    const observerCall = mockIntersectionObserver.mock.calls.find(call => {
+      const options = call[1]
+      return options && options.rootMargin === rootMargin
+    })
+
+    if (observerCall) {
+      const [callback] = observerCall
+      const mockEntry = {
+        isIntersecting: true,
+        boundingClientRect: mockRect(),
+        ...entry,
+      }
+      callback([mockEntry])
+    }
+  }
 
   beforeEach(() => {
-    // IntersectionObserver isn't available in test environment
-    const mockIntersectionObserver = jest.fn()
+    scrollListener = null
+
+    mockIntersectionObserver = jest.fn()
     mockIntersectionObserver.mockReturnValue({
-      observe: () => null,
-      unobserve: () => null,
-      disconnect: () => null,
+      observe: jest.fn(),
+      unobserve: jest.fn(),
+      disconnect: jest.fn(),
     })
     window.IntersectionObserver = mockIntersectionObserver
+
+    Object.defineProperty(window, 'pageYOffset', {
+      writable: true,
+      configurable: true,
+      value: 0,
+    })
+
+    addEventListenerSpy = jest
+      .spyOn(window, 'addEventListener')
+      .mockImplementation((type: string, listener: EventListenerOrEventListenerObject) => {
+        if (type === 'scroll') {
+          scrollListener = typeof listener === 'function' ? listener : listener.handleEvent
+        }
+      })
+  })
+
+  afterEach(() => {
+    cleanup()
+
+    addEventListenerSpy.mockRestore()
+
+    jest.clearAllMocks()
   })
 
   it('renders the root element correctly into the document', () => {
@@ -108,15 +190,13 @@ describe('AnchorNav', () => {
     const {getByTestId} = render(<MockAnchorNavFixture />)
     const actionEl = getByTestId(AnchorNav.testIds.action)
     expect(actionEl).toBeInTheDocument() // renders
-    expect(actionEl).toBeInTheDocument() // renders as an anchor
     expect(actionEl).toHaveAttribute('href', '#') // renders with correct href
   })
 
   it('renders an secondary correctly', () => {
     const {getByTestId} = render(<MockAnchorNavFixture withSecondAction />)
     const secondaryActionEl = getByTestId(AnchorNav.testIds.secondaryAction)
     expect(secondaryActionEl).toBeInTheDocument() // renders
-    expect(secondaryActionEl).toBeInTheDocument() // renders as an anchor
     expect(secondaryActionEl).toHaveAttribute('href', '#') // renders with correct href
   })
 
@@ -158,4 +238,32 @@ describe('AnchorNav', () => {
 
     expect(results).toHaveNoViolations()
   })
+
+  it('shows an equivalent height spacer when nav is sticky and hides it when not', async () => {
+    const MockPage = () => (
+      <div style={{height: '200vh'}}>
+        <MockAnchorNavFixture />
+        <div style={{height: '100vh'}}>After nav</div>
+      </div>
+    )
+
+    const {getByTestId, queryByTestId} = render(<MockPage />)
+    const rootEl = getByTestId(AnchorNav.testIds.root)
+    const navPosition = 100
+
+    expect(rootEl).not.toHaveClass('AnchorNav--stuck')
+    expect(queryByTestId(AnchorNav.testIds.navSpacer)).not.toBeInTheDocument()
+
+    await simulateNavBecomingSticky(navPosition, navPosition + 50)
+    expect(rootEl).toHaveClass('AnchorNav--stuck')
+    expect(getByTestId(AnchorNav.testIds.navSpacer)).toBeInTheDocument()
+
+    await act(async () => {
+      setScrollPosition(navPosition - 50)
+      await triggerScrollEvent()
+    })
+
+    expect(rootEl).not.toHaveClass('AnchorNav--stuck')
+    expect(queryByTestId(AnchorNav.testIds.navSpacer)).not.toBeInTheDocument()
+  })
 })

packages/react/src/AnchorNav/AnchorNav.tsx

@@ -32,6 +32,9 @@ const testIds = {
   get secondaryAction() {
     return `${this.root}-secondary-action`
   },
+  get navSpacer() {
+    return `${this.root}-nav-spacer`
+  },
 }
 
 export type AnchorNavProps = BaseProps<HTMLElement> & {
@@ -56,6 +59,7 @@ function _AnchorNav({children, enableDefaultBgColor = false, hideUntilSticky = f
   const [intersectionEntry, setIntersectionEntry] = useState<IntersectionObserverEntry>()
   const [initialYOffset, setInitialYOffset] = useState<undefined | number>()
   const [navShouldFix, setNavShouldFix] = useState<boolean>(false)
+  const [navHeight, setNavHeight] = useState<number>(0)
 
   const wrapperRef = useRef<HTMLDivElement | null>(null)
   const rootRef = useRef<HTMLElement | null>(null)
@@ -88,6 +92,13 @@ function _AnchorNav({children, enableDefaultBgColor = false, hideUntilSticky = f
   useKeyboardEscape(closeMenuCallback)
   useExpandedMenu(menuOpen, linkContainerRef, !isLarge)
 
+  useEffect(() => {
+    if (wrapperRef.current) {
+      const height = wrapperRef.current.offsetHeight
+      setNavHeight(height)
+    }
+  }, [isLarge])
+
   useEffect(() => {
     const queryResult = window.matchMedia('(prefers-reduced-motion: reduce)')
 
@@ -271,6 +282,7 @@ function _AnchorNav({children, enableDefaultBgColor = false, hideUntilSticky = f
           aria-hidden
         />
       </nav>
+      {navShouldFix && <div style={{height: navHeight}} aria-hidden="true" data-testid={testIds.navSpacer} />}
     </div>
   )
 }