Repository navigation
fix(application-shell): let clicks on navigation menu labels reach their link (FEC-1479) - #4224
fparadelact wants to merge 1 commit into
Conversation
…eir link The submenu's safe area started its triangle exactly under the pointer, so when the submenu opened on hover, the release of a click on the menu item often landed on the safe area instead of the label's link, and the click only opened the submenu. Start the triangle 2px off the pointer, on the side away from the submenu, so the label stays clickable and every path into the submenu still crosses the triangle.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: 3b08e86 The changes in this PR will be included in the next version bump. This PR includes changesets to release 29 packages
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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4224 +/- ##
==========================================
+ Coverage 75.44% 75.95% +0.50%
==========================================
Files 297 297
Lines 7713 7714 +1
Branches 2435 2396 -39
==========================================
+ Hits 5819 5859 +40
+ Misses 1873 1834 -39
Partials 21 21
Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
fparadelact
changed the title
| submenuRef.current.style.setProperty( | ||
| '--safe-start', | ||
| `${safeX}% calc(${safeY}% ${isSubmenuAboveMenuItem ? '-' : '+'} 2px)` | ||
| ); |
There was a problem hiding this comment.
Thanks, the fix works. In my browser test, label clicks now reach the link: 0/60 before, 59/60 with this PR, including clicks at sub-pixel positions.
I found one side effect, in the collapsed navbar, and I suggest a small change before merging.
Collapsed navbar
In the collapsed navbar, the submenu starts at the bottom of the menu item (menuItemBottom). So the pointer on an icon is never inside the safe area, and clicks on icons were never lost.
But the pointer's Y is then clamped to 0% (or 100% when the submenu opens upward), and the + 2px moves the tip into the safe area. This leaves a thin band right below the hovered icon that the triangle no longer covers. When the mouse passes through that band on its way to the submenu, the next icon takes the hover and the submenu switches.
Suggestion: add the offset only when the pointer is inside the safe area.
// When the pointer is above or below the safe area (collapsed navbar), Y is
// clamped to 0% or 100%. An offset there would open a gap along that edge.
const isPointerInsideSafeArea = safeY > 0 && safeY < 100;
const startY = isPointerInsideSafeArea
? `calc(${safeY}% ${isSubmenuAboveMenuItem ? "-" : "+"} 2px)`
: `${safeY}%`;
submenuRef.current.style.setProperty("--safe-start", `${safeX}% ${startY}`);With this change, the existing tests in navbar.spec.tsx still pass. A test for the clamped case would be good, for example: pointer above the safe area, so --safe-start has no offset.
Merchant Center
No code in merchant-center-frontend depends on these internals (sometimes devs target internals). The Cypress command navigateToMenu force-clicks the submenu link, so the e2e tests do not depend on where the label click lands.
Summary
Clicking the label of a left navigation menu item that has a submenu (Products, Categories) often only opens the submenu instead of navigating. The submenu's safe area, the invisible triangle that keeps the submenu open while the pointer travels to it, starts exactly under the pointer and takes the release of the click. This starts the triangle 2px off the pointer, on the side away from the submenu.
Observed behavior
pointerdownlands on the label, inside the<a>. Hovering has opened the submenu, and the tip is written at the pointer's position.pointeruplands on theSafeAreaspan, outside the<a>. Theclickgoes to their common ancestor, so theNavLinknever gets it.mousemove's whole-pixelclientX/Y, while pointer events keep the fraction, so the pointer sits a fraction of a pixel right of and below the tip. The click is lost when that fraction falls inside the triangle's corner, or when it is zero. If the lastmousemovecame before the submenu settled, the tip is stale and the click succeeds.The race is why a test can miss it.
Expected behavior
A click on the label always navigates. The submenu stays open on every path into it, including diagonals across other menu items.
What changed
isSubmenuAboveMenuItemstate for the direction.1%is under 1px for a submenu shorter than 100px, and a sub-pixel offset could reach the tip again.pointer-events: nonewhile over the item (loses every diagonal, as Michael found in fix(application-shell): keep the left navigation scrollable over submenus (FEC-1449) #4204), the tip 8px right of the pointer (loses slow diagonals), the tip at the item's right edge (loses every diagonal).Testing
Unit tests:
--safe-startfor a downward and an upward submenu. jsdom does no hit-testing, so they cover the value, not the click.Playwright against the playground built from this branch (Chromium, 1920×1080). It presses the label at sub-pixel offsets from a fresh tip, and moves from the label to every submenu entry and straight right, slow and fast. The upward case pushes the menu item down so the submenu flips.
The click offsets are chosen to probe the triangle's corner, so the before column is not a real-world rate.
On staging, by emulation (the probe rewrote
--safe-startin the page): the same offset kept all 44 paths into the submenus of Products, Categories and Settings, and all 6 diagonals across a neighbouring menu item. On a straight move to the neighbour, the neighbour still took over (2/2).By hand in the playground: the label always navigates. The submenu stays open while the pointer is over the item or moves towards the submenu.