From a2a55ec4c6adf0a6b29077cc9fe057d3b03b4257 Mon Sep 17 00:00:00 2001 From: Grant Fitzsimmons <37256050+grantfitzsimmons@users.noreply.github.com> Date: Fri, 2 Oct 2026 11:25:02 -0500 Subject: [PATCH] fix(data-views): smarter width handling --- .../__tests__/useResponsiveSplitView.test.tsx | 91 +++++++++++++++++++ .../js_src/lib/components/DataViews/index.tsx | 55 ++++++----- .../lib/components/QueryBuilder/Header.tsx | 12 +-- .../QueryBuilder/QueryBuilderResults.tsx | 8 +- .../QueryBuilder/ResultsWrapper.tsx | 26 ++++-- .../lib/components/QueryBuilder/SplitView.tsx | 21 ++++- .../lib/components/QueryBuilder/Wrapped.tsx | 8 +- .../QueryBuilder/__tests__/SplitView.test.tsx | 49 ++++++++++ .../QueryBuilder/useQuerySplitView.ts | 36 ++++---- .../lib/hooks/useResponsiveSplitView.ts | 46 ++++++++++ 10 files changed, 281 insertions(+), 71 deletions(-) create mode 100644 specifyweb/frontend/js_src/lib/components/DataViews/__tests__/useResponsiveSplitView.test.tsx create mode 100644 specifyweb/frontend/js_src/lib/components/QueryBuilder/__tests__/SplitView.test.tsx create mode 100644 specifyweb/frontend/js_src/lib/hooks/useResponsiveSplitView.ts diff --git a/specifyweb/frontend/js_src/lib/components/DataViews/__tests__/useResponsiveSplitView.test.tsx b/specifyweb/frontend/js_src/lib/components/DataViews/__tests__/useResponsiveSplitView.test.tsx new file mode 100644 index 00000000000..ba026c5fc65 --- /dev/null +++ b/specifyweb/frontend/js_src/lib/components/DataViews/__tests__/useResponsiveSplitView.test.tsx @@ -0,0 +1,91 @@ +import { act, render, screen } from '@testing-library/react'; +import React from 'react'; + +import { + minimumHorizontalSplitWidth, + minimumSplitPaneWidth, + splitViewHandleWidth, + useResponsiveSplitView, +} from '../../../hooks/useResponsiveSplitView'; + +const horizontalAttribute = 'data-horizontal'; +const falseValue = 'false'; +const splitContainerTestId = 'split-container'; + +function TestSplit({ + preferredHorizontal, + showContainer = true, +}: { + readonly preferredHorizontal: boolean; + readonly showContainer?: boolean; +}): JSX.Element { + const { + canUseHorizontalSplit, + containerRef, + isHorizontal, + maximumPrimaryPaneWidth, + } = useResponsiveSplitView(preferredHorizontal); + return showContainer ? ( +
+ ) : ( + + ); +} + +function resizeContainer(width: number): void { + const container = screen.getByTestId(splitContainerTestId); + Object.defineProperty(container, 'clientWidth', { + configurable: true, + value: width, + }); + act((): void => { + window.dispatchEvent(new Event('resize')); + }); +} + +test('uses the container width to switch split orientation while resizing', () => { + render(); + const container = screen.getByTestId(splitContainerTestId); + + resizeContainer(minimumHorizontalSplitWidth - 1); + expect(container).toHaveAttribute('data-can-use-horizontal', falseValue); + expect(container).toHaveAttribute(horizontalAttribute, falseValue); + + resizeContainer(minimumHorizontalSplitWidth); + expect(container).toHaveAttribute('data-can-use-horizontal', 'true'); + expect(container).toHaveAttribute(horizontalAttribute, 'true'); + expect(container).toHaveAttribute( + 'data-primary-pane-max-width', + String(minimumSplitPaneWidth + splitViewHandleWidth) + ); +}); + +test('keeps the preferred vertical orientation when there is enough width', () => { + const { rerender } = render(); + const container = screen.getByTestId(splitContainerTestId); + resizeContainer(minimumHorizontalSplitWidth); + + rerender(); + expect(container).toHaveAttribute(horizontalAttribute, falseValue); +}); + +test('remeasures a replacement container after the query editor closes', () => { + const { rerender } = render(); + const originalContainer = screen.getByTestId(splitContainerTestId); + resizeContainer(minimumHorizontalSplitWidth - 1); + expect(originalContainer).toHaveAttribute(horizontalAttribute, falseValue); + + rerender(); + rerender(); + + const replacementContainer = screen.getByTestId(splitContainerTestId); + expect(replacementContainer).not.toBe(originalContainer); + resizeContainer(minimumHorizontalSplitWidth); + expect(replacementContainer).toHaveAttribute(horizontalAttribute, 'true'); +}); diff --git a/specifyweb/frontend/js_src/lib/components/DataViews/index.tsx b/specifyweb/frontend/js_src/lib/components/DataViews/index.tsx index 14caa258c3b..6c97a8deac6 100644 --- a/specifyweb/frontend/js_src/lib/components/DataViews/index.tsx +++ b/specifyweb/frontend/js_src/lib/components/DataViews/index.tsx @@ -3,20 +3,25 @@ import { useParams } from 'react-router-dom'; import { commonText } from '../../localization/common'; import { dataViewsText } from '../../localization/dataViews'; +import { useResponsiveSplitView } from '../../hooks/useResponsiveSplitView'; +import { H2 } from '../Atoms'; import { Button } from '../Atoms/Button'; import { DataEntry } from '../Atoms/DataEntry'; -import { H2 } from '../Atoms'; -import type { Tables } from '../DataModel/types'; import { getTable } from '../DataModel/tables'; +import type { Tables } from '../DataModel/types'; +import { raise } from '../Errors/Crash'; +import { RecordSelectorFromIds } from '../FormSliders/RecordSelectorFromIds'; +import { Dialog } from '../Molecules/Dialog'; +import { TableIcon } from '../Molecules/TableIcon'; +import { hasPermission } from '../Permissions/helpers'; import { PermissionDenied, ProtectedTable, } from '../Permissions/PermissionDenied'; -import { hasPermission } from '../Permissions/helpers'; -import { RecordSelectorFromIds } from '../FormSliders/RecordSelectorFromIds'; -import { QueryResultsWrapper } from '../QueryBuilder/ResultsWrapper'; +import { userPreferences } from '../Preferences/userPreferences'; import { parseQueryFields, unParseQueryFields } from '../QueryBuilder/helpers'; import { queryIdField } from '../QueryBuilder/Results'; +import { QueryResultsWrapper } from '../QueryBuilder/ResultsWrapper'; import { SplitView, SplitViewOrientationButton, @@ -24,8 +29,7 @@ import { useSplitViewOrientation, } from '../QueryBuilder/SplitView'; import { NotFoundView } from '../Router/NotFoundView'; -import { Dialog } from '../Molecules/Dialog'; -import { raise } from '../Errors/Crash'; +import type { DataViewQueriesFile } from './queries'; import { getDataViewQueryDefinition, makeDataViewQuery, @@ -33,13 +37,7 @@ import { serializeDataViewQueries, useDataViewQueries, } from './queries'; -import type { DataViewQueriesFile } from './queries'; import { DataViewQueryEditorContent } from './QueryEditor'; -import { TableIcon } from '../Molecules/TableIcon'; -import { userPreferences } from '../Preferences/userPreferences'; -import { listen } from '../../utils/events'; - -const SMALL_SCREEN_WIDTH = 768; export function TableDataView(): JSX.Element { const { tableName = '' } = useParams(); @@ -122,19 +120,15 @@ function LoadedDataViewFromTable({ 'splitViewOrientation' ); const [rawIsSplit, setIsSplit] = React.useState(splitViewByDefault); - const [canSplit, setCanSplit] = React.useState( - window.innerWidth >= SMALL_SCREEN_WIDTH - ); - React.useEffect(() => { - const handleResize = (): void => - setCanSplit(window.innerWidth >= SMALL_SCREEN_WIDTH); - handleResize(); - return listen(window, 'resize', handleResize); - }, []); - const isSplit = rawIsSplit && canSplit; - const { isHorizontal, toggleOrientation } = useSplitViewOrientation( - splitViewOrientation === 'horizontal' - ); + const isSplit = rawIsSplit; + const { isHorizontal: preferredIsHorizontal, toggleOrientation } = + useSplitViewOrientation(splitViewOrientation === 'horizontal'); + const { + canUseHorizontalSplit, + containerRef: splitViewRef, + isHorizontal, + maximumPrimaryPaneWidth, + } = useResponsiveSplitView(preferredIsHorizontal); const [refreshToken, setRefreshToken] = React.useState(0); const [queryRunCount, setQueryRunCount] = React.useState(1); const [queryData, setQueryData] = React.useState(); @@ -359,23 +353,26 @@ function LoadedDataViewFromTable({
setIsSplit((split) => !split)} /> -
+
diff --git a/specifyweb/frontend/js_src/lib/components/QueryBuilder/Header.tsx b/specifyweb/frontend/js_src/lib/components/QueryBuilder/Header.tsx index 34d9609c5a2..4af326a6e43 100644 --- a/specifyweb/frontend/js_src/lib/components/QueryBuilder/Header.tsx +++ b/specifyweb/frontend/js_src/lib/components/QueryBuilder/Header.tsx @@ -47,7 +47,7 @@ export function QueryHeader({ onTriedToSave: handleTriedToSave, onSaved: handleSaved, isSplit, - canSplit, + canUseHorizontalSplit, isHorizontal, onToggleSplit, onToggleOrientation, @@ -67,7 +67,7 @@ export function QueryHeader({ readonly onTriedToSave: () => void; readonly onSaved: () => void; readonly isSplit: boolean; - readonly canSplit: boolean; + readonly canUseHorizontalSplit: boolean; readonly isHorizontal: boolean; readonly onToggleSplit: () => void; readonly onToggleOrientation: () => void; @@ -135,13 +135,9 @@ export function QueryHeader({ ) : undefined} {hasPermission('/querybuilder/query', 'execute') && ( <> - + diff --git a/specifyweb/frontend/js_src/lib/components/QueryBuilder/QueryBuilderResults.tsx b/specifyweb/frontend/js_src/lib/components/QueryBuilder/QueryBuilderResults.tsx index 315c80185a7..671786dd0bf 100644 --- a/specifyweb/frontend/js_src/lib/components/QueryBuilder/QueryBuilderResults.tsx +++ b/specifyweb/frontend/js_src/lib/components/QueryBuilder/QueryBuilderResults.tsx @@ -1,7 +1,7 @@ import React from 'react'; import { commonText } from '../../localization/common'; -import { localized, type RA } from '../../utils/types'; +import { type RA, localized } from '../../utils/types'; import { BatchEditFromQuery } from '../BatchEdit'; import type { SerializedResource } from '../DataModel/helperTypes'; import type { SpecifyResource } from '../DataModel/legacyTypes'; @@ -34,6 +34,8 @@ export function QueryBuilderResults({ resultsRef, isSplit, isHorizontal, + maximumPrimaryPaneWidth, + splitViewRef, onReRun: handleReRun, onResults: handleResults, onSelected: handleSelected, @@ -61,6 +63,8 @@ export function QueryBuilderResults({ >; readonly isSplit: boolean; readonly isHorizontal: boolean; + readonly maximumPrimaryPaneWidth: number; + readonly splitViewRef: React.RefCallback; readonly onReRun: () => void; readonly onResults?: (results: RA) => void; readonly onSelected: (ids: RA) => void; @@ -149,8 +153,10 @@ export function QueryBuilderResults({ resultsRef={resultsRef} selectedRows={[selectedRows, setSelectedRows]} isSplit={isSplit} + splitContainerRef={splitViewRef} splitHorizontal={isHorizontal} splitPane={recordPreview} + splitPrimaryPaneMaxWidth={`${maximumPrimaryPaneWidth}px`} table={table} onReRun={handleReRun} onResults={handleResults} diff --git a/specifyweb/frontend/js_src/lib/components/QueryBuilder/ResultsWrapper.tsx b/specifyweb/frontend/js_src/lib/components/QueryBuilder/ResultsWrapper.tsx index d235b929fdf..07dbd3ea1d3 100644 --- a/specifyweb/frontend/js_src/lib/components/QueryBuilder/ResultsWrapper.tsx +++ b/specifyweb/frontend/js_src/lib/components/QueryBuilder/ResultsWrapper.tsx @@ -37,7 +37,9 @@ export function QueryResultsWrapper({ onReRun: handleReRun, refreshToken, splitPane, + splitContainerRef, splitHorizontal, + splitPrimaryPaneMaxWidth, isSplit, ...props }: ResultsProps & { @@ -49,7 +51,9 @@ export function QueryResultsWrapper({ readonly restoreScrollTopRef?: React.MutableRefObject; readonly refreshToken?: number; readonly splitPane?: JSX.Element; + readonly splitContainerRef?: React.RefCallback; readonly splitHorizontal?: boolean; + readonly splitPrimaryPaneMaxWidth?: string; readonly isSplit?: boolean; readonly onReRun: () => void; }): JSX.Element | null { @@ -79,14 +83,20 @@ export function QueryResultsWrapper({ return splitPane === undefined ? ( queryResults ) : ( - +
+ +
); } diff --git a/specifyweb/frontend/js_src/lib/components/QueryBuilder/SplitView.tsx b/specifyweb/frontend/js_src/lib/components/QueryBuilder/SplitView.tsx index 3a2e0890cd9..8f3f7a2784e 100644 --- a/specifyweb/frontend/js_src/lib/components/QueryBuilder/SplitView.tsx +++ b/specifyweb/frontend/js_src/lib/components/QueryBuilder/SplitView.tsx @@ -1,9 +1,9 @@ -import React from 'react'; import Splitter from 'm-react-splitters'; +import React from 'react'; -import { Button } from '../Atoms/Button'; -import { treeText } from '../../localization/tree'; import { useTriggerState } from '../../hooks/useTriggerState'; +import { treeText } from '../../localization/tree'; +import { Button } from '../Atoms/Button'; export function useSplitViewOrientation(defaultHorizontal = true): { readonly isHorizontal: boolean; @@ -60,6 +60,7 @@ export function SplitView({ primaryPane, secondaryPane, primaryPaneKey, + primaryPaneMaxWidth, secondaryPaneKey, isHorizontal, isSplit = true, @@ -67,10 +68,21 @@ export function SplitView({ readonly primaryPane: JSX.Element; readonly secondaryPane: JSX.Element; readonly primaryPaneKey: string; + readonly primaryPaneMaxWidth?: string; readonly secondaryPaneKey: string; readonly isHorizontal: boolean; readonly isSplit?: boolean; }): JSX.Element { + const splitterRef = React.useRef | null>( + null + ); + const previousIsHorizontal = React.useRef(isHorizontal); + React.useLayoutEffect(() => { + if (previousIsHorizontal.current !== isHorizontal) + splitterRef.current?.setState({ primaryPane: undefined }); + previousIsHorizontal.current = isHorizontal; + }, [isHorizontal]); + return (
dispatch({ type: 'RunQueryAction' })} diff --git a/specifyweb/frontend/js_src/lib/components/QueryBuilder/__tests__/SplitView.test.tsx b/specifyweb/frontend/js_src/lib/components/QueryBuilder/__tests__/SplitView.test.tsx new file mode 100644 index 00000000000..f3a54609a66 --- /dev/null +++ b/specifyweb/frontend/js_src/lib/components/QueryBuilder/__tests__/SplitView.test.tsx @@ -0,0 +1,49 @@ +import { render } from '@testing-library/react'; +import React from 'react'; + +import { SplitView } from '../SplitView'; + +const mockSetState = jest.fn(); + +jest.mock('m-react-splitters', () => { + const actualReact = jest.requireActual('react'); + const MockSplitter = actualReact.forwardRef(function MockSplitter( + { children }: { readonly children?: React.ReactNode }, + ref: React.ForwardedRef<{ readonly setState: typeof mockSetState }> + ): JSX.Element { + actualReact.useImperativeHandle( + ref, + () => ({ setState: mockSetState }), + [] + ); + return
{children}
; + }); + return { __esModule: true, default: MockSplitter }; +}); + +const primaryPane =
; +const secondaryPane =
; + +test('clears the dragged pane size when the orientation changes', () => { + const { rerender } = render( + + ); + expect(mockSetState).not.toHaveBeenCalled(); + + rerender( + + ); + expect(mockSetState).toHaveBeenCalledWith({ primaryPane: undefined }); +}); diff --git a/specifyweb/frontend/js_src/lib/components/QueryBuilder/useQuerySplitView.ts b/specifyweb/frontend/js_src/lib/components/QueryBuilder/useQuerySplitView.ts index 1f4b055a45f..93771e3a6b7 100644 --- a/specifyweb/frontend/js_src/lib/components/QueryBuilder/useQuerySplitView.ts +++ b/specifyweb/frontend/js_src/lib/components/QueryBuilder/useQuerySplitView.ts @@ -1,12 +1,10 @@ import React from 'react'; +import { useResponsiveSplitView } from '../../hooks/useResponsiveSplitView'; import type { RA } from '../../utils/types'; -import { listen } from '../../utils/events'; +import { userPreferences } from '../Preferences/userPreferences'; import { queryIdField, type QueryResultRow } from './Results'; import { useSplitViewOrientation } from './SplitView'; -import { userPreferences } from '../Preferences/userPreferences'; - -const SMALL_SCREEN_WIDTH = 768; export function useQuerySplitView( resultsRef: React.MutableRefObject< @@ -21,7 +19,9 @@ export function useQuerySplitView( readonly selectedIndex: number; readonly setSelectedIndex: React.Dispatch>; readonly isSplit: boolean; - readonly canSplit: boolean; + readonly canUseHorizontalSplit: boolean; + readonly splitViewRef: React.RefCallback; + readonly maximumPrimaryPaneWidth: number; readonly isHorizontal: boolean; readonly toggleSplit: () => void; readonly toggleOrientation: () => void; @@ -42,19 +42,15 @@ export function useQuerySplitView( 'splitViewOrientation' ); const [rawIsSplit, setIsSplit] = React.useState(splitViewByDefault); - const [canSplit, setCanSplit] = React.useState( - window.innerWidth >= SMALL_SCREEN_WIDTH - ); - React.useEffect(() => { - const handleResize = (): void => - setCanSplit(window.innerWidth >= SMALL_SCREEN_WIDTH); - handleResize(); - return listen(window, 'resize', handleResize); - }, []); - const isSplit = rawIsSplit && canSplit; - const { isHorizontal, toggleOrientation } = useSplitViewOrientation( - splitViewOrientation === 'horizontal' - ); + const isSplit = rawIsSplit; + const { isHorizontal: preferredIsHorizontal, toggleOrientation } = + useSplitViewOrientation(splitViewOrientation === 'horizontal'); + const { + canUseHorizontalSplit, + containerRef: splitViewRef, + isHorizontal, + maximumPrimaryPaneWidth, + } = useResponsiveSplitView(preferredIsHorizontal); const selectFirstResult = React.useCallback((): boolean => { const firstId = resultsRef.current?.find( @@ -120,7 +116,9 @@ export function useQuerySplitView( selectedIndex, setSelectedIndex, isSplit, - canSplit, + canUseHorizontalSplit, + splitViewRef, + maximumPrimaryPaneWidth, isHorizontal, toggleSplit, toggleOrientation, diff --git a/specifyweb/frontend/js_src/lib/hooks/useResponsiveSplitView.ts b/specifyweb/frontend/js_src/lib/hooks/useResponsiveSplitView.ts new file mode 100644 index 00000000000..a5fe60a3273 --- /dev/null +++ b/specifyweb/frontend/js_src/lib/hooks/useResponsiveSplitView.ts @@ -0,0 +1,46 @@ +import React from 'react'; + +import { listen } from '../utils/events'; + +export const minimumSplitPaneWidth = 768; +export const splitViewHandleWidth = 10; +const minimumCombinedPaneWidth = minimumSplitPaneWidth * 2; +const combinedHandleAllowance = splitViewHandleWidth * 2; +export const minimumHorizontalSplitWidth = + minimumCombinedPaneWidth + combinedHandleAllowance; + +export function useResponsiveSplitView(preferredHorizontal: boolean): { + readonly canUseHorizontalSplit: boolean; + readonly containerRef: React.RefCallback; + readonly isHorizontal: boolean; + readonly maximumPrimaryPaneWidth: number; +} { + const [container, setContainer] = React.useState(null); + const containerRef = React.useCallback(setContainer, [setContainer]); + const [containerWidth, setContainerWidth] = React.useState(0); + + React.useEffect(() => { + if (container === null) return undefined; + + const updateWidth = (): void => setContainerWidth(container.clientWidth); + updateWidth(); + const observer = new ResizeObserver(updateWidth); + observer.observe(container); + const removeResizeListener = listen(window, 'resize', updateWidth); + return (): void => { + observer.disconnect(); + removeResizeListener(); + }; + }, [container]); + + const canUseHorizontalSplit = containerWidth >= minimumHorizontalSplitWidth; + return { + canUseHorizontalSplit, + containerRef, + isHorizontal: preferredHorizontal && canUseHorizontalSplit, + maximumPrimaryPaneWidth: Math.max( + 0, + containerWidth - minimumSplitPaneWidth - splitViewHandleWidth + ), + }; +}