From e459890c1eb6a93d35a57245d5576dc44701a4c1 Mon Sep 17 00:00:00 2001 From: varun2735126 Date: Mon, 17 Aug 2026 22:36:04 +0530 Subject: [PATCH] fix(table): keep item drop target highlight independent of measurement state The drop target background for a TableView row is painted on its cell wrappers, because those are opaque and sit on top of the row. The rule selected them with `.react-spectrum-Table-cellWrapper.react-spectrum-Table-cellWrapper--dropTarget`, chaining the base class purely for specificity. But that base class is applied conditionally on `!layoutInfo.estimatedSize`, so any time a cell height is still an estimate the selector stops matching and the row silently loses its highlight, even though the modifier class is present. Select on the modifier alone and take the specificity from a `.react-spectrum-Table` ancestor instead, so it still wins over the quiet variant's cell wrapper background. Also adds the regression coverage this area was missing (see #5404): pointer and keyboard drags now assert that the targeted row and each of its cell wrappers get the drop target classes, that the highlight follows the target rather than being left behind on a stale one, and that it clears when the drag ends. --- .../@adobe/react-spectrum/src/table/table.css | 10 +- .../test/table/TableDnd.test.js | 95 +++++++++++++++++++ 2 files changed, 102 insertions(+), 3 deletions(-) diff --git a/packages/@adobe/react-spectrum/src/table/table.css b/packages/@adobe/react-spectrum/src/table/table.css index ed803cdca1f..489ca1dd4ca 100644 --- a/packages/@adobe/react-spectrum/src/table/table.css +++ b/packages/@adobe/react-spectrum/src/table/table.css @@ -225,10 +225,14 @@ /* forced-color-adjust: none, so that box-shadow style will render */ forced-color-adjust: none; } +} - .react-spectrum-Table-cellWrapper.react-spectrum-Table-cellWrapper--dropTarget { - background-color: var(--spectrum-table-droptarget-background-color); - } +/* Deliberately not chained with .react-spectrum-Table-cellWrapper: that class is only applied while + the cell height is not an estimate, so chaining it here drops the drop target background whenever + the table is still measuring. The .react-spectrum-Table ancestor supplies the specificity instead, + so this still wins over .spectrum-Table--quiet .spectrum-Table-row .spectrum-Table-cellWrapper. */ +.react-spectrum-Table .react-spectrum-Table-row .react-spectrum-Table-cellWrapper--dropTarget { + background-color: var(--spectrum-table-droptarget-background-color); } @media (forced-colors: active) { diff --git a/packages/@adobe/react-spectrum/test/table/TableDnd.test.js b/packages/@adobe/react-spectrum/test/table/TableDnd.test.js index acfc98faf70..922bb0cf991 100644 --- a/packages/@adobe/react-spectrum/test/table/TableDnd.test.js +++ b/packages/@adobe/react-spectrum/test/table/TableDnd.test.js @@ -3953,5 +3953,100 @@ describe('TableView', function () { fireEvent.keyUp(document.body, {key: 'Escape'}); }); }); + + describe('drop target highlight', function () { + // The highlight is painted on the row (border) and on each of its cell wrappers (background), + // because the cell wrappers are opaque and sit on top of the row. See issue #5404. + function getCellWrappers(row) { + return [...row.children]; + } + + function expectHighlighted(row, isHighlighted) { + expect(row.className.includes('react-spectrum-Table-row--dropTarget')).toBe(isHighlighted); + let wrappers = getCellWrappers(row); + expect(wrappers.length).toBeGreaterThan(0); + for (let wrapper of wrappers) { + expect(wrapper.className.includes('react-spectrum-Table-cellWrapper--dropTarget')).toBe( + isHighlighted + ); + } + } + + it('should highlight the row and its cells while dragging over an item drop target', async function () { + let {getByRole} = render(); + let grid = getByRole('grid'); + let rows = within(within(grid).getAllByRole('rowgroup')[1]).getAllByRole('row'); + + let dragCell = within(rows[1]).getAllByRole('rowheader')[0]; + let dataTransfer = new DataTransfer(); + fireEvent.pointerDown(dragCell, { + pointerType: 'mouse', + button: 0, + pointerId: 1, + clientX: 0, + clientY: 0 + }); + fireEvent(dragCell, new DragEvent('dragstart', {dataTransfer, clientX: 0, clientY: 0})); + act(() => jest.runAllTimers()); + expectHighlighted(rows[0], false); + + // Drop onto the middle of the first row, which is an "on" drop position. + fireEvent(rows[0], new DragEvent('dragover', {dataTransfer, clientX: 1, clientY: 20})); + act(() => { + jest.advanceTimersByTime(100); + }); + + expectHighlighted(rows[0], true); + expectHighlighted(rows[3], false); + + // "Three" is not a folder, so it only accepts insert positions. Dragging over it must clear + // the highlight rather than leaving it behind on the previous target. + fireEvent(rows[3], new DragEvent('dragover', {dataTransfer, clientX: 1, clientY: 145})); + act(() => { + jest.advanceTimersByTime(100); + }); + + expectHighlighted(rows[0], false); + expectHighlighted(rows[3], false); + + fireEvent.pointerUp(dragCell, { + pointerType: 'mouse', + button: 0, + pointerId: 1, + clientX: 1, + clientY: 145 + }); + fireEvent(rows[3], new DragEvent('drop', {dataTransfer, clientX: 1, clientY: 145})); + fireEvent(dragCell, new DragEvent('dragend', {dataTransfer, clientX: 1, clientY: 145})); + act(() => jest.runAllTimers()); + }); + + it('should highlight the row and its cells when a keyboard drag targets an item', async function () { + let {getByRole} = render(); + let grid = getByRole('grid'); + // Rows other than the active drop target are aria-hidden during a keyboard drag, so the + // hidden option is needed to reach them. + let findRow = name => + within(grid) + .getAllByRole('row', {hidden: true}) + .find(row => within(row).queryByText(name)); + + // Start a keyboard drag from "Folder 1". "Folder 2" is the only other row that accepts an + // "on" drop, so it is the first drop target. + await user.tab(); + await user.keyboard('{ArrowRight}'); + await user.keyboard('{Enter}'); + act(() => jest.runAllTimers()); + expect(document.activeElement).toHaveAttribute('aria-label', 'Drop on Folder 2'); + + expectHighlighted(findRow('Folder 2'), true); + expectHighlighted(findRow('Six'), false); + + fireEvent.keyDown(document.body, {key: 'Escape'}); + fireEvent.keyUp(document.body, {key: 'Escape'}); + act(() => jest.runAllTimers()); + expectHighlighted(findRow('Folder 2'), false); + }); + }); }); });