Skip to content

Restore link behaviour on datagrid rows - #6868

Merged
lkostrowski merged 5 commits into
saleor:mainfrom
ebrahim2355:fix/6345-datagrid-row-anchor-position
Aug 31, 2026
Merged

Restore link behaviour on datagrid rows#6868
lkostrowski merged 5 commits into
saleor:mainfrom
ebrahim2355:fix/6345-datagrid-row-anchor-position

Conversation

@ebrahim2355

Copy link
Copy Markdown
Contributor

Fixes #6345

Right clicking a product name in the catalogue no longer offers "Open link in new tab", and middle clicking navigates in the current tab instead of opening a new one. The reporter confirmed it still happens on dashboard 3.23.19. Ctrl/Cmd + left click is currently the only way to open a row in a new tab.

Cause

Each list datagrid keeps a single anchor element and moves it over the cell under the cursor, so a row can behave like an ordinary link. Glide reports cell bounds in viewport coordinates — the same bounds TooltipContainer consumes with position: fixed, as its own comment notes. useRowAnchor instead converted them to document coordinates by adding window.scrollX/scrollY, and applied them with position: absolute, whose containing block is the position: relative wrapper in PreventHistoryBack.

The anchor therefore never sat on the cell it pointed at — it was displaced by the wrapper's own offset on the page. Native link gestures hit the grid canvas, which has no handling for them. Ctrl/Cmd + left click kept working only because onCellClicked dispatches a synthetic click straight at the anchor, wherever it happens to be.

Change

  • Position the anchor with position: fixed and use the reported bounds unchanged, so it lands exactly over the hovered cell.
  • Because pointer events now reach the anchor rather than the grid, its visibility has to be exact: it is shown only while it covers the cell under the cursor, and hidden when the pointer moves to the header, the row selection checkbox, a cell with its own action, or off the grid. Glide ignores mouse moves that land on an overlay, so an anchor left parked over a cell would swallow the pointer returning to that cell and the row would lose its hover highlight.
  • Wheel events land on the anchor too. The previous workaround hid it on wheel, which could not work — the browser has already latched the gesture onto the anchor's scroll chain, and React registers onWheel as a passive listener, so preventDefault is ignored. Wheel deltas are forwarded to the grid's own scroller for the axes it can scroll, the page keeps the rest, and the anchor hides until the next hover repositions it.

rowAnchor is only passed by readonly list datagrids, so this does not affect inline cell editing. Left click still resolves to a single navigation: Glide's onMouseUp begins with if (isOutside) return, so onRowClick does not also fire, and the anchor path carries the same navigatorOpts={{ state: getPrevLocationState(location) }} those grids already pass.

Verification

useRowAnchor gained regression coverage: the positioning test uses non-zero page scroll and non-zero bounds, which is what the existing test (all-zero bounds, no scroll) could not catch. It fails on main with 320px instead of 200px.

Driven in Chromium against a real ProductListDatagrid, with the grid deliberately offset from the document origin and the page scrolled:

before after
element under cursor grid canvas div the <a>, with href
anchor rect vs pointer (584, 335) vs (488, 188) covers the pointer
middle click nothing opens the product in a new tab
horizontal wheel over row scrolls grid scrolls grid
row hover highlight yes yes, and clears on leave
checkbox column clickable yes yes

Also confirmed: vertical wheel still scrolls the page, the page does not scroll sideways into a browser back gesture, and the anchor comes back with a fresh href after a scroll.

@ebrahim2355
ebrahim2355 requested review from a team and lkostrowski August 26, 2026 10:35
@changeset-bot

changeset-bot Bot commented Aug 26, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: c780e84

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
saleor-dashboard Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

ebrahim2355 and others added 2 commits August 26, 2026 18:30
Each list datagrid keeps one anchor element that it moves over the cell under the
cursor, so a row can be middle clicked or right clicked like an ordinary link.
Glide reports cell bounds in viewport coordinates, but the anchor added the page
scroll offset to them and was positioned with position: absolute inside a
position: relative wrapper. It therefore landed away from the cell it pointed at,
displaced by the wrapper's own position on the page, and native link gestures hit
the grid canvas instead. Only Ctrl/Cmd + left click still opened a new tab,
because that path dispatches a synthetic click straight at the anchor.

Position the anchor with position: fixed and use the reported bounds unchanged,
which puts it exactly over the hovered cell.

Now that the anchor really covers the cell, pointer events reach it rather than
the grid, so its visibility has to be exact: it is shown only while it covers the
cell under the cursor, and hidden as soon as the pointer moves to the header, the
row selection checkbox, a cell with its own action, or off the grid. Glide
ignores mouse moves that land on an overlay, so an anchor left parked over a cell
would swallow the pointer returning to that cell and the row would lose its hover
highlight.

Wheel events land on the anchor too. The previous workaround hid it on wheel,
which could not work: the browser has already latched the gesture onto the
anchor's scroll chain, and React registers onWheel as a passive listener, so the
scroll could not be redirected. Forward the wheel delta to the grid's own
scroller for the axes it can scroll, leave the rest to the page, and hide the
anchor until the next hover repositions it.

Fixes saleor#6345
The overlay must bind its wheel listener when the <a> mounts — lists start in a loading state, so an effect that ran once with a null ref never attached. Hide immediately (not after 100ms), on any scroll or resize, and drop href so a parked target cannot open the wrong row. Middle click stays native; cmd/ctrl-click that lands on the canvas opens the href with window.open because a synthetic click is untrusted.
@mirekm
mirekm force-pushed the fix/6345-datagrid-row-anchor-position branch from 7813cf3 to db917b0 Compare August 26, 2026 19:59
@codecov

codecov Bot commented Aug 26, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 85.07463% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 57.18%. Comparing base (b6662d2) to head (c780e84).

Files with missing lines Patch % Lines
src/components/Datagrid/Datagrid.tsx 28.57% 9 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6868      +/-   ##
==========================================
+ Coverage   57.16%   57.18%   +0.02%     
==========================================
  Files        3564     3565       +1     
  Lines       74115    74153      +38     
  Branches    18815    18870      +55     
==========================================
+ Hits        42369    42408      +39     
- Misses      31066    31068       +2     
+ Partials      680      677       -3     
Flag Coverage Δ
storybook 49.61% <40.00%> (-0.01%) ⬇️
units 52.47% <85.07%> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@lkostrowski
lkostrowski merged commit a7e18d5 into saleor:main Aug 31, 2026
14 of 15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Regression: rightclicking on product name used to show "open in new tab" option

3 participants