fix(hillshade): apply mix-blend-mode per tile to fix max-zoom dropouts (#168)

[jasoneplumb]

Summary

• The hillshade multiply blend was applied to `.hillshade-blend` (Leaflet's layer container). Its inner `.leaflet-tile-container` uses `transform: translate3d` for zoom animation, which can establish a stacking context that intermittently isolates a container-level blend from siblings — visible as the multiply effect "dropping out" at the closest zoom levels, most noticeable on the topo basemap where Esri's hillshade upscales from its z=16 native ceiling.
• Move the rule to `img.leaflet-tile` under `.hillshade-blend` so each tile image blends individually with the base map, sidestepping the container's transform-driven stacking context.

Closes #168

Why this is the right level

The Leaflet pane structure is:
```
.leaflet-tile-pane
.leaflet-layer (className: 'hillshade-blend')
.leaflet-tile-container (transform: translate3d(...))
img.leaflet-tile (each tile)
```

The transform on the middle container is what creates the isolation risk. Targeting the tile images keeps the blend on the leaf rendered content; the base layer's tile images are siblings (in the parent tile-pane stacking context) and the blend pairs with them as expected.

Test plan

• `npm run type-check` clean
• `npm run lint` clean
• `npm test` clean (628 tests pass)
• Manual verification on real device: enable Hillshade overlay with the Topographic basemap, zoom in to the closest level, pan around — hillshade should blend consistently without dropouts
• No regression on streets / cyclosm / humanitarian basemaps

🤖 Generated with Claude Code

[claude]

Code Review

Overview

This PR fixes an intermittent bug where mix-blend-mode: multiply on the hillshade layer drops out at max zoom. The root cause is correctly diagnosed: the inner .leaflet-tile-container's transform: translate3d creates a new stacking context that can isolate the container-level blend from adjacent layers. Moving the rule to img.leaflet-tile sidesteps this by blending each tile image directly in the parent pane's stacking context.

---

Code Correctness

The fix is technically sound. This is a well-known CSS compositing edge case — transform on a child establishes a stacking context that breaks out-of-flow blending with siblings. Applying mix-blend-mode at the leaf (img) level is the right workaround.

One alternative worth knowing about: adding isolation: isolate to the parent .leaflet-tile-pane could also prevent the stacking context from escaping. However, that would change blending behavior across all layers in the pane, not just hillshade. The tile-image approach is more surgical and correct.

Minor concern: .leaflet-tile is a Leaflet-internal class. If Leaflet renames it in a future version this will silently break. The risk is low (Leaflet 1.x has been stable here for years), but worth a note in the project's Leaflet upgrade checklist if one exists.

---

Inline Comment Weight

The added comment block (3 lines) is heavier than CLAUDE.md's convention of a single brief line only when the WHY is non-obvious. The stacking context issue is non-obvious, so a comment is justified — but the detail level here duplicates the PR description. Consider trimming to a single line:

/* Each tile blends individually — a container-level blend isolates under translate3d zoom animation. */
.hillshade-blend img.leaflet-tile { mix-blend-mode: multiply; }

---

Test Checklist Discrepancy

The PR description checks off:

npm test clean (628 tests pass)

Per CLAUDE.md, this project has no test framework and no test script in package.json. Either the checklist was copied from a template without adjustment, or package.json has been updated since CLAUDE.md was written. Either way, this item should be corrected — if tests don't exist, remove the checkbox; if they do, update CLAUDE.md to reflect it.

---

Summary

Area	Status
Bug fix correctness	✅ Correct approach
CSS selector specificity	✅ More targeted than before
Inline comments	⚠️ Trim to one line per project conventions
Test checklist	⚠️ npm test claim contradicts CLAUDE.md — verify or remove
Manual testing	⏳ Not yet checked off — needs real-device verification at max zoom

The core change is a clean 1-line fix with solid reasoning. Addressing the checklist discrepancy and trimming the comments would bring it fully in line with project conventions.

[jasoneplumb]

Cycle-2 fixes in the latest push:

• Comment trimmed to one line per project convention.
• CLAUDE.md refreshed — npm test now listed (vitest, 628 tests), recording replaced by guidance in features, file-structure list updated with guidance.ts / routing.ts / geo.ts / layers-control.ts / consent.ts / keepalive.ts. The `No automated tests" note is gone — that was the staleness you flagged.

[claude]

Code Review

Overview

This PR fixes a visual bug (#168) where the hillshade multiply blend drops out at max zoom levels. The fix moves mix-blend-mode: multiply from the Leaflet layer container (.hillshade-blend) down to individual tile images (.hillshade-blend img.leaflet-tile). The PR also updates CLAUDE.md to reflect the current project state.

---

CSS Fix (src/style.css)

The fix is correct and the root cause analysis is accurate.

/* Before */
.hillshade-blend { mix-blend-mode: multiply; }

/* After */
.hillshade-blend img.leaflet-tile { mix-blend-mode: multiply; }

The stacking context isolation issue is a well-known CSS compositing behavior: when a descendant uses transform, it creates a new stacking context that causes mix-blend-mode on an ancestor to composite against an opaque backdrop rather than blending with true siblings. Placing the blend on the leaf <img> elements avoids this entirely.

Observations:

• The selector .hillshade-blend img.leaflet-tile is appropriately scoped — only hillshade tiles are targeted, no spillover risk.
• The updated comment (Per-tile multiply — a container-level blend can isolate under .leaflet-tile-container's translate3d.) is concise and captures the why, consistent with project conventions.
• Minor performance note: Applying mix-blend-mode to N tile images instead of one container means the GPU composites N blends rather than one. In practice, modern browsers GPU-accelerate this well and the tile count (~16–25 visible at any time) makes this negligible. Worth knowing, but not a concern here.
• No regression risk on other basemaps: The fix only affects elements under .hillshade-blend, which is only added when the hillshade overlay is active. Streets/CyclOSM/Humanitarian are unaffected.

---

CLAUDE.md Updates

The documentation updates are accurate and useful:

• New features (navigation, PWA caching, consent modal, wake lock) are correctly documented.
• The test command (npm test) and size-limit command (npm run size) are valuable additions.
• File structure is updated to reflect the current module list.
• Removing "No automated tests" and replacing it with the correct vitest + lint + type-check workflow is a good correction.
• ADR links in the architecture section are a nice addition for deeper context.

One small inconsistency to consider: the PR test plan references npm test passing 628 tests, but the CLAUDE.md test command note says "vitest for unit tests (pure-function tests; no DOM/Leaflet integration)". Both are fine as-is; just noting the manual browser verification items are unchecked — that's expected for a visual bug fix and not a blocker.

---

Summary

Area	Assessment
Correctness	✅ Fix targets the exact root cause
Code style	✅ Follows project conventions (minimal comments, targeted selectors)
Performance	✅ No meaningful overhead
Security	✅ No concerns (CSS-only change)
Test coverage	✅ Unit tests pass; manual visual check is appropriately flagged as pending

Verdict: Ready to merge after manual verification is completed. The CSS change is minimal, well-reasoned, and the documentation updates are a net positive.

🤖 Generated with Claude Code