fix: enable glossary term tooltips on mobile via tap interaction

[sonyccd]

Summary

• Fixes glossary term tooltips not working on mobile devices
• On mobile, tapping a highlighted term now shows the definition popover
• Tapping elsewhere or another term dismisses the popover
• Desktop hover behavior remains unchanged

Problem

Highlighted glossary terms in practice questions show definitions via tooltips on desktop (hover), but on mobile there was no way to view definitions because touch devices don't have hover states.

Solution

Created a responsive GlossaryTermTooltip wrapper component that:

• Desktop: Uses Radix UI Tooltip with existing hover behavior
• Mobile: Uses Radix UI Popover with tap-to-show, tap-away-to-dismiss behavior
• Uses context to ensure only one definition popover is open at a time

Test plan

• Desktop: hover over highlighted terms in Random Practice → tooltip appears
• Mobile (DevTools device emulation): tap highlighted term → popover appears
• Mobile: tap elsewhere → popover closes
• Mobile: tap another term → first closes, new one opens
• Verify both light/dark themes work correctly

🤖 Generated with Claude Code

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Review	Updated (UTC)
openhamprep	Ready	Preview, Comment	Jan 14, 2026 3:09pm

[supabase]

Updates to Preview Branch (fix/mobile-glossary-tooltips) ↗︎

Deployments	Status	Updated
Database	✅	Wed, 14 Jan 2026 15:09:32 UTC
Services	✅	Wed, 14 Jan 2026 15:09:32 UTC
APIs	✅	Wed, 14 Jan 2026 15:09:32 UTC

Tasks are run on every commit but only new migration files are pushed.
Close and reopen this PR if you want to apply changes from existing seed or migration files.

Tasks	Status	Updated
Configurations	✅	Wed, 14 Jan 2026 15:09:33 UTC
Migrations	✅	Wed, 14 Jan 2026 15:09:34 UTC
Seeding	✅	Wed, 14 Jan 2026 15:09:34 UTC
Edge Functions	✅	Wed, 14 Jan 2026 15:09:38 UTC

---

View logs for this Workflow Run ↗︎.
Learn more about Supabase for Git ↗︎.

[claude]

Pull Request Review: Mobile Glossary Term Tooltips

Overview

This PR successfully addresses mobile accessibility for glossary term definitions by implementing a responsive wrapper that uses Popover on mobile (tap-to-show) and Tooltip on desktop (hover). The solution is well-architected with clean separation of concerns.

---

✅ Strengths

1. Excellent Architectural Design

• Context-based state management (GlossaryTermProvider) ensures only one popover is open at a time - great UX
• Clean abstraction with GlossaryTermTooltip wrapper component
• Shared GlossaryTermContentInner component eliminates duplication and ensures consistent styling

2. Responsive Design Pattern

• Proper use of useIsMobile() hook for device detection
• Leverages existing Radix UI primitives correctly
• Maintains desktop hover behavior unchanged (good for backwards compatibility)

3. Code Quality

• Follows project conventions from CLAUDE.md
• Uses semantic tokens (text-primary, text-popover-foreground) correctly
• Clean component composition with proper TypeScript typing

4. Minimal Changes

• Surgical modification to GlossaryHighlightedText.tsx (refactor, not rewrite)
• Avoids over-engineering - solves the exact problem stated

---

⚠️ Issues & Recommendations

1. Critical: Test Coverage Missing

Severity: High

The PR introduces a new component (GlossaryTermTooltip.tsx) but doesn't update the existing test suite. The test file GlossaryHighlightedText.test.tsx will likely fail or not cover the new behavior.

Recommendation:

// Add to GlossaryHighlightedText.test.tsx or create GlossaryTermTooltip.test.tsx

// Mock useIsMobile
vi.mock('@/hooks/use-mobile', () => ({
  useIsMobile: vi.fn(() => false), // default to desktop
}));

describe('GlossaryTermTooltip', () => {
  it('renders Tooltip on desktop', () => {
    vi.mocked(useIsMobile).mockReturnValue(false);
    // Test tooltip rendering
  });

  it('renders Popover on mobile', () => {
    vi.mocked(useIsMobile).mockReturnValue(true);
    // Test popover rendering with tap interactions
  });

  it('closes other popovers when opening a new one on mobile', () => {
    // Test context state management
  });
});

2. Potential: SSR/Hydration Issue

Severity: Medium

The useIsMobile() hook returns undefined initially (line 6 of use-mobile.tsx), then updates after first render. This could cause hydration mismatches if the app uses SSR.

Current behavior:

• First render: isMobile = false (due to !!undefined = false)
• After effect: isMobile updates to actual value
• This may cause a flash of desktop UI on mobile

Recommendation:
Consider handling the undefined state explicitly:

// In GlossaryTermTooltip.tsx
const isMobile = useIsMobile();

// Option 1: Show nothing until determined
if (isMobile === undefined) {
  return children;
}

// Option 2: Default to mobile-safe behavior
const isMobile = useIsMobile() ?? false; // or true for mobile-first

3. Accessibility: Missing ARIA Attributes

Severity: Low-Medium

The trigger elements have role="button" and tabIndex={0} in the child span, but the Radix primitives may override or not properly merge these attributes.

Recommendation:
Verify that:

• Keyboard navigation works (Tab to focus, Enter/Space to activate on mobile)
• Screen readers announce the interactive nature of terms
• Test with NVDA/JAWS/VoiceOver

4. UX: Popover Dismiss Behavior

Severity: Low

The implementation allows tapping outside to dismiss, but this isn't explicitly mentioned in the code. Radix Popover defaults to dismissing on outside click, which is good, but worth documenting.

Recommendation:
Add a comment or ensure it's in the test plan:

// Popover automatically dismisses on outside click via Radix default behavior

5. Performance: Context Re-renders

Severity: Low

The GlossaryTermProvider wraps all glossary text, potentially causing re-renders when openTermId changes. This is likely negligible but worth monitoring.

Observation:
For pages with many glossary terms, consider using useMemo or React.memo if performance becomes an issue. Current implementation is fine for now.

6. Styling: Popover Width Inconsistency

Severity: Very Low

Desktop tooltip uses max-w-xs (320px), but mobile popover uses w-auto max-w-xs with additional padding overrides (px-3 py-1.5). The base PopoverContent has w-72 (288px) and p-4 by default.

Current styles:

• Tooltip: max-w-xs text-sm + base padding from UI component
• Popover: w-auto max-w-xs px-3 py-1.5 text-sm (overrides base w-72 p-4)

Recommendation:
Verify that both render with similar visual appearance across themes. The padding difference might cause slight visual inconsistency.

---

🧪 Testing Recommendations

Per the PR description, the test plan should include:

1. ✅ Desktop hover behavior (unchanged)
2. ✅ Mobile tap-to-show behavior
3. ✅ Mobile tap-away dismissal
4. ✅ Mobile single popover constraint
5. ✅ Light/dark theme verification
6. Add: Keyboard navigation on mobile
7. Add: Screen reader testing
8. Add: Verify tests pass: npm test

---

🔒 Security Considerations

✅ No security concerns identified

• No user input is rendered unsafely
• Glossary terms come from the database (already sanitized)
• Uses React's built-in XSS protection
• No external data sources or new API calls

---

📊 Performance Considerations

✅ Minimal performance impact

• Context state is lightweight (single string ID)
• useIsMobile() hook uses matchMedia API efficiently
• No new network requests
• Component complexity is low

Potential optimization (future):
If many terms are on a single page, consider virtualizing or lazy-loading definitions.

---

📝 Code Style Adherence

✅ Follows CLAUDE.md conventions:

• Uses semantic color tokens ✅
• TypeScript with explicit interfaces ✅
• React functional components with arrow functions ✅
• Proper separation of concerns ✅
• Minimal over-engineering ✅

---

🎯 Final Verdict

Approval Status: ✅ Approve with minor improvements

This is a solid PR that solves the stated problem elegantly. The architecture is sound, the code is clean, and it follows project conventions. The main gap is test coverage, which should be addressed before merging.

Required before merge:

1. ⚠️ Add tests for mobile/desktop behavior
2. ⚠️ Verify existing tests pass

Nice-to-have:

3. Handle SSR hydration edge case
4. Add keyboard navigation test
5. Document dismiss behavior

Summary:

• Code Quality: A
• Architecture: A
• Test Coverage: C (needs work)
• Overall: B+ (will be A with tests)

Great work on the implementation! The responsive pattern and context management are exemplary. Just need those tests! 🎉

---

Reviewed with: Claude Code
Project conventions: /CLAUDE.md

[sentry]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.83%. Comparing base (9c7e8ef) to head (bbb4a1a).
⚠️ Report is 3 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #159      +/-   ##
==========================================
+ Coverage   86.79%   86.83%   +0.03%     
==========================================
  Files         115      116       +1     
  Lines       14521    14565      +44     
  Branches     2231     2238       +7     
==========================================
+ Hits        12603    12647      +44     
  Misses       1918     1918              

Flag	Coverage Δ
unittests	86.83% <100.00%> (+0.03%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
src/components/GlossaryHighlightedText.tsx	95.31% <100.00%> (-0.28%)	⬇️
src/components/GlossaryTermTooltip.tsx	100.00% <100.00%> (ø)

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[claude]

PR Review: Mobile Glossary Term Tooltips

Great work on addressing the mobile interaction issue! This is a well-thought-out solution that properly adapts the UX for touch devices. Here's my detailed review:

✅ Strengths

1. Clean Architecture

• The GlossaryTermTooltip component cleanly encapsulates the responsive behavior
• Proper separation of concerns with a dedicated context for managing open state
• Shared GlossaryTermContentInner component avoids duplication between Tooltip and Popover

2. Excellent Test Coverage

• Comprehensive test suite with 243 lines covering:
  • Desktop tooltip behavior
  • Mobile popover behavior
  • Single popover enforcement
  • Content styling
  • Edge cases (missing provider)
• Tests use proper mocking of useIsMobile hook

3. Follows Project Conventions

• Uses semantic color tokens (text-primary, text-popover-foreground) ✅
• Follows React patterns from CLAUDE.md (const arrow functions, explicit interfaces)
• Proper use of Radix UI primitives

4. Good UX Design

• Context ensures only one popover is open at a time on mobile (prevents clutter)
• Tap-to-toggle behavior is intuitive for mobile users
• Desktop hover behavior remains unchanged (no regression)

---

🔍 Issues & Recommendations

1. Initial Render Flash on Mobile (Low Priority)

Location: GlossaryTermTooltip.tsx:55

The comment acknowledges this:

// Note: useIsMobile returns false during SSR/initial render, then updates after hydration.
// This means mobile users briefly see Tooltip (hover) behavior before Popover activates.

Issue: Mobile users will briefly see the tooltip (which requires hover) before the popover activates. This could cause a momentary flash or incorrect interaction state.

Recommendation: Consider initializing useIsMobile with undefined and showing a loading state until hydration completes. However, looking at the useIsMobile hook (line 18), it returns !!isMobile which converts undefined to false, so this might require updating the hook itself. Given the comment suggests this is "acceptable UX," this may be fine as-is.

2. Context Robustness (Low Priority)

Location: GlossaryTermTooltip.tsx:57-64

Issue: The mobile popover uses optional chaining for context access. If the GlossaryTermProvider is missing, the popover will render but the "single popover" constraint won't work. Multiple popovers could be open simultaneously.

Recommendation: Either:

1. Make the provider required and throw a helpful error if missing
2. Add a comment explaining the degraded behavior without the provider

The test file shows this is handled (line 181-196), but the runtime behavior could be clearer.

3. Accessibility Consideration (Medium Priority)

Issue: The mobile popover doesn't appear to have explicit ARIA attributes for screen reader users. When a glossary term is clicked on mobile, screen readers should announce that a definition has been revealed.

Recommendation: Consider adding role="tooltip" and aria-label to the PopoverContent. Radix UI may handle this automatically, but it's worth verifying with a screen reader on mobile.

4. Popover Styling Discrepancy (Low Priority)

Location: GlossaryTermTooltip.tsx:73

The popover content uses px-3 py-1.5 while the default PopoverContent in ui/popover.tsx:21 uses p-4. This is intentional to keep the popover compact, which is good. Just noting for consistency—the styling is fine as-is.

5. Test Isolation (Low Priority)

Location: GlossaryTermTooltip.test.tsx:27

Observation: The tests mock useIsMobile globally, which is correct, but some tests (lines 182-196) don't explicitly set the mock value before testing. Consider ensuring every test explicitly sets the useIsMobile mock value in its beforeEach or directly in the test for clarity.

---

🔒 Security Review

✅ No security concerns identified:

• No user input is rendered without sanitization
• No XSS vulnerabilities
• No sensitive data exposure

---

⚡ Performance Review

✅ Performance looks good:

• Context state is minimal (single string)
• No unnecessary re-renders
• Proper use of React Context API
• Components are lazy-loaded via Radix UI

---

📝 Additional Observations

Positive:

1. The PR description is excellent—clear problem statement, solution, and test plan
2. Code is well-commented where it matters (hydration note)
3. Type safety is maintained throughout
4. The refactoring improved the component structure (extracted complexity from GlossaryHighlightedText)

Questions:

1. Have you tested this on an actual mobile device? DevTools device emulation is good, but touch behavior can differ on real devices (especially iOS Safari).
2. What happens if the glossary term is near the viewport edge? Radix UI should handle auto-positioning, but worth testing on mobile viewports.

---

🎯 Final Verdict

Recommendation: Approve with minor suggestions

This is a solid implementation that solves a real UX problem. The issues I've identified are minor and mostly nice-to-haves. The code is:

• Well-tested ✅
• Well-architected ✅
• Follows project conventions ✅
• Solves the stated problem ✅

Before merging:

1. ✅ Verify tests pass (see test plan in PR description)
2. ⚠️ Strongly recommend: Test on actual mobile devices (iOS Safari, Android Chrome)
3. Consider: Add a quick accessibility check with a mobile screen reader
4. Optional: Address the context robustness issue (make provider required or document fallback behavior)

Great work on this feature! 🎉