Skip to content

library: add interactive cover banner with hover hotspots (initial state)#147

Merged
MaryWylde merged 2 commits into
devfrom
feat/library-animation
Jun 16, 2026
Merged

library: add interactive cover banner with hover hotspots (initial state)#147
MaryWylde merged 2 commits into
devfrom
feat/library-animation

Conversation

@MaryWylde

Copy link
Copy Markdown
Contributor

Initial version of the library cover animation: hovering a building reveals a glow silhouette and a glass info card, driven by percentage-based hotspot geometry over the 1440x852 design frame. Hover-by-default with a switchable click mode. Hotspot coordinates and placeholder library data are a first pass and will be refined; real library content is still to come.

…ate)

Initial version of the library cover animation: hovering a building reveals
a glow silhouette and a glass info card, driven by percentage-based hotspot
geometry over the 1440x852 design frame. Hover-by-default with a switchable
click mode. Hotspot coordinates and placeholder library data are a first
pass and will be refined; real library content is still to come.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@MaryWylde

Copy link
Copy Markdown
Contributor Author

@claude, review this PR.

@claude

claude Bot commented Jun 16, 2026

Copy link
Copy Markdown

Claude finished @MaryWylde's task in 3m 30s —— View job


PR Review: library: add interactive cover banner with hover hotspots

Tasks

  • Load tooling and repo context
  • Read changed files
  • Analyze code quality, patterns, and issues
  • Post review feedback

Overall this is a well-structured first pass — the percentage-based geometry, the mode-switch hook, and the reduced-motion handling are all solid. A few things need attention before the follow-up PRs build on this foundation.


Bugs

1. Hover mode: card is unreachable with the mouse

InteractiveCover.tsx:120 — In hover mode, onMouseLeave is attached to the .layer container, which is correct for dismissal. But onMouseLeave on the button (via useHotspotTrigger) fires close() the moment the cursor leaves the button and crosses into the card area. The card disappears before the user can read it.

The fix is to move all hover listeners to the .layer wrapper (or the hotspot group), not the individual buttons. open fires on button mouseenter; close fires on .layer mouseleave. The current architecture has the button's own onMouseLeave racing against the layer's. Fix this →

2. Cover image is missing from the repo

Home.tsx:144 references src="/assets/library/library.png" but public/assets/library/ only contains audio-cover.png and book-cover.png. The component will render a broken image on any environment. If the image is served via CDN or added in a follow-up, that should be documented here to avoid confusion.

3. Aspect ratio mismatch between SCSS and hotspot geometry

InteractiveCover.module.scss:4 locks the frame to calc(3 / 2) (1536×1024, ratio 1.500), but coverHotspots.ts:9 (JSDoc) says percentages were derived from a 1440×852 design frame (ratio ≈ 1.690). These are different aspect ratios. The frame will render at 3:2, but the hotspot coordinate system was computed at 16.9:10 — every hit box and card anchor will be offset. Confirm the actual cover image dimensions and align the SCSS aspect-ratio to match, or re-derive hotspot percentages against the 3:2 frame.


Convention violations (AGENTS.md)

4. Barrel files should be index.ts with a default re-export

AGENTS.md L106–L116 specifies:

// index.ts
import X from './X';
export default X;

Both new barrels (InteractiveCover/index.tsx, LibraryInfoCard/index.tsx) use export * from ... instead and have a .tsx extension despite containing no JSX. The existing library components (LibraryCard, AboutLibraryModal) have the same issue, but AGENTS.md explicitly says to use the correct pattern going forward.

5. InteractiveCoverProps doesn't need its own .types.ts

InteractiveCover.types.ts exports a single interface used only by InteractiveCover.tsx. AGENTS.md L121: "No .types.ts sibling unless the type is shared across multiple files." Move the interface inline above the component. (LibraryInfoCard.types.ts is fine — its LibraryInfoCardProps is imported by coverHotspots.ts.)

6. Hardcoded colors instead of design tokens

LibraryInfoCard.module.scss and InteractiveCover.module.scss both use hardcoded hex values (#ffffff, rgba(32, 54, 44, 0.35), #5b7d6f, etc.). keepsimple-style.md defines the design token layer — values like the glass panel green (#20362C) and white should reference variables from there, not be inlined.


Accessibility

7. aria-pressed vs aria-expanded in click mode

InteractiveCover.tsx:42aria-pressed describes a toggle button state (on/off). For a button that reveals associated content, aria-expanded is semantically correct. The revealed card div should also receive aria-hidden={!isActive} so screen readers skip it when collapsed.


Performance

8. will-change lists too many properties

InteractiveCover.module.scss:56:

will-change: opacity, transform, clip-path, filter;

Four promoted layers per hotspot (5 hotspots = 20 composited layers). clip-path animation in particular is expensive — it forces pixel-level recalculation every frame on some browsers. Consider whether the clip-path reveal is essential or can be replaced with a simpler transform + opacity approach.


Minor

9. backdrop-filter transition may not animate in Chrome

LibraryInfoCard.module.scss:26–27 — Transitioning backdrop-filter from blur(0) to blur(8px) is supported in Safari and Firefox but is known to drop frames or jump in Chromium. Worth testing on Chrome before this ships. The prefers-reduced-motion block correctly strips the backdrop-filter transition, so that path is clean.

10. Fixed height: 314px on the info card

LibraryInfoCard.module.scss:8 — With real library content (especially longer about text), the card will overflow. Consider min-height: 314px and letting the card grow, or add overflow: hidden intentionally if the design calls for clipping.

11. Stale closure in useHotspotTrigger

useHotspotTrigger.ts:31toggle and close read isActive/activeId from closure scope rather than using the functional updater pattern (setActiveId(prev => ...)). In React 18 concurrent mode, this can read stale state under batched updates. Low risk for now but worth fixing before click mode is wired up more broadly.

12. Misleading variable name

InteractiveCover.tsx:86 — The handler is named onPointerDown but is registered on 'mousedown'. Either rename to onMouseDown or switch to 'pointerdown' for broader input device support.


What's good

  • The useHotspotTrigger hook cleanly centralises the mode logic — flipping hover ↔ click really is one prop change.
  • prefers-reduced-motion handling in both SCSS files is thorough.
  • Percentage-based geometry is the right call; no runtime measurement is a real maintenance win.
  • The Escape / outside-click dismissal in click mode is correctly scoped to the frame ref.
  • SVG hotspot files with embedded drop-shadow filters are clean and match the glass UI aesthetic.

- Trim will-change to opacity/transform so clip-path and filter are no
  longer permanently promoted to compositor layers (4 -> 2 per hotspot).
- Let the info card grow with real content (min-height instead of a fixed
  height) and stay compact on mobile.
- Use functional state updaters in the hotspot trigger handlers to avoid
  reading a stale activeId under batched/concurrent updates.
- Rename the click-mode dismiss handler to match its mousedown listener.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@MaryWylde MaryWylde merged commit 89edc5a into dev Jun 16, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant