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:
+ ++ Select an option to see more information. +
+ + {/* This Select opens a nested Popover - the preview should stay open */} + + + +Hover over the link below to see a preview with a ComboBox inside:
+ +Edge case: PreviewTrigger inside another PreviewTrigger:
+ ++ This preview contains another link with its own preview: +
+ ++ This is a nested preview! Both should stay open while hovering. +
+Nested content
+Preview content
+