From 5f299af532288026aedb899d5d0d5a9345e1ee92 Mon Sep 17 00:00:00 2001 From: "oleksandr.buhaienko" Date: Tue, 20 May 2025 12:06:17 +0300 Subject: [PATCH 1/3] fix: fixed Tabs menu is not displaying the "more" button --- .../tabs/useIndexOfLastVisibleChild.js | 104 +++++++++--------- 1 file changed, 52 insertions(+), 52 deletions(-) diff --git a/src/components/NavigationBar/tabs/useIndexOfLastVisibleChild.js b/src/components/NavigationBar/tabs/useIndexOfLastVisibleChild.js index fee5c8db9..b94a032f2 100644 --- a/src/components/NavigationBar/tabs/useIndexOfLastVisibleChild.js +++ b/src/components/NavigationBar/tabs/useIndexOfLastVisibleChild.js @@ -1,7 +1,5 @@ import { useLayoutEffect, useRef, useState } from 'react'; -import { useWindowSize } from '@openedx/paragon'; - const invisibleStyle = { position: 'absolute', left: 0, @@ -10,68 +8,70 @@ const invisibleStyle = { }; /** - * This hook will find the index of the last child of a containing element - * that fits within its bounding rectangle. This is done by summing the widths - * of the children until they exceed the width of the container. + * This hook calculates the index of the last child that can fit into the + * container element without overflowing. All children are rendered, but those + * that exceed the available width are styled with `invisibleStyle` to hide them + * visually while preserving their dimensions for measurement. * - * The hook returns an array containing: - * [indexOfLastVisibleChild, containerElementRef, invisibleStyle, overflowElementRef] + * It uses ResizeObserver to automatically react to any changes in container + * size or child widths — without requiring a window resize event. * - * indexOfLastVisibleChild - the index of the last visible child - * containerElementRef - a ref to be added to the containing html node - * invisibleStyle - a set of styles to be applied to child of the containing node - * if it needs to be hidden. These styles remove the element visually, from - * screen readers, and from normal layout flow. But, importantly, these styles - * preserve the width of the element, so that future width calculations will - * still be accurate. - * overflowElementRef - a ref to be added to an html node inside the container - * that is likely to be used to contain a "More" type dropdown or other - * mechanism to reveal hidden children. The width of this element is always - * included when determining which children will fit or not. Usage of this ref - * is optional. + * Returns: + * [ + * indexOfLastVisibleChild, // Index of the last tab that fits in the container + * containerElementRef, // Ref to attach to the tabs container + * invisibleStyle, // Style object to apply to "hidden" tabs + * overflowElementRef // Ref to the overflow ("More...") element + * ] */ export default function useIndexOfLastVisibleChild() { const containerElementRef = useRef(null); const overflowElementRef = useRef(null); - const containingRectRef = useRef({}); const [indexOfLastVisibleChild, setIndexOfLastVisibleChild] = useState(-1); - const windowSize = useWindowSize(); - useLayoutEffect(() => { - const containingRect = containerElementRef.current.getBoundingClientRect(); + // Measures how many tab elements fit within the container's width + const measureVisibleChildren = () => { + const container = containerElementRef.current; + const overflow = overflowElementRef.current; + if (!container) { return; } + + const containingRect = container.getBoundingClientRect(); + + // Get all children excluding the overflow element + const children = Array.from(container.children).filter(child => child !== overflow); + + let sumWidth = overflow ? overflow.getBoundingClientRect().width : 0; + let lastVisibleIndex = -1; - // No-op if the width is unchanged. - // (Assumes tabs themselves don't change count or width). - if (!containingRect.width === containingRectRef.current.width) { - return; + for (let i = 0; i < children.length; i++) { + const width = Math.floor(children[i].getBoundingClientRect().width); + sumWidth += width; + + if (sumWidth <= containingRect.width) { + lastVisibleIndex = i; + } else { + break; + } } - // Update for future comparison - containingRectRef.current = containingRect; - // Get array of child nodes from NodeList form - const childNodesArr = Array.prototype.slice.call(containerElementRef.current.children); - const { nextIndexOfLastVisibleChild } = childNodesArr - // filter out the overflow element - .filter(childNode => childNode !== overflowElementRef.current) - // sum the widths to find the last visible element's index - .reduce((acc, childNode, index) => { - // use floor to prevent rounding errors - acc.sumWidth += Math.floor(childNode.getBoundingClientRect().width); - if (acc.sumWidth <= containingRect.width) { - acc.nextIndexOfLastVisibleChild = index; - } - return acc; - }, { - // Include the overflow element's width to begin with. Doing this means - // sometimes we'll show a dropdown with one item in it when it would fit, - // but allowing this case dramatically simplifies the calculations we need - // to do above. - sumWidth: overflowElementRef.current ? overflowElementRef.current.getBoundingClientRect().width : 0, - nextIndexOfLastVisibleChild: -1, - }); + setIndexOfLastVisibleChild(lastVisibleIndex); + }; + + useLayoutEffect(() => { + const container = containerElementRef.current; + if (!container) { return undefined; } + + // ResizeObserver tracks size changes of the container or its children + const resizeObserver = new ResizeObserver(() => { + measureVisibleChildren(); + }); + + resizeObserver.observe(container); + // Run once on mount to ensure accurate measurement from the start + measureVisibleChildren(); - setIndexOfLastVisibleChild(nextIndexOfLastVisibleChild); - }, [windowSize, containerElementRef.current]); + return () => resizeObserver.disconnect(); + }, []); return [indexOfLastVisibleChild, containerElementRef, invisibleStyle, overflowElementRef]; } From d6f2699d48e6a99be34276681fa7de4d293959c9 Mon Sep 17 00:00:00 2001 From: "oleksandr.buhaienko" Date: Tue, 8 Jul 2025 11:02:28 +0300 Subject: [PATCH 2/3] test: add tests --- .../tabs/useIndexOfLastVisibleChild.js | 11 ++- .../tabs/useIndexOfLastVisibleChild.test.jsx | 80 +++++++++++++++++++ 2 files changed, 88 insertions(+), 3 deletions(-) create mode 100644 src/components/NavigationBar/tabs/useIndexOfLastVisibleChild.test.jsx diff --git a/src/components/NavigationBar/tabs/useIndexOfLastVisibleChild.js b/src/components/NavigationBar/tabs/useIndexOfLastVisibleChild.js index b94a032f2..12cbf0fb9 100644 --- a/src/components/NavigationBar/tabs/useIndexOfLastVisibleChild.js +++ b/src/components/NavigationBar/tabs/useIndexOfLastVisibleChild.js @@ -49,7 +49,7 @@ export default function useIndexOfLastVisibleChild() { if (sumWidth <= containingRect.width) { lastVisibleIndex = i; - } else { + } /* istanbul ignore else */ else { break; } } @@ -59,7 +59,9 @@ export default function useIndexOfLastVisibleChild() { useLayoutEffect(() => { const container = containerElementRef.current; - if (!container) { return undefined; } + if (!container) { + return () => {}; + } // ResizeObserver tracks size changes of the container or its children const resizeObserver = new ResizeObserver(() => { @@ -70,7 +72,10 @@ export default function useIndexOfLastVisibleChild() { // Run once on mount to ensure accurate measurement from the start measureVisibleChildren(); - return () => resizeObserver.disconnect(); + /* istanbul ignore next */ + return () => { + resizeObserver.disconnect(); + }; }, []); return [indexOfLastVisibleChild, containerElementRef, invisibleStyle, overflowElementRef]; diff --git a/src/components/NavigationBar/tabs/useIndexOfLastVisibleChild.test.jsx b/src/components/NavigationBar/tabs/useIndexOfLastVisibleChild.test.jsx new file mode 100644 index 000000000..d1b7fff0f --- /dev/null +++ b/src/components/NavigationBar/tabs/useIndexOfLastVisibleChild.test.jsx @@ -0,0 +1,80 @@ +import React from 'react'; + +import { render, renderHook } from '@testing-library/react'; + +import useIndexOfLastVisibleChild from './useIndexOfLastVisibleChild'; + +describe('useIndexOfLastVisibleChild', () => { + let observeMock; + let disconnectMock; + + beforeAll(() => { + observeMock = jest.fn(); + disconnectMock = jest.fn(); + global.ResizeObserver = class { + observe = observeMock; + + disconnect = disconnectMock; + }; + }); + + afterAll(() => { + delete global.ResizeObserver; + }); + + beforeEach(() => { + observeMock.mockReset(); + disconnectMock.mockReset(); + }); + + it('calls disconnect on cleanup when container exists', () => { + const TestComponent = () => { + const [, containerRef] = useIndexOfLastVisibleChild(); + return
; + }; + const { unmount } = render(); + unmount(); + expect(disconnectMock).toHaveBeenCalled(); + }); + + it('handles missing container gracefully (no observer or disconnect)', () => { + const { unmount } = renderHook(() => useIndexOfLastVisibleChild()); + unmount(); + expect(disconnectMock).not.toHaveBeenCalled(); + }); + + it('returns -1 if there are no children', () => { + const TestComponent = () => { + const [, containerRef] = useIndexOfLastVisibleChild(); + React.useEffect(() => { + const container = document.createElement('div'); + container.getBoundingClientRect = () => ({ width: 100 }); + containerRef.current = container; + }, []); + return
; + }; + const { unmount } = render(); + unmount(); + }); + + it('triggers break when child tabs exceed container width', () => { + const TestComponent = () => { + const [, containerRef] = useIndexOfLastVisibleChild(); + React.useEffect(() => { + const container = document.createElement('div'); + container.getBoundingClientRect = () => ({ width: 100 }); + const child1 = document.createElement('div'); + child1.getBoundingClientRect = () => ({ width: 80 }); + const child2 = document.createElement('div'); + child2.getBoundingClientRect = () => ({ width: 80 }); + container.appendChild(child1); + container.appendChild(child2); + + containerRef.current = container; + }, []); + return
; + }; + const { unmount } = render(); + unmount(); + }); +}); From 16d24a3bf0fcc6a0d6c166d8c76702f827792747 Mon Sep 17 00:00:00 2001 From: PKulkoRaccoonGang Date: Fri, 11 Jul 2025 14:30:52 +0300 Subject: [PATCH 3/3] test: added tests --- .../tabs/useIndexOfLastVisibleChild.js | 2 +- .../tabs/useIndexOfLastVisibleChild.test.jsx | 184 +++++++++++++++++- 2 files changed, 184 insertions(+), 2 deletions(-) diff --git a/src/components/NavigationBar/tabs/useIndexOfLastVisibleChild.js b/src/components/NavigationBar/tabs/useIndexOfLastVisibleChild.js index 12cbf0fb9..9f115af29 100644 --- a/src/components/NavigationBar/tabs/useIndexOfLastVisibleChild.js +++ b/src/components/NavigationBar/tabs/useIndexOfLastVisibleChild.js @@ -49,7 +49,7 @@ export default function useIndexOfLastVisibleChild() { if (sumWidth <= containingRect.width) { lastVisibleIndex = i; - } /* istanbul ignore else */ else { + } else { break; } } diff --git a/src/components/NavigationBar/tabs/useIndexOfLastVisibleChild.test.jsx b/src/components/NavigationBar/tabs/useIndexOfLastVisibleChild.test.jsx index d1b7fff0f..d8dfe5daf 100644 --- a/src/components/NavigationBar/tabs/useIndexOfLastVisibleChild.test.jsx +++ b/src/components/NavigationBar/tabs/useIndexOfLastVisibleChild.test.jsx @@ -1,6 +1,6 @@ import React from 'react'; -import { render, renderHook } from '@testing-library/react'; +import { act, render, renderHook } from '@testing-library/react'; import useIndexOfLastVisibleChild from './useIndexOfLastVisibleChild'; @@ -77,4 +77,186 @@ describe('useIndexOfLastVisibleChild', () => { const { unmount } = render(); unmount(); }); + + it('calls measureVisibleChildren on mount and when ResizeObserver triggers', () => { + const TestComponent = () => { + const [, containerRef] = useIndexOfLastVisibleChild(); + React.useEffect(() => { + const container = document.createElement('div'); + container.getBoundingClientRect = () => ({ width: 200 }); + containerRef.current = container; + }, []); + return
; + }; + + const { unmount } = render(); + + expect(observeMock).toHaveBeenCalled(); + + act(() => { + const resizeObserverCallback = observeMock.mock.calls[0][0]; + if (resizeObserverCallback && typeof resizeObserverCallback === 'function') { + resizeObserverCallback(); + } + }); + + unmount(); + }); + + it('calculates correct last visible index when children fit within container', () => { + let resizeObserverCallback; + + global.ResizeObserver = function (callback) { + resizeObserverCallback = callback; + this.observe = jest.fn(); + this.disconnect = jest.fn(); + }; + + const TestComponent = () => { + const [lastVisibleIndex, containerRef, , overflowRef] = useIndexOfLastVisibleChild(); + + React.useEffect(() => { + const container = document.createElement('div'); + container.getBoundingClientRect = () => ({ width: 200 }); + + const child1 = document.createElement('div'); + child1.getBoundingClientRect = () => ({ width: 50 }); + const child2 = document.createElement('div'); + child2.getBoundingClientRect = () => ({ width: 60 }); + const child3 = document.createElement('div'); + child3.getBoundingClientRect = () => ({ width: 70 }); + + container.appendChild(child1); + container.appendChild(child2); + container.appendChild(child3); + + containerRef.current = container; + overflowRef.current = null; + }, []); + + return ( +
+
+
{lastVisibleIndex}
+
+ ); + }; + + const { getByTestId, unmount } = render(); + + act(() => { + if (resizeObserverCallback) { + resizeObserverCallback(); + } + }); + + // The last visible index should be 2 (all three children fit: 50 + 60 + 70 = 180 <= 200) + expect(getByTestId('last-visible-index').textContent).toBe('2'); + + unmount(); + }); + + it('handles overflow element in width calculation', () => { + let resizeObserverCallback; + + global.ResizeObserver = function (callback) { + resizeObserverCallback = callback; + this.observe = jest.fn(); + this.disconnect = jest.fn(); + }; + + const TestComponent = () => { + const [lastVisibleIndex, containerRef, , overflowRef] = useIndexOfLastVisibleChild(); + + React.useEffect(() => { + const container = document.createElement('div'); + container.getBoundingClientRect = () => ({ width: 150 }); + + const overflow = document.createElement('div'); + overflow.getBoundingClientRect = () => ({ width: 30 }); + + const child1 = document.createElement('div'); + child1.getBoundingClientRect = () => ({ width: 50 }); + const child2 = document.createElement('div'); + child2.getBoundingClientRect = () => ({ width: 60 }); + + container.appendChild(child1); + container.appendChild(child2); + container.appendChild(overflow); + + containerRef.current = container; + overflowRef.current = overflow; + }, []); + + return ( +
+
+
{lastVisibleIndex}
+
+ ); + }; + + const { getByTestId, unmount } = render(); + + act(() => { + if (resizeObserverCallback) { + resizeObserverCallback(); + } + }); + + // With overflow width (30) + child1 (50) + child2 (60) = 140 <= 150 + // So last visible index should be 1 (child2) + expect(getByTestId('last-visible-index').textContent).toBe('1'); + + unmount(); + }); + + it('returns -1 when no children fit within container width', () => { + let resizeObserverCallback; + + global.ResizeObserver = function (callback) { + resizeObserverCallback = callback; + this.observe = jest.fn(); + this.disconnect = jest.fn(); + }; + + const TestComponent = () => { + const [lastVisibleIndex, containerRef] = useIndexOfLastVisibleChild(); + + React.useEffect(() => { + const container = document.createElement('div'); + container.getBoundingClientRect = () => ({ width: 50 }); + + const child1 = document.createElement('div'); + child1.getBoundingClientRect = () => ({ width: 100 }); + const child2 = document.createElement('div'); + child2.getBoundingClientRect = () => ({ width: 80 }); + + container.appendChild(child1); + container.appendChild(child2); + + containerRef.current = container; + }, []); + + return ( +
+
+
{lastVisibleIndex}
+
+ ); + }; + + const { getByTestId, unmount } = render(); + + act(() => { + if (resizeObserverCallback) { + resizeObserverCallback(); + } + }); + + // No children fit within 50px width, so should return -1 + expect(getByTestId('last-visible-index').textContent).toBe('-1'); + + unmount(); + }); });