# Implementation Plan: Fix HP Popover Overflow

**Branch**: `024-fix-hp-popover-overflow` | **Date**: 2026-03-09 | **Spec**: [spec.md](./spec.md)
**Input**: Feature specification from `/specs/024-fix-hp-popover-overflow/spec.md`

## Summary

The HP adjustment popover (`HpAdjustPopover`) overflows the right edge of the viewport when the trigger element (HP display) is positioned near the right side of the combatant row. This causes the popover to be partially hidden and introduces a horizontal scrollbar. The fix adds viewport-aware horizontal positioning using a `useLayoutEffect` measurement pattern, consistent with the existing `ConditionPicker` component which already handles vertical overflow.

## Technical Context

**Language/Version**: TypeScript 5.8 (strict mode, verbatimModuleSyntax)
**Primary Dependencies**: React 19, Tailwind CSS v4, Lucide React (icons)
**Storage**: N/A (no storage changes — purely presentational fix)
**Testing**: Vitest
**Target Platform**: Web browser (desktop + mobile)
**Project Type**: Web application (monorepo: apps/web + packages/domain + packages/application)
**Performance Goals**: N/A (simple layout fix)
**Constraints**: No heavy positioning libraries; lightweight DOM measurement only
**Scale/Scope**: Single component fix (~20 lines changed)

## Constitution Check

*GATE: Must pass before Phase 0 research. Re-check after Phase 1 design.*

| Principle | Status | Notes |
|-----------|--------|-------|
| I. Deterministic Domain Core | PASS | No domain changes — adapter-layer only |
| II. Layered Architecture | PASS | Change is entirely in the adapter layer (web UI component) |
| III. Agent Boundary | N/A | No agent involvement |
| IV. Clarification-First | PASS | Bug fix with clear spec, no ambiguity |
| V. Escalation Gates | PASS | Scope matches spec exactly |
| VI. MVP Baseline Language | PASS | No scope restrictions involved |
| VII. No Gameplay Rules | PASS | No gameplay mechanics involved |

All gates pass. No violations.

## Project Structure

### Documentation (this feature)

```text
specs/024-fix-hp-popover-overflow/
├── plan.md              # This file
├── research.md          # Phase 0 output
└── tasks.md             # Phase 2 output (/speckit.tasks command)
```

### Source Code (repository root)

```text
apps/web/src/components/
├── hp-adjust-popover.tsx    # FIX: Add viewport-aware horizontal positioning
└── combatant-row.tsx        # Context: ClickableHp renders popover (relative parent)
```

**Structure Decision**: This is a single-component bug fix within the existing `apps/web` adapter layer. No new files, packages, or architectural changes needed.

## Design

### Root Cause

The `HpAdjustPopover` uses `absolute` positioning relative to its parent `ClickableHp` container (which has `relative` positioning). The popover renders to the right of the HP value display. When the HP display is near the right edge of the viewport (common since HP is in one of the rightmost grid columns), the popover extends beyond the viewport boundary.

The combatant list container has `overflow-y-auto` but no `overflow-x: hidden`, so the popover overflow manifests as a horizontal scrollbar on the page.

### Approach: position: fixed with useLayoutEffect viewport clamping

Switch the popover from `position: absolute` to `position: fixed` and use a `useLayoutEffect` hook to:
1. Measure the trigger element's viewport rect via `parentElement.getBoundingClientRect()`
2. Measure the popover's own width via `getBoundingClientRect()`
3. Position the popover just below the trigger (`trigger.bottom + 4`)
4. Clamp the `left` coordinate so the popover stays within viewport bounds (8px padding)
5. Render with `visibility: hidden` until positioned to avoid a flash

This approach was chosen because the popover's scroll container ancestor (`overflow-y-auto`) clips absolutely-positioned children, making `position: absolute` with `right-0` or `translateX` ineffective — the popover gets clipped rather than repositioned.

### Implementation Detail

- Replace `absolute` with `fixed` positioning on the popover container
- Add `useLayoutEffect` that reads `parentElement.getBoundingClientRect()` for the trigger position and `el.getBoundingClientRect()` for the popover width
- Compute `left = trigger.left`, clamped to `[8, vw - popover.width - 8]` using `document.documentElement.clientWidth`
- Set `top = trigger.bottom + 4`
- Store computed `{ top, left }` in state; render `visibility: hidden` until positioned
- Click-outside detection continues to work since `document.addEventListener("mousedown")` is not affected by positioning scheme

### Alternatives Considered

- **CSS-only `overflow-x: hidden`** on ancestor: Would clip the popover rather than reposition it, and could hide other legitimate content.
- **Heavy positioning libraries (Floating UI, Popper)**: Overkill for a small popover with a simple overflow case.
- **`position: absolute` with `right-0` flip** (ConditionPicker pattern): The popover's relative parent is too small (just the HP number), so `right-0` barely shifts the popover. Also, the scroll container's overflow clipping still clips the popover.
- **`position: absolute` with `translateX`**: Same clipping issue — the scroll container clips the popover regardless of transform.

### Post-Design Constitution Re-check

All gates still pass. The fix is entirely in the adapter layer, uses no domain logic, and stays within spec scope.