feat: improve citation component UX with enhanced interactions and visual feedback

[bensonwong]

Summary

This PR enhances the citation component user experience with improved interaction patterns, better visual feedback, and increased content display area.

Changes Made

1. Fixed Click-Outside Popover Dismissal Bug

Added desktop click-outside handler that bypasses grace period while respecting image overlay states.

Files: src/react/CitationComponent.tsx

2. High-Contrast Hover States for Chip Variant

Implemented inverted color schemes - dark bg/white text in light mode, light bg/dark text in dark mode.

Files: src/react/CitationVariants.tsx

3. Increased Popover Width

Increased from 384px to 480px for better content display.

Files: src/react/CitationComponent.tsx, src/react/constants.ts

4. Enhanced CitationDrawer Header

Dual-line display showing page title and domain separately.

Files: src/react/CitationDrawer.tsx

5. Removed Superscript Brackets

Changed default to hideBrackets=true for cleaner display.

Files: src/react/CitationVariants.tsx

6. Code Quality & Tests

Added 4 new tests for click-outside behavior. All passing.

Files: src/tests/CitationComponentBehavior.test.tsx

Test Coverage

• Dismisses when clicking outside
• Does not dismiss when clicking inside trigger/popover
• Dismisses during grace period

All tests passing: 4 pass, 5 skip, 0 fail

[github-actions]

✅ Playwright Test Report

Status: Tests passed

📊 Download Report & Snapshots (see Artifacts section)

What's in the Visual Snapshots

The gallery includes visual snapshots for:

• 🖥️ Desktop showcase (all variants × all states)
• 📱 Mobile showcase (iPhone SE viewport)
• 📟 Tablet showcase (iPad viewport)
• 🔍 Popover states (verified, partial, not found)
• 🔗 URL citation variants

---

Run ID: 22034196332

[claude]

Code Review Summary

I've reviewed PR #244 and found the changes to be well-implemented with good test coverage. Here's my detailed analysis:

---

✅ Positive Aspects

1. Excellent Bug Fix - Desktop Click-Outside Handler

The new desktop click-outside dismiss handler (lines 1956-2013 in CitationComponent.tsx) is well-designed:

• Proper separation of concerns: Desktop (mousedown) vs mobile (touchstart) handlers
• Clear documentation: Excellent inline comments explaining the grace period bypass
• Correct event handling: Uses capture phase to intercept clicks before bubbling
• Smart timeout cleanup: Clears pending hover timeouts when dismissing immediately
• Ref-based checks: Uses isAnyOverlayOpenRef to avoid stale closures

2. Comprehensive Test Coverage

The new test suite (src/__tests__/CitationComponentBehavior.test.tsx) adds 157 lines of well-structured tests:

• ✅ Click outside to dismiss
• ✅ Click inside trigger (should not dismiss)
• ✅ Click inside popover content (should not dismiss)
• ✅ Dismissal during grace period after content expansion
• Good use of matchMedia mocking to simulate desktop environment

3. UX Improvements

• Superscript variant: Removing brackets (hideBrackets = true) is cleaner for academic-style citations
• Popover sizing: Increased from 384px to 480px provides more breathing room for content
• Image sizing: Larger max dimensions (70vw/50vh → 480px/360px) improve readability
• Citation drawer header: Adding domain/URL subtitle for URL sources improves clarity

4. Accessibility Enhancement

Added proper aria-label to SourceGroupHeader (line 263 in CitationDrawer.tsx)

---

⚠️ Issues & Concerns

1. CRITICAL: Chip Variant Hover State Has Low Contrast Risk

Lines 233-241 in CitationVariants.tsx

The new hover states use solid color backgrounds with white text, which creates severe visual jarring and potential accessibility issues:

// BEFORE (subtle):
"hover:bg-amber-50 dark:hover:bg-amber-950/30"

// AFTER (jarring):
"hover:bg-amber-700 hover:text-white dark:hover:bg-amber-200 dark:hover:text-amber-900"

Problems:

• Sudden color inversion on hover feels aggressive and unnatural
• Dark mode amber-200 background with amber-900 text may fail WCAG AA contrast (needs verification)
• Inconsistent with design system: Most UI libraries use subtle hover states, not dramatic inversions
• Visual noise: When multiple citations are present, hovering creates distracting color flashes

Recommendation: Revert to subtle hover states or use a 10-20% opacity overlay instead of full color inversion.

2. MEDIUM: Missing Type Guard in Event Handlers

While the click-outside handlers have good type guards (target instanceof Node), they could be more defensive:

// Current (line 1979-1982):
const target = e.target;
if (\!(target instanceof Node)) {
  return;
}

// Suggested improvement:
const target = e.target;
if (\!target || \!(target instanceof Node)) {
  return;
}

Reasoning: In edge cases (synthetic events, certain browser extensions), e.target could be null.

3. MINOR: Duplicate Logic Between Mobile/Desktop Handlers

The mobile (lines 1916-1941) and desktop (lines 1971-2000) click-outside handlers have ~80% code duplication.

Suggestion: Extract common logic into a shared helper to improve maintainability and reduce bugs from inconsistent updates.

4. MINOR: Test Coverage Gap

The PR description mentions a bug with clicking suppression during the 2.5s grace period. Consider adding a regression test that:

1. Opens popover
2. Expands content (triggering grace period)
3. Clicks outside immediately (< 2.5s)
4. Asserts popover closes (this would fail before the fix)

---

🔒 Security Review

✅ No security concerns identified

• Event handlers properly check .contains() to prevent XSS via DOM manipulation
• No new external dependencies
• No eval/innerHTML usage
• Proper sanitization in place for URLs

---

🚀 Performance Considerations

✅ Good performance practices:

• Document-level listeners only attach when needed (isMobile && isHovering)
• Proper cleanup in useEffect return functions
• Uses capture phase efficiently

---

📋 Additional Observations

PR Description Clarity

The PR description mentions confusion about the drawer header. The changes (lines 278-288 in CitationDrawer.tsx) actually improve clarity by adding a domain subtitle for URL sources. This helps users distinguish between:

• Title: "Build muscular arms with kettlebell moves"
• Domain: "fitandwell.com"

The intent appears sound. If still confusing, consider adding a visual separator (e.g., · or |) between favicon and text.

---

🎯 Recommendations

Must Fix Before Merge:

1. Revert chip hover state changes or provide justification + accessibility audit

Should Fix:

2. Add null check to event target guards
3. Add regression test for the original bug

Nice to Have:

4. Extract duplicate logic into shared helper
5. Add visual separator in drawer header if clarity is still a concern

---

✅ Final Verdict

Approve with changes requested.

The core bug fix (desktop click-outside dismiss) is well-implemented and thoroughly tested. The popover sizing improvements and accessibility enhancements are positive changes. However, the chip variant hover state needs reconsideration due to accessibility and UX concerns.

Suggested merge path:

1. Revert lines 233-241 in CitationVariants.tsx to previous subtle hover states
2. Merge remaining changes
3. Address chip hover UX in a separate PR with design review

Great work on the comprehensive test coverage! 🎉

[claude]

Pull Request Review: feat: improve citation component UX with enhanced interactions and visual feedback

Overall Assessment

This PR makes solid UX improvements to the citation component with well-structured changes. The code quality is high, with excellent documentation and comprehensive test coverage. However, I've identified a few areas that need attention before merging.

---

Critical Issues

1. Test Code Duplication - DRY Violation

File: src/__tests__/CitationComponentBehavior.test.tsx

The new test suite has significant code duplication. The matchMedia mock setup is repeated identically in all 4 test cases:

Object.defineProperty(window, "matchMedia", {
  writable: true,
  value: jest.fn().mockImplementation(query => ({
    matches: query === "(pointer: fine)",
    media: query,
    onchange: null,
    addEventListener: jest.fn(),
    removeEventListener: jest.fn(),
    dispatchEvent: jest.fn(),
  })),
});

Recommendation: Extract this to a beforeEach hook or a test helper function:

describe("desktop click-outside dismiss", () => {
  beforeEach(() => {
    // Mock desktop environment
    Object.defineProperty(window, "matchMedia", {
      writable: true,
      value: jest.fn().mockImplementation(query => ({
        matches: query === "(pointer: fine)",
        media: query,
        onchange: null,
        addEventListener: jest.fn(),
        removeEventListener: jest.fn(),
        dispatchEvent: jest.fn(),
      })),
    });
  });

  it("dismisses popover when clicking outside on desktop", async () => {
    // Test implementation without mock setup...
  });
  // ... other tests
});

---

Moderate Concerns

2. High-Contrast Hover States May Be Too Aggressive

File: src/react/CitationVariants.tsx:233-241

The new hover states use fully inverted colors (dark background with white text). While this provides excellent visual feedback, it may feel jarring compared to the previous subtle hover states:

// Before: Subtle background tint
"hover:bg-amber-50 dark:hover:bg-amber-950/30"

// After: Full inversion
"hover:bg-amber-700 hover:text-white dark:hover:bg-amber-200 dark:hover:text-amber-900"

Considerations:

• The high contrast might be too visually "loud" for inline citations
• May distract from reading flow when hovering over multiple citations
• Good accessibility improvement for users who need high contrast

Recommendation: Consider A/B testing or gathering user feedback. If keeping this approach, document the design decision in the PR description or code comments.

---

3. Popover Width Increase Needs Responsive Consideration

Files: src/react/CitationComponent.tsx, src/react/constants.ts

The popover width increased from 384px to 480px (25% increase). While this provides more content space, it could cause issues on smaller screens:

• The max-width constraint is calc(100vw - 32px), which should prevent overflow
• On tablets (768px width), the popover now takes up 62.5% of screen width vs 50% before
• This is a significant visual presence on medium-sized screens

Recommendation:

• Consider using a responsive width: smaller on mobile/tablet, larger on desktop
• Or add a note in documentation about when to customize via --dc-popover-width
• The CSS custom property approach is good for flexibility

---

4. Missing Test Coverage for Image Overlay Interaction

File: src/__tests__/CitationComponentBehavior.test.tsx:2677-2679

The test file includes this comment:

// Note: Testing the image overlay protection is complex due to interaction modes.
// The handler checks isAnyOverlayOpenRef which is set by the CitationOverlayContext.
// This is covered by integration tests and visual testing.

Concern: While integration tests may cover this, the critical behavior (not dismissing popover when image overlay is open) should have a unit test. This is a regression-prone edge case.

Recommendation: Add a test that:

1. Opens the popover
2. Opens the image overlay (mocking isAnyOverlayOpenRef.current = true)
3. Clicks outside
4. Verifies popover stays open

---

Minor Issues

5. Inconsistent Default Change Documentation

File: src/react/CitationVariants.tsx:321

The hideBrackets default changed from false to true (removing brackets by default). This is a breaking visual change for existing users.

Recommendation:

• Add a migration note in the PR description
• Consider bumping the minor version (semver)
• Document in CHANGELOG or release notes

---

6. Code Comments Could Be More Concise

File: src/react/CitationComponent.tsx:1956-1967

The comment block is thorough but verbose (12 lines). Consider condensing:

// Desktop click-outside handler: dismisses popover when clicking outside trigger/content.
// Uses mousedown (not touchstart) for desktop UX consistency. Bypasses grace period to
// respect intentional clicks. Preserves popover when image overlay is open.

---

Positive Highlights

✅ Excellent test coverage - 4 new tests covering the critical behavior
✅ Clear separation of concerns - Desktop vs mobile handlers are well-separated
✅ Good use of refs - Prevents stale closure issues with isAnyOverlayOpenRef
✅ Accessibility improvements - Added aria-label to CitationDrawer header
✅ Capture phase handling - Correct use of capture phase for click-outside detection
✅ Thorough documentation - Comments explain the "why" not just the "what"

---

Performance Considerations

No significant performance concerns. The event listeners are properly cleaned up in the useEffect return function. The capture phase event handling is appropriate for this use case.

---

Security

No security concerns identified. The code doesn't introduce any XSS vulnerabilities or unsafe DOM manipulation.

---

Recommendations Summary

Before merging:

1. ✅ Refactor test code to eliminate duplication (extract beforeEach)
2. ⚠️ Add test coverage for image overlay interaction
3. ⚠️ Document the hideBrackets breaking change

Consider for follow-up:

• Gather user feedback on high-contrast hover states
• Add responsive popover width for tablets
• Document popover width customization in user guide

---

Overall, this is a well-crafted PR with thoughtful UX improvements. The main issue is test code duplication, which should be addressed before merging. The other concerns are more about design decisions and edge case coverage.

Verdict: Approve with minor changes requested ✨

[claude]

PR Review: feat: improve citation component UX with enhanced interactions and visual feedback

Overall, this is a well-crafted PR with thoughtful improvements to the citation component UX. The changes are focused, well-tested, and demonstrate good attention to detail. Here's my detailed feedback:

---

✅ Strengths

1. Excellent Click-Outside Implementation

The desktop click-outside handler (CitationComponent.tsx:1956-2013) is well-designed:

• ✅ Properly uses mousedown with capture phase for early detection
• ✅ Correctly bypasses grace period for intentional clicks while preserving overlay protection
• ✅ Clear separation between mobile (touch) and desktop (mouse) handlers
• ✅ Excellent inline documentation explaining the rationale
• ✅ Comprehensive test coverage (4 new tests)

2. High-Quality Test Coverage

The new tests (CitationComponentBehavior.test.tsx:2525-2680) are excellent:

• ✅ Cover all critical scenarios: outside clicks, inside clicks, grace period behavior
• ✅ Properly mock matchMedia for desktop simulation
• ✅ Good use of test helpers (waitForPopoverVisible, waitForPopoverDismissed)
• ✅ Clear test descriptions and comments

3. Improved Visual Feedback

The chip variant hover states (CitationVariants.tsx:233-241) provide much better contrast:

• ✅ Inverted color scheme (dark bg/white text) is more noticeable
• ✅ Consistent pattern across all status states
• ✅ Works well in both light and dark modes

4. Better Information Hierarchy

The CitationDrawer dual-line header (CitationDrawer.tsx:279-284) improves scannability:

• ✅ Title and domain separation reduces visual clutter
• ✅ Muted domain text provides context without competing for attention
• ✅ Proper accessibility with aria-label

---

🔍 Code Quality Observations

Minor Issues

1. Potential Memory Leak in Event Listener (CitationComponent.tsx:1968-2013)
The cleanup function is correct, but the dependency array [isMobile, isHovering] means the listener is recreated every time isHovering changes. This could lead to rapid mount/unmount cycles.

Suggestion: Consider using a ref pattern similar to the mobile handler to avoid recreating the listener:

// Keep isHovering in a ref to avoid recreation
const isHoveringRef = useRef(isHovering);
useEffect(() => {
  isHoveringRef.current = isHovering;
}, [isHovering]);

useEffect(() => {
  if (isMobile) return;

  const handleOutsideClick = (e: MouseEvent) => {
    // Only proceed if hovering (check ref to avoid stale closure)
    if (!isHoveringRef.current) return;

    // ... rest of the logic
  };

  document.addEventListener("mousedown", handleOutsideClick, { capture: true });
  return () => {
    document.removeEventListener("mousedown", handleOutsideClick, { capture: true });
  };
}, [isMobile]); // Only recreate on device type change

2. Inconsistent Test Mocking
The matchMedia mock is duplicated across all 4 tests. Consider extracting to a beforeEach or test helper:

describe("desktop click-outside dismiss", () => {
  beforeEach(() => {
    mockDesktopEnvironment();
  });

  // ... tests
});

3. Magic Numbers in Image Dimensions (CitationComponent.tsx:934-935)
The new dimensions 480px and 360px are hardcoded. Consider extracting to constants:

const POPOVER_IMAGE_MAX_WIDTH = "min(70vw, 480px)";
const POPOVER_IMAGE_MAX_HEIGHT = "min(50vh, 360px)";

---

🎯 Performance Considerations

Good Practices

• ✅ Using capture: true for early event interception
• ✅ Proper cleanup of event listeners in useEffect
• ✅ Type guards before DOM operations

Potential Optimization

The click handler checks isAnyOverlayOpenRef.current on every mousedown event document-wide. While the performance impact is minimal, you could add an early return if the popover isn't even open:

const handleOutsideClick = (e: MouseEvent) => {
  // Early return if popover not shown (avoids unnecessary ref checks)
  if (!isHoveringRef.current) return;

  if (isAnyOverlayOpenRef.current) return;
  // ... rest
};

---

🔒 Security

No security concerns identified:

• ✅ Event listener cleanup prevents memory leaks
• ✅ Type guards prevent unsafe DOM operations
• ✅ No XSS vectors introduced
• ✅ Proper use of stopPropagation to prevent event bubbling issues

---

🧪 Test Coverage

Excellent Coverage ✅

• Click outside dismisses popover
• Click inside trigger doesn't dismiss
• Click inside popover doesn't dismiss
• Dismisses during grace period

Potential Gap

Consider adding a test for the image overlay scenario mentioned in the comment (line 2677-2679). Even if complex, a smoke test would be valuable:

it("does not dismiss when image overlay is open", async () => {
  // Test that clicking outside while viewing zoomed image doesn't dismiss popover
});

---

📊 Accessibility

Good Practices ✅

• aria-label added to drawer header
• Proper role="heading" and aria-level={3}
• Button has aria-label for zoom action

---

🎨 Visual Design

The changes align well with modern UI patterns:

• ✅ Increased popover width (384px → 480px) provides more breathing room
• ✅ High-contrast hover states match accessibility guidelines
• ✅ Dual-line header follows common design patterns (see: Gmail, Slack)
• ✅ Superscript bracket removal reduces visual noise

---

📝 Documentation

The code is well-commented:

• ✅ Clear explanation of why separate mobile/desktop handlers
• ✅ Explanation of grace period bypass rationale
• ✅ Note about overlay protection
• ✅ Test comments explain expected behavior

---

🚀 Recommendation

APPROVE with minor suggestions. The PR is production-ready as-is. The suggestions above are optimizations, not blockers.

Action Items (Optional)

1. Consider extracting matchMedia mock to helper
2. Consider ref pattern for isHovering to reduce listener recreation
3. Consider adding image overlay test case
4. Consider extracting magic numbers to constants

Great work! This PR demonstrates thoughtful UX design and solid engineering practices.

[claude]

PR Review: Citation Component UX Improvements

This PR makes several thoughtful UX enhancements to the citation component. The changes are well-tested and demonstrate good attention to detail.

Strengths

1. Click-Outside Handler Implementation

• Clean separation between mobile and desktop handlers with clear rationale
• Proper use of capture phase for early event interception
• Good edge case handling: respects image overlay state, bypasses grace period intentionally
• Excellent inline documentation explaining the why behind the implementation
• Comprehensive test coverage with 4 new test cases covering all scenarios

2. Test Quality

• Tests are well-structured with clear naming and comments
• Good use of beforeEach to mock desktop environment consistently
• Tests cover positive and negative cases thoroughly
• Grace period edge case is tested explicitly

3. Popover Width Increase

• Consistent change across the codebase (constants, component, tests)
• Maintains responsive behavior with viewport constraints
• Updated test assertions to match new dimensions

4. Visual Enhancements

• High-contrast hover states provide better feedback
• CitationDrawer dual-line header improves information hierarchy
• Superscript default change reduces visual clutter

Code Quality Observations

1. Event Handler Cleanup
The dependency array includes isHovering, which means this effect re-runs every time the popover opens/closes. This is correct behavior - you do not want the click-outside handler active when the popover is closed. Adding a comment clarifying this intentional behavior would be helpful.

2. Color Accessibility
The inverted hover states are visually striking. Consider testing these with a color blindness simulator to ensure amber-700, red-700, and green-700 are sufficiently distinguishable when all displayed as white text on colored backgrounds.

3. Dual-Line Header Layout
The layout properly prevents showing the domain when it is the same as the source name. Question: What happens when group.sourceDomain is a full URL instead of just the domain? Should there be URL parsing/sanitization here?

Performance Considerations

The desktop click-outside handler adds/removes document-level event listeners on every popover open/close cycle. For typical usage this is negligible. Only optimize if profiling shows this is a bottleneck.

Security Considerations

No concerns identified. The changes do not introduce new attack vectors.

Test Coverage Assessment

Current Coverage:

• Click outside dismisses popover
• Click inside trigger does not dismiss
• Click inside popover content does not dismiss
• Dismisses during grace period
• Image overlay protection covered by integration tests

Suggestions:

1. Add test for rapid open/close cycles to ensure no memory leaks
2. Add test for device type changes
3. Consider adding visual regression tests for the new hover states

Minor Suggestions

Consider extracting the popover dimension magic numbers to named constants in constants.ts for better maintainability.

Summary

This is a solid PR with well-thought-out changes. The click-outside handler is implemented correctly with good test coverage, and the visual enhancements improve usability. My suggestions are mostly minor optimizations and documentation improvements rather than critical issues.

Recommendation: Approve with minor suggestions

The code is production-ready as-is. The suggestions above are optional improvements that could be addressed in follow-up PRs if desired.

Great work on the comprehensive testing and clear documentation!

[claude]

Code Review Summary

This PR successfully enhances the citation component UX with improved interaction patterns and visual feedback. The implementation is well-tested, follows existing conventions, and addresses real usability issues. Overall: Approved with minor observations.

---

✅ Strengths

1. Excellent Click-Outside Handler Implementation

The new desktop click-outside dismiss handler (CitationComponent.tsx:1956-2013) is well-designed:

• ✅ Properly uses capture phase with mousedown for desktop (vs touchstart for mobile)
• ✅ Correctly bypasses grace period for intentional clicks while preserving it for mouseleave events
• ✅ Respects isAnyOverlayOpenRef to prevent dismissal when image overlay is open
• ✅ Mirrors mobile handler pattern for consistency
• ✅ Comprehensive comments explain the "why" not just the "what"

2. Comprehensive Test Coverage

The test additions are exemplary:

• ✅ 4 new tests covering all critical scenarios
• ✅ Proper use of beforeEach to eliminate duplication
• ✅ Tests verify behavior during grace period (the bug this fixes)
• ✅ Playwright tests updated to reflect intentional behavior changes

3. Consistent Constant Usage

Good refactoring to use POPOVER_WIDTH_VAR and POPOVER_WIDTH_DEFAULT constants instead of hardcoded values.

4. Improved Visual Feedback

The high-contrast chip hover states are a significant UX improvement:

• Light mode: Dark bg with white text
• Dark mode: Light bg with dark text
• Creates strong visual feedback for interaction

5. Enhanced CitationDrawer Header

The dual-line display (title + domain) improves clarity for URL citations without adding visual clutter.

---

🔍 Observations & Suggestions

1. Accessibility: Color Contrast Verification

The new chip hover states use inverted colors (CitationVariants.tsx:233-241). Verify these meet WCAG AA contrast ratios (4.5:1 for normal text). All appear to meet standards, but worth confirming in browser DevTools.

2. Potential Edge Case: Rapid Clicks During Grace Period

The desktop handler clears hoverCloseTimeoutRef (line 1997-2000), which is good. The implementation looks safe since both handlers set isHovering(false), but worth noting for future maintenance.

3. Superscript Brackets Default Change

Changing hideBrackets default from false to true is a breaking change for existing users who rely on the default behavior. However, this is a visual-only change and the cleaner look (¹✓ vs [1✓]) is likely preferred by most users. ✅ Acceptable as a non-major-version UX improvement.

4. Popover Width Increase Impact

The increase from 384px to 480px (25% larger) is significant. May feel too wide on smaller desktop screens (~1366px laptops), but the max-width: calc(100vw - 32px) prevents overflow ✅. The CSS custom property --dc-popover-width allows users to override if needed. Good design decision.

5. CitationDrawer Domain Display Logic

The logic correctly hides duplicate domains when sourceName === sourceDomain ✅

---

🔒 Security

✅ No security concerns identified:

• No user input is being rendered unsanitized
• Event handlers properly check target types
• No new XSS vectors introduced

---

⚡ Performance

✅ No performance regressions:

• Event listeners properly cleaned up in useEffect returns
• Listeners only attached when needed (isMobile || !isHovering guards)
• No unnecessary re-renders introduced

Minor optimization opportunity (not required): The desktop click handler recreates on every isHovering change. This is fine since popovers open/close infrequently. Current implementation prioritizes code clarity over micro-optimization. ✅ Good trade-off.

---

📝 Code Quality

✅ Excellent:

• Comprehensive inline comments explaining "why"
• Consistent with existing codebase patterns
• Proper TypeScript type guards
• No linting errors (biome formatting addressed)

---

🧪 Testing

✅ Strong coverage:

• Unit tests for click-outside behavior
• Playwright tests updated for intentional changes
• Tests use proper async/await patterns
• No skipped tests related to these changes

CI Status: Tests passing ✅ (test suite: SUCCESS, lint-and-validate: SUCCESS)

---

Final Recommendation

APPROVE ✅

This PR represents high-quality work that meaningfully improves UX without introducing technical debt. The click-outside handler fixes a real usability bug, the visual improvements enhance discoverability, and the test coverage ensures regression prevention.

Nitpicks are minor and non-blocking. Ready to merge.

---

Optional Follow-ups (Future PRs)

1. Consider adding a visual regression test for the new chip hover states
2. Document the popover width CSS custom property in CLAUDE.md if not already present
3. Consider user testing feedback on the 480px width to ensure it feels right across screen sizes

Great work! 🎉

[claude]

Code Review: PR #244

Overall, this is a well-structured PR with clear UX improvements and excellent test coverage. The changes are focused and the code quality is high. Here's my detailed review:

---

✅ Strengths

1. Excellent Documentation

• The PR description clearly explains each change with affected files
• Inline comments in the code are thorough (especially the desktop click-outside handler rationale)
• Test descriptions are clear and cover the key scenarios

2. Strong Test Coverage

• Added 4 new tests for click-outside behavior
• Tests cover positive and negative cases (clicking outside, inside trigger, inside popover)
• Tests verify the grace period bypass behavior
• Desktop environment is properly mocked with matchMedia

3. Thoughtful UX Improvements

• Click-outside dismiss: Fixes a real usability issue where users couldn't easily dismiss popovers
• High-contrast hover states: The inverted color scheme (dark bg in light mode, light bg in dark mode) provides clear visual feedback
• Increased popover width: 384px → 480px gives more breathing room for content
• Dual-line drawer header: Separating title and domain improves scanability

4. Code Quality

• Good separation of concerns (desktop vs mobile handlers)
• Proper cleanup in useEffect return statements
• Uses capture phase for event handling (correct pattern)
• Consistent with existing codebase patterns

---

🔍 Issues & Concerns

1. ⚠️ Documentation Update Needed (Minor)

The comment in constants.ts (lines 115-119) references the old default width:

/**
 * ...
 * :root {
 *   --dc-popover-width: 500px; // Override default 384px  ← OUTDATED
 * }
 * ...
 */
export const POPOVER_WIDTH_DEFAULT = "480px";

Fix: Update the comment to say // Override default 480px

---

2. 🤔 Potential Accessibility Concern (Medium)

In CitationDrawer.tsx, the new dual-line header shows domain as a separate <span>:

<div className="flex-1 min-w-0 flex flex-col">
  <span className="text-sm font-medium...">{sourceName}</span>
  {isUrlSource && group.sourceDomain && group.sourceDomain !== sourceName && (
    <span className="text-[11px] text-gray-500...">{group.sourceDomain}</span>
  )}
</div>

Concern: Screen readers will announce these as two separate text nodes without clear semantic relationship. The existing aria-label on the parent <div> helps, but consider enhancing it to explicitly include domain information for URL sources.

---

3. 🎨 High-Contrast Hover: Accessibility Check (Low)

The new high-contrast hover states should be verified for WCAG contrast ratios:

// Light mode: white text on colored background
hover:bg-green-700 hover:text-white

// Dark mode: dark text on light background
dark:hover:bg-green-200 dark:hover:text-green-900

Action Needed: Verify these combinations meet WCAG AA (4.5:1 for normal text):

• White on green-700, red-700, amber-700
• green-900 on green-200, red-900 on red-200, amber-900 on amber-200

Most should pass, but amber combinations sometimes have contrast issues.

---

4. 📏 Magic Numbers: Consider Constants (Low)

The popover image size changes use inline values:

maxWidth="min(70vw, 480px)"
maxHeight="min(50vh, 360px)"

Suggestion: Consider extracting these to named constants for consistency with POPOVER_WIDTH_DEFAULT.

---

🧪 Testing

Strengths:

• Desktop environment properly mocked
• Click-outside scenarios well covered
• Grace period bypass explicitly tested

Potential Gaps:

1. Missing test: Click-outside behavior when image overlay is open (acknowledged in comment)
2. Keyboard navigation: Escape key dismissal testing

---

🔒 Security

No security concerns identified. The changes are purely UI/UX focused.

---

⚡ Performance

No performance concerns. The new useEffect is properly scoped and cleaned up.

---

📋 Recommendations

Must Fix:

1. ✏️ Update the outdated comment in constants.ts (line 117)

Should Consider:

2. ♿ Enhance accessibility of dual-line drawer header
3. 🎨 Verify WCAG contrast ratios for new hover states (especially amber)

Nice to Have:

4. 📏 Extract image size constants
5. 🧪 Add test for click-outside with image overlay

---

🎯 Overall Assessment

Recommendation: Approve with minor changes

This PR delivers meaningful UX improvements with solid test coverage. The click-outside fix addresses a real usability issue, and the visual enhancements improve the overall experience.

The identified issues are minor and mostly preventative (accessibility, documentation consistency). The core functionality is sound.

Estimated Risk: Low
Test Coverage: Good (4 new tests, all passing)
Breaking Changes: None (visual changes only)

---

📝 Action Items for Author

1. Update comment in constants.ts (line 117) to reflect new default width
2. Verify WCAG contrast ratios for hover states (particularly amber combinations)
3. Consider accessibility improvements for drawer header (optional but recommended)

Great work! The UX improvements are well thought out. 🎉

[claude]

Code Review: PR #244 - Citation Component UX Improvements

Thank you for this comprehensive UX enhancement PR! The changes improve the citation component's usability and visual feedback. Here's my detailed review:

---

✅ Strengths

1. Desktop Click-Outside Handler (CitationComponent.tsx)

• Well-implemented: The separation of desktop (mousedown) and mobile (touchstart) handlers is architecturally sound
• Good documentation: Clear inline comments explain the rationale for bypassing grace period
• Proper cleanup: Event listeners are correctly removed in the useEffect cleanup function
• Correct event handling: Using capture phase prevents event bubbling issues

2. Test Coverage (CitationComponentBehavior.test.tsx)

• Comprehensive: 4 new tests cover the main interaction scenarios
• Good mocking: Desktop environment properly mocked with matchMedia
• Clear structure: Well-organized test suite with descriptive test names

3. High-Contrast Hover States (CitationVariants.tsx)

• Accessibility win: Inverted color schemes (dark bg/white text) provide better visual feedback
• Consistent: Applied uniformly across all status types (verified, partial, pending, error)

4. Enhanced Drawer Header (CitationDrawer.tsx)

• Better UX: Dual-line display separates page title from domain
• Accessibility: Added comprehensive aria-label with source name, domain, and citation count

5. Increased Popover Width

• Better content display: 384px → 480px provides more room for verification details
• Consistent updates: Constants properly updated and used throughout

---

⚠️ Concerns & Suggestions

1. Potential Click Event Conflicts (CitationComponent.tsx:1968-1998)

Issue: The desktop click-outside handler uses mousedown with capture phase, which might interfere with other click handlers on the page.

// Current implementation
document.addEventListener("mousedown", handleOutsideClick, {
  capture: true,
});

Suggestion: Consider adding a check to ensure the popover is actually open before attaching the listener, or use a more defensive approach:

// Only attach listener when popover is open
useEffect(() => {
  if (isMobile || \!isHovering) return;
  // ... existing handler code
}, [isMobile, isHovering]); // ✓ Already correctly scoped

Status: Actually, looking closer, this is already correctly scoped to only attach when isHovering is true. Good work! ✓

2. Missing Accessibility Consideration (CitationVariants.tsx:224-232)

Issue: The high-contrast hover states change text color to white/dark, which is great visually, but there's no indication if this meets WCAG AA contrast ratios.

Suggestion: Verify contrast ratios:

• bg-amber-700 (#b45309) + white text → Should pass WCAG AA
• dark:bg-amber-200 (#fde68a) + text-amber-900 (#78350f) → Should pass WCAG AA

Action: Add a comment or test confirming WCAG AA compliance for these color combinations.

3. Default Behavior Change (CitationVariants.tsx:321)

Issue: Changing hideBrackets default from false to true is a breaking change for existing users who expect brackets in superscript citations.

// Before: [1]
// After:  1
hideBrackets = true,  // Changed from false

Suggestion:

• Option A: Document this as a breaking change in the PR description and changelog
• Option B: Keep default as false and let users opt into the cleaner look with hideBrackets={true}
• Option C: Add a version bump (minor or major depending on semantic versioning policy)

Recommendation: I'd suggest documenting this as a breaking change and ensuring it's mentioned in release notes.

4. Magic Numbers (constants.ts:123-127)

Issue: The new image size constraints use viewport-relative values without explanation:

export const VERIFICATION_IMAGE_MAX_WIDTH = "min(70vw, 480px)";
export const VERIFICATION_IMAGE_MAX_HEIGHT = "min(50vh, 360px)";

Suggestion: Add comments explaining the rationale:

// 70vw ensures image doesn't exceed viewport on mobile, 480px caps it on desktop
export const VERIFICATION_IMAGE_MAX_WIDTH = "min(70vw, 480px)";
// 50vh prevents tall images from pushing content off-screen
export const VERIFICATION_IMAGE_MAX_HEIGHT = "min(50vh, 360px)";

5. Test Gap (CitationComponentBehavior.test.tsx:2643-2645)

Issue: The comment mentions image overlay protection testing is complex and "covered by integration tests," but there's no test in this file.

// Note: Testing the image overlay protection is complex...
// This is covered by integration tests and visual testing.

Suggestion: Either:

• Add a test here that verifies isAnyOverlayOpenRef prevents dismissal, OR
• Link to the specific integration test file/line that covers this scenario

6. Hover State Color Intensity (CitationVariants.tsx:226-231)

Minor concern: The new hover states are very high contrast (700 weight backgrounds). While this is bold and clear, it might feel jarring compared to the previous subtle hover states.

Suggestion: Consider A/B testing or gathering user feedback on the hover intensity. You might want to provide a middle ground option (e.g., 600 weight backgrounds with adjusted text colors).

---

🔒 Security Considerations

• ✓ No XSS vulnerabilities introduced
• ✓ Event handlers properly cleaned up (no memory leaks)
• ✓ No unsafe DOM manipulation

---

🎯 Performance Considerations

• ✓ Event listeners properly scoped to prevent unnecessary re-renders
• ✓ useMemo used for computed class names (good optimization)
• ⚠️ Minor: Two separate document-level event listeners (touchstart + mousedown) when only one is active at a time. This is fine, but be aware of the overhead if many citations are rendered simultaneously.

Suggestion: If performance becomes an issue with many citations, consider delegating to a single global handler via a context provider.

---

📝 Documentation & Code Quality

• ✓ Excellent inline documentation in CitationComponent.tsx
• ✓ Clear commit message and PR description
• ✓ Updated Playwright tests to match new behavior
• ⚠️ Missing changelog entry for the breaking change (hideBrackets default)

---

🧪 Testing Recommendations

1. Add tests for:

   • Image overlay protection during click-outside
   • Contrast ratio verification for hover states
   • Responsive behavior at different viewport sizes (for the new popover width)

2. Manual testing checklist:

   • Test on mobile devices (iOS Safari, Android Chrome)
   • Test keyboard navigation (does click-outside interfere with keyboard users?)
   • Test with screen readers (verify aria-label changes are announced)
   • Test with multiple citations on the same page (no event listener conflicts)

---

📊 Summary

Overall Assessment: ⭐⭐⭐⭐☆ (4/5)

This is a solid PR that improves UX with thoughtful changes. The main concerns are:

1. Breaking change (hideBrackets default) needs documentation
2. Color contrast should be verified for WCAG compliance
3. Test coverage gap for image overlay protection

Recommendation: ✅ Approve with minor changes

Action Items:

1. Document the hideBrackets breaking change
2. Add WCAG contrast verification (comment or test)
3. Add test or reference for image overlay protection
4. Consider adding JSDoc comments for the new constants

Great work overall! The desktop click-outside handler is particularly well-implemented. 🎉