Implement the 020-fix-zero-hp-opacity feature that replaces container-level opacity dimming with element-level opacity on individual leaf elements so that HP popover and condition picker render at full opacity for unconscious combatants
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
84
specs/020-fix-zero-hp-opacity/research.md
Normal file
84
specs/020-fix-zero-hp-opacity/research.md
Normal file
@@ -0,0 +1,84 @@
|
||||
# Research: Fix Zero-HP Opacity
|
||||
|
||||
**Feature**: 020-fix-zero-hp-opacity
|
||||
**Date**: 2026-03-06
|
||||
|
||||
## Root Cause Analysis
|
||||
|
||||
### Decision: The bug is caused by CSS `opacity` inheritance on the row container
|
||||
|
||||
**Rationale**: In `combatant-row.tsx:312`, the class `opacity-50` is applied to the outermost `<div>` when the combatant's HP status is `"unconscious"`. CSS `opacity` creates a new stacking context and affects all descendant elements — it cannot be overridden by child elements setting their own opacity. The `HpAdjustPopover` and `ConditionPicker` are rendered as children of this container (positioned `absolute` within it), so they inherit the 50% opacity.
|
||||
|
||||
**Alternatives considered**:
|
||||
- **Portal-based rendering**: Move popouts to a React portal outside the row container. This would work but is over-engineered for this fix and would break the current relative positioning pattern used by both components.
|
||||
- **CSS `filter: opacity()`**: Same inheritance problem as the `opacity` property.
|
||||
|
||||
## Fix Approach
|
||||
|
||||
### Decision: Replace `opacity-50` on the row container with scoped opacity on row content elements only
|
||||
|
||||
**Rationale**: Instead of making the entire row container semi-transparent, apply the dimming effect only to the static content elements within the row (the grid row with name/initiative/HP/AC and the condition tags area). The popout/dropdown elements sit outside this visual flow and should not be dimmed. This can be achieved by:
|
||||
|
||||
1. Removing `opacity-50` from the outer container.
|
||||
2. Adding a wrapper `<div>` (or applying directly to the existing grid `<div>` and conditions `<div>`) with `opacity-50` when unconscious — but **not** wrapping the popover/picker elements.
|
||||
|
||||
However, since the `HpAdjustPopover` is rendered inside `ClickableHp` which is inside the grid, and `ConditionPicker` is rendered inside the conditions container, the cleanest approach is:
|
||||
|
||||
- Apply `opacity-50` to the inner grid and conditions container directly rather than the outer wrapper.
|
||||
- Since the popovers are absolutely positioned children of these containers, they'll still inherit opacity. The actual fix needs to use a different visual treatment that doesn't cascade.
|
||||
|
||||
### Decision: Use text/color-based dimming instead of CSS opacity
|
||||
|
||||
**Rationale**: Replace the `opacity-50` approach with `text-muted-foreground` (or similar muted color classes) applied to the row's static text elements. This achieves the same visual "dimmed" effect for the unconscious state without using CSS `opacity`, which unavoidably cascades to absolutely-positioned children.
|
||||
|
||||
Alternatively, apply `opacity-50` specifically to individual leaf elements (initiative input, name, AC display, HP display, condition tags) rather than a container. However, the simplest single-point fix is:
|
||||
|
||||
- Keep `opacity-50` on the row container but add `opacity-100` (via a utility class) to the popover/picker wrapper elements. Since CSS opacity on a child cannot override a parent's opacity, this won't work directly.
|
||||
|
||||
### Final Decision: Use CSS `filter` or restructure to isolate popovers
|
||||
|
||||
The simplest correct fix: move the popovers outside the opacity scope by changing the structure so the row content that gets dimmed is a separate child from the popover containers.
|
||||
|
||||
**Approach**:
|
||||
1. Remove `opacity-50` from the outer row `<div>`.
|
||||
2. Wrap the row's visible content (grid + conditions area) in a `<div>` that receives `opacity-50` when unconscious.
|
||||
3. Render `HpAdjustPopover` and `ConditionPicker` as siblings of this wrapper (still within the row for positioning context) rather than deep inside the dimmed content.
|
||||
|
||||
**Rejected** for being too invasive to component structure. The popover is deeply nested inside `ClickableHp`.
|
||||
|
||||
### Actual Simplest Fix: Use `text-opacity` / color-based dimming
|
||||
|
||||
Replace the container-level `opacity-50` with Tailwind's color-based dimming:
|
||||
- `text-muted-foreground/50` for text elements
|
||||
- Or simply use a lighter text color and reduced border opacity
|
||||
|
||||
**But**: This changes more styling rules than necessary and may not look identical.
|
||||
|
||||
### Pragmatic Fix: Apply `opacity-50` to the grid and conditions wrapper, and reset opacity on popover elements using isolation
|
||||
|
||||
Since CSS `opacity` on a parent always affects children, the real fix is to **not apply `opacity` to any ancestor of the popovers**. The most surgical approach:
|
||||
|
||||
1. Remove `opacity-50` from the outer `<div>`.
|
||||
2. Add `opacity-50` to the inner grid `<div>` (line 316).
|
||||
3. Add `opacity-50` to the conditions container `<div>` (line 388).
|
||||
4. The `HpAdjustPopover` (absolutely positioned, `z-10`) and `ConditionPicker` (also absolutely positioned) are children of elements **within** these containers — they'll still be affected.
|
||||
|
||||
**Final approach**: The only way to truly isolate is to ensure popovers aren't DOM children of dimmed elements. Use a two-sibling layout within the `relative` containers:
|
||||
|
||||
For `ClickableHp`: The popover is already inside a `<div className="relative">`. We can't change that without moving the component.
|
||||
|
||||
### Conclusion: Simplest correct approach
|
||||
|
||||
Apply `opacity-50` to individual non-interactive display elements within the row rather than any container that has popover children. Specifically:
|
||||
|
||||
1. Remove `opacity-50` from the outer row container (line 312).
|
||||
2. When `status === "unconscious"`, pass a prop or apply conditional classes to:
|
||||
- The initiative input wrapper
|
||||
- The name display
|
||||
- The AC display
|
||||
- The HP number display (the button text, not the popover)
|
||||
- The condition tags
|
||||
- The remove button
|
||||
3. Popovers and pickers remain unaffected since they're not inside any dimmed container.
|
||||
|
||||
This is the most targeted fix with minimal structural changes.
|
||||
Reference in New Issue
Block a user