Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -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
155 changes: 155 additions & 0 deletions LINT-FIX-REPORT.md
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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 (
<div style={{padding: '50px'}}>
<p>Hover over the link below to see a preview with a Select inside:</p>

<PreviewTrigger delay={200} closeDelay={100}>
<Link href="https://example.com" target="_blank">
Example Product
</Link>
<Popover
style={{
background: 'white',
border: '1px solid #ccc',
borderRadius: '4px',
padding: '16px',
boxShadow: '0 4px 12px rgba(0,0,0,0.15)',
minWidth: '300px'
}}>
<h3 style={{margin: '0 0 12px 0'}}>Product Details</h3>
<p style={{margin: '0 0 12px 0', color: '#666'}}>
Select an option to see more information.
</p>

{/* This Select opens a nested Popover - the preview should stay open */}
<Select placeholder="Choose a variant" style={{marginBottom: '12px'}}>
<Label>Product Variant</Label>
<Button>
<SelectValue />
<span aria-hidden="true">▼</span>
</Button>
<Popover>
<ListBox>
<ListBoxItem id="small">Small</ListBoxItem>
<ListBoxItem id="medium">Medium</ListBoxItem>
<ListBoxItem id="large">Large</ListBoxItem>
<ListBoxItem id="xl">Extra Large</ListBoxItem>
</ListBox>
</Popover>
</Select>

<Button
style={{
background: '#0074e0',
color: 'white',
border: 'none',
padding: '8px 16px',
borderRadius: '4px',
cursor: 'pointer'
}}>
Add to Cart
</Button>
</Popover>
</PreviewTrigger>
</div>
);
}

export function PreviewWithComboBox() {
return (
<div style={{padding: '50px'}}>
<p>Hover over the link below to see a preview with a ComboBox inside:</p>

<PreviewTrigger delay={200} closeDelay={100}>
<Link href="https://example.com" target="_blank">
Search Documentation
</Link>
<Popover
style={{
background: 'white',
border: '1px solid #ccc',
borderRadius: '4px',
padding: '16px',
boxShadow: '0 4px 12px rgba(0,0,0,0.15)',
minWidth: '300px'
}}>
<h3 style={{margin: '0 0 12px 0'}}>Quick Search</h3>

{/* This ComboBox opens a nested Popover - the preview should stay open */}
<ComboBox>
<Label>Search topics</Label>
<div style={{display: 'flex', gap: '8px'}}>
<Input placeholder="Type to search..." />
<Button>▼</Button>
</div>
<Popover>
<ListBox>
<ListBoxItem>Getting Started</ListBoxItem>
<ListBoxItem>Components</ListBoxItem>
<ListBoxItem>Hooks</ListBoxItem>
<ListBoxItem>Accessibility</ListBoxItem>
<ListBoxItem>Internationalization</ListBoxItem>
</ListBox>
</Popover>
</ComboBox>
</Popover>
</PreviewTrigger>
</div>
);
}

export function NestedPreviewTriggers() {
return (
<div style={{padding: '50px'}}>
<p>Edge case: PreviewTrigger inside another PreviewTrigger:</p>

<PreviewTrigger delay={200} closeDelay={100}>
<Link href="https://example.com" target="_blank">
Parent Link
</Link>
<Popover
style={{
background: 'white',
border: '1px solid #ccc',
borderRadius: '4px',
padding: '16px',
boxShadow: '0 4px 12px rgba(0,0,0,0.15)',
minWidth: '250px'
}}>
<h3 style={{margin: '0 0 12px 0'}}>Parent Preview</h3>
<p style={{margin: '0 0 12px 0'}}>
This preview contains another link with its own preview:
</p>

<PreviewTrigger delay={200} closeDelay={100}>
<Link href="https://example.com/nested" target="_blank">
Nested Link
</Link>
<Popover
style={{
background: 'white',
border: '1px solid #0074e0',
borderRadius: '4px',
padding: '12px',
boxShadow: '0 4px 12px rgba(0,0,0,0.15)',
minWidth: '200px'
}}>
<p style={{margin: 0}}>
This is a nested preview! Both should stay open while hovering.
</p>
</Popover>
</PreviewTrigger>
</Popover>
</PreviewTrigger>
</div>
);
}
Loading