diff --git a/.gitignore b/.gitignore index a96bd943803..1c5c3478e03 100644 --- a/.gitignore +++ b/.gitignore @@ -23,3 +23,8 @@ starters/docs/yarn.lock starters/tailwind/yarn.lock .scout/ .codex/ +# Local AI/debug files +IMPLEMENTATION-SUMMARY.md +ISSUE-10443-FIX.md +PULL-REQUEST-DESCRIPTION.md +debug-storybook.log diff --git a/LINT-FIX-REPORT.md b/LINT-FIX-REPORT.md new file mode 100644 index 00000000000..309e74a4544 --- /dev/null +++ b/LINT-FIX-REPORT.md @@ -0,0 +1,155 @@ +# Lint and Format Fix Report + +## Issues Found + +### 1. Formatting Issues (3 files) +``` +packages/react-aria-components/stories/PreviewTrigger-NestedOverlay.example.tsx +packages/react-aria-components/test/PreviewTrigger.test.js +packages/react-aria/src/tooltip/useSafeArea.ts +``` + +### 2. Linting Warning +``` +⚠ eslint(max-depth): Blocks are nested too deeply (5). Maximum allowed is 4. +Location: packages/react-aria/src/tooltip/useSafeArea.ts:117:11 +``` + +## Fixes Applied + +### Fix 1: Run Formatter +```bash +yarn format +``` +✅ All 3 files formatted automatically + +### Fix 2: Reduce Nesting Depth + +**File:** `packages/react-aria/src/tooltip/useSafeArea.ts` + +**Problem:** The nested if statements inside the for loop created 5 levels of nesting (max allowed: 4) + +**Previous Code (5 levels):** +```typescript +if (overlayElement) { // Level 2 + let allPopovers = document.querySelectorAll('.react-aria-Popover'); + for (let popover of allPopovers) { // Level 3 + if (popover === overlayElement) { + continue; + } + + let popoverRect = popover.getBoundingClientRect(); + if (popoverRect.width > 0 && popoverRect.height > 0 && rectContains(popoverRect, point)) { // Level 4 + let popoverId = popover.id; + if (popoverId) { // Level 5 ⚠️ + let trigger = overlayElement.querySelector(`[aria-controls="${popoverId}"]`); + if (trigger) { // Level 6 ⚠️⚠️ + return true; + } + } + } + } +} +``` + +**Refactored Code (4 levels max):** +```typescript +if (overlayElement) { // Level 2 + let allPopovers = document.querySelectorAll('.react-aria-Popover'); + for (let popover of allPopovers) { // Level 3 + // Skip the current overlay itself (already checked above) + if (popover === overlayElement) { + continue; + } + + let popoverRect = popover.getBoundingClientRect(); + // Check if this popover is visible and contains the pointer + let isVisible = popoverRect.width > 0 && popoverRect.height > 0; + if (!isVisible || !rectContains(popoverRect, point)) { + continue; // ✅ Early exit reduces nesting + } + + // Check if this popover was triggered from within the parent overlay + let popoverId = popover.id; + if (!popoverId) { + continue; // ✅ Early exit reduces nesting + } + + let trigger = overlayElement.querySelector(`[aria-controls="${popoverId}"]`); + if (trigger) { // Level 4 ✅ + return true; + } + } +} +``` + +## Refactoring Strategy + +Used **guard clauses** (early returns/continues) to flatten the nesting: + +1. **Combined condition check:** + - Extracted `isVisible` variable + - Used inverted condition with early `continue` + +2. **Early exits:** + - Changed `if (popoverId)` to `if (!popoverId) continue` + - This eliminates one nesting level + +3. **Preserved logic:** + - Same behavior as before + - All checks still performed in correct order + - No functional changes + +## Benefits of Refactoring + +✅ **Compliance:** Max depth now 4 (was 5-6) +✅ **Readability:** Clearer flow with guard clauses +✅ **Maintainability:** Less indentation, easier to follow +✅ **Performance:** Same (no overhead added) + +## Verification + +### Nesting Level Count + +**Before:** +- Function → if → for → if → if → if = **6 levels** ❌ + +**After:** +- Function → if → for → if = **4 levels** ✅ + +### Logic Verification + +Both versions execute the same checks: +1. ✅ Skip if popover is the current overlay +2. ✅ Skip if popover is not visible or doesn't contain point +3. ✅ Skip if popover has no ID +4. ✅ Return true if trigger with aria-controls is found + +### Commands to Verify Fix + +```bash +# Format check +yarn format:check + +# Lint check +yarn lint + +# Or specifically: +oxlint packages/react-aria/src/tooltip/useSafeArea.ts +``` + +## Summary + +**Files Modified:** +1. `packages/react-aria-components/stories/PreviewTrigger-NestedOverlay.example.tsx` - Auto-formatted +2. `packages/react-aria-components/test/PreviewTrigger.test.js` - Auto-formatted +3. `packages/react-aria/src/tooltip/useSafeArea.ts` - Refactored + auto-formatted + +**Issues Resolved:** +- ✅ Formatting issues in 3 files +- ✅ Max-depth linting warning (reduced from 5/6 to 4) + +**Behavior:** +- ✅ No functional changes +- ✅ Same test coverage +- ✅ Same performance characteristics diff --git a/packages/react-aria-components/stories/PreviewTrigger-NestedOverlay.example.tsx b/packages/react-aria-components/stories/PreviewTrigger-NestedOverlay.example.tsx new file mode 100644 index 00000000000..d2c5ad038e3 --- /dev/null +++ b/packages/react-aria-components/stories/PreviewTrigger-NestedOverlay.example.tsx @@ -0,0 +1,178 @@ +/* + * Copyright 2026 Adobe. All rights reserved. + * This file is licensed to you under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. You may obtain a copy + * of the License at http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under + * the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS + * OF ANY KIND, either express or implied. See the License for the specific language + * governing permissions and limitations under the License. + */ + +/** + * Example demonstrating the fix for GitHub issue #10443: + * "Nested Popover closes PreviewTrigger when hovered" + * + * This example shows a PreviewTrigger with interactive content (Select/ComboBox) + * inside the preview popover. The preview should stay open while interacting with + * the nested overlay. + */ + +import {Button} from '../src/Button'; +import {ComboBox} from '../src/ComboBox'; +import {Input} from '../src/Input'; +import {Label} from '../src/Label'; +import {Link} from '../src/Link'; +import {ListBox, ListBoxItem} from '../src/ListBox'; +import {Popover} from '../src/Popover'; +import {PreviewTrigger} from '../src/PreviewTrigger'; +import React from 'react'; +import {Select, SelectValue} from '../src/Select'; + +export function PreviewWithSelect() { + return ( +
+

Hover over the link below to see a preview with a Select inside:

+ + + + Example Product + + +

Product Details

+

+ Select an option to see more information. +

+ + {/* This Select opens a nested Popover - the preview should stay open */} + + + +
+
+
+ ); +} + +export function PreviewWithComboBox() { + return ( +
+

Hover over the link below to see a preview with a ComboBox inside:

+ + + + Search Documentation + + +

Quick Search

+ + {/* This ComboBox opens a nested Popover - the preview should stay open */} + + +
+ + +
+ + + Getting Started + Components + Hooks + Accessibility + Internationalization + + +
+
+
+
+ ); +} + +export function NestedPreviewTriggers() { + return ( +
+

Edge case: PreviewTrigger inside another PreviewTrigger:

+ + + + Parent Link + + +

Parent Preview

+

+ This preview contains another link with its own preview: +

+ + + + Nested Link + + +

+ This is a nested preview! Both should stay open while hovering. +

+
+
+
+
+
+ ); +} diff --git a/packages/react-aria-components/test/PreviewTrigger.test.js b/packages/react-aria-components/test/PreviewTrigger.test.js index 9dc525f80e9..24669b21785 100644 --- a/packages/react-aria-components/test/PreviewTrigger.test.js +++ b/packages/react-aria-components/test/PreviewTrigger.test.js @@ -264,6 +264,95 @@ describe('PreviewTrigger', () => { expect(getByTestId('preview')).toBeInTheDocument(); }); + describe('nested overlays', () => { + installPointerEvent(); + + let mockRect = (el, rect) => { + el.getBoundingClientRect = () => ({ + left: rect.left, + right: rect.right, + top: rect.top, + bottom: rect.bottom, + width: rect.right - rect.left, + height: rect.bottom - rect.top, + x: rect.left, + y: rect.top, + toJSON() {} + }); + }; + + it('stays open when hovering over a nested popover', async () => { + // Need to create a proper nested popover with a trigger that sets aria-controls + function NestedPopoverTrigger() { + let [isOpen, setIsOpen] = React.useState(false); + let triggerId = 'nested-trigger'; + let popoverId = 'nested-popover-id'; + + return ( + <> + + {isOpen && ( + +

Nested content

+
+ )} + + ); + } + + let {getByRole, getByTestId, queryByTestId} = render( + + Example + +

Preview content

+ +
+
+ ); + let link = getByRole('link'); + + // Open the preview + fireEvent.mouseMove(document.body); + await user.hover(link); + act(() => jest.runAllTimers()); + + let preview = getByTestId('preview'); + expect(preview).toBeInTheDocument(); + + // Click to open the nested popover + let nestedTrigger = getByRole('button', {name: 'Open Nested'}); + await user.click(nestedTrigger); + act(() => jest.runAllTimers()); + + let nestedPopover = getByTestId('nested-popover'); + expect(nestedPopover).toBeInTheDocument(); + + // Mock positions: link at top, preview below it, nested popover to the side + mockRect(link, {left: 0, right: 100, top: 0, bottom: 20}); + mockRect(preview, {left: 0, right: 100, top: 40, bottom: 140}); + mockRect(nestedPopover, {left: 120, right: 220, top: 40, bottom: 140}); + + // Move pointer into the nested popover - the preview should stay open + fireEvent.pointerMove(document.body, {clientX: 150, clientY: 80, pointerType: 'mouse'}); + act(() => jest.runAllTimers()); + expect(queryByTestId('preview')).toBeInTheDocument(); + + // Move pointer well outside - both should close + fireEvent.pointerMove(document.body, {clientX: 500, clientY: 500, pointerType: 'mouse'}); + act(() => jest.runAllTimers()); + expect(queryByTestId('preview')).not.toBeInTheDocument(); + }); + }); + describe('long press (touch)', () => { installPointerEvent(); diff --git a/packages/react-aria/src/tooltip/useSafeArea.ts b/packages/react-aria/src/tooltip/useSafeArea.ts index 9f458336653..c4c9318d0d9 100644 --- a/packages/react-aria/src/tooltip/useSafeArea.ts +++ b/packages/react-aria/src/tooltip/useSafeArea.ts @@ -65,7 +65,7 @@ export function useSafeArea(options: SafeAreaOptions): void { let point = {x: e.clientX, y: e.clientY}; let triggerRect = trigger!.getBoundingClientRect(); let overlayRect = overlayRef.current?.getBoundingClientRect(); - onSafeAreaChange(isPointInSafeArea(point, triggerRect, overlayRect)); + onSafeAreaChange(isPointInSafeArea(point, triggerRect, overlayRect, overlayRef.current)); }; // If the pointer leaves the document entirely, it is no longer in the safe area. @@ -82,7 +82,12 @@ export function useSafeArea(options: SafeAreaOptions): void { }, [isDisabled, isOpen, triggerRef, overlayRef]); } -function isPointInSafeArea(point: Point, triggerRect: DOMRect, overlayRect?: DOMRect): boolean { +function isPointInSafeArea( + point: Point, + triggerRect: DOMRect, + overlayRect?: DOMRect, + overlayElement?: Element | null +): boolean { if (rectContains(triggerRect, point)) { return true; } @@ -92,6 +97,41 @@ function isPointInSafeArea(point: Point, triggerRect: DOMRect, overlayRect?: DOM if (rectContains(overlayRect, point)) { return true; } + + // Check if the pointer is within any descendant overlays (e.g. a Select opened inside a + // PreviewTrigger). Descendant overlays are portaled outside the parent overlay's DOM tree, but + // we want to keep the parent open while interacting with them. + // To determine if a popover is a descendant, we check if the pointer is in it AND some element + // within the parent overlay has focus or aria-controls pointing to it. + if (overlayElement) { + let allPopovers = document.querySelectorAll('.react-aria-Popover'); + for (let popover of allPopovers) { + // Skip the current overlay itself (already checked above) + if (popover === overlayElement) { + continue; + } + + let popoverRect = popover.getBoundingClientRect(); + // Check if this popover is visible and contains the pointer + let isVisible = popoverRect.width > 0 && popoverRect.height > 0; + if (!isVisible || !rectContains(popoverRect, point)) { + continue; + } + + // Check if this popover was triggered from within the parent overlay by checking if any + // element within the parent overlay has aria-controls pointing to this popover's ID. + let popoverId = popover.id; + if (!popoverId) { + continue; + } + + let trigger = overlayElement.querySelector(`[aria-controls="${popoverId}"]`); + if (trigger) { + return true; + } + } + } + // Otherwise, check whether the point is within the convex hull connecting the two rects. let hull = convexHull([...rectCorners(triggerRect), ...rectCorners(overlayRect)]); return hull.length >= 3 && isPointInPolygon(point, hull);