feat(ui): codify map-floating glass, theme-aware shadows, tighter radius - #1764
Conversation
Follow-up to #1754. Three changes, all working through tokens rather than per-component edits. Map-floating chrome ("instrument glass") - The translucent-blur treatment was already in use across 11 components, but copy-pasted with drifted values: bg-background/90 vs /95, backdrop-blur-sm vs -md vs bare backdrop-blur. Replace all of them with one `.map-glass` class so the look has a single home. - Encodes the rule the app was already half-following: chrome docked BESIDE the map (toolbar, status bar, side panels) is opaque; panels floating OVER the map are translucent, because translucency is what signals "this sits above your data". - Deliberately a plain unlayered class, not an `@utility`. It is applied next to components that already emit a bg-* utility (MapGrid's Button variant="outline" carries bg-background); as a utility both would sit in the same cascade layer and the winner would depend on Tailwind's internal sort rather than intent. - Added to MapGrid's pane controls, which float over the map but never had it. Deliberately NOT added to PixelTimeSeriesControl or the dialogs: that panel is content (a chart) rather than chrome, and dialogs sit on a dimmed overlay, not on the map. DesktopShell's file-drop scrim keeps its own bg-background/70, being a full-cover dim rather than a panel. - Falls back to 97% opaque via @supports where backdrop-filter is unavailable; at 85% with no blur, panels over live imagery are unreadable. Theme-aware shadow scale - Tailwind v4 bakes a 10%-black colour into its shadow utilities, so all ~65 shadow sites in the app were effectively invisible in dark mode and elevation rested entirely on the surface ladder from #1754. Redefine --shadow-sm/md/lg/xl against inheritable --elev-penumbra/--elev-umbra tokens that switch per theme. Fixes every existing site without touching a component. - Those tokens are plain custom properties precisely so they inherit: Tailwind registers --tw-shadow-color with `inherits: false`, so retinting shadows by setting that on :root silently does nothing. - Colours are written bare; Tailwind adds the var(--tw-shadow-color, …) wrapper itself, so `shadow-<color>` overrides keep working. Other - --radius 0.5rem -> 0.375rem. A dense tool UI reads sharper, and the derived md/sm steps stay positive. This is the most subjective change here. - One global prefers-reduced-motion guard. Two components respected the preference against ~53 transition sites; motion is reduced to a near-instant cross-fade rather than removed, since an abrupt swap is its own problem. Verified in the built app with Playwright: 5 .map-glass panels render as rgba(17,21,28,0.85) with blur(12px) saturate(1.4) and radius 4px, shadows resolve to rgba(0,0,0,0.4)/rgba(0,0,0,0.55) in dark instead of the old invisible 10% black, and the toolbar stays rgb(255,255,255) opaque in light. Not verified: backdrop-filter cost on real GPU compositing. The test browser runs SwiftShader, where blur on/off medians were identical (66.8 vs 66.7ms) at a software-bound ~15fps, so the measurement says nothing useful. Note the blur radius went from 4px to 12px plus a saturate(), so GPU cost is higher than on main — worth a look on real hardware.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe desktop map interface now uses shared ChangesMap glass theme consolidation
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Biome (2.5.6)apps/geolibre-desktop/src/index.cssFile contains syntax errors that prevent linting: Line 6: Tailwind-specific syntax is disabled.; Line 22: Tailwind-specific syntax is disabled.; Line 59: Tailwind-specific syntax is disabled.; Line 95: Tailwind-specific syntax is disabled. 🔧 ESLint
ESLint install failed: dependency version conflict. Check your lock file or package.json. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🔍 Cloudflare PR preview
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
All four inline comments posted successfully. Code reviewBugs
Quality
Security: none found. Performance: none found beyond what the author already flagged (untested CLAUDE.md adherence: The scoped-override guidance ("add scoped overrides in |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/geolibre-desktop/src/components/legend/MapLegendPanel.tsx (1)
467-473: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winKeep the legend panel at the stronger opacity.
The comment documents that
/90text washed out in dark theme over bright map content, butmap-glassapplies0.85alpha behind the backdrop filter. Add/keep a stronger legend-specific glass opacity or preserve the previous opacity while retaining the blur and saturation, then update the comment to describe the final rule.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/geolibre-desktop/src/components/legend/MapLegendPanel.tsx` around lines 467 - 473, Update the legend panel element identified by panelRef and its surrounding opacity comment so the panel uses a stronger legend-specific glass opacity than map-glass’s 0.85 alpha, while retaining the existing blur and saturation behavior. Preserve the intended dark-theme text contrast and revise the comment to accurately describe the final opacity rule.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/geolibre-desktop/src/components/layout/CollaborationStatusBadge.tsx`:
- Line 452: Preserve hover feedback for the glass controls by adding a stronger
interactive glass hover variant or moving the hover surface to a child element.
Apply the fix at CollaborationStatusBadge.tsx:452, MapGrid.tsx:168,
MapGrid.tsx:182, and MapGrid.tsx:220, ensuring each control’s hover appearance
remains visible.
In `@apps/geolibre-desktop/src/index.css`:
- Around line 54-66: Verify the performance of the .map-glass backdrop-filter on
target hardware, including several visible migrated map panels on low-end
integrated GPUs, and address any unacceptable rendering or interaction
degradation before release.
---
Outside diff comments:
In `@apps/geolibre-desktop/src/components/legend/MapLegendPanel.tsx`:
- Around line 467-473: Update the legend panel element identified by panelRef
and its surrounding opacity comment so the panel uses a stronger legend-specific
glass opacity than map-glass’s 0.85 alpha, while retaining the existing blur and
saturation behavior. Preserve the intended dark-theme text contrast and revise
the comment to accurately describe the final opacity rule.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 5eff4dcb-c4bb-4ea8-87af-96d6320fc73a
📒 Files selected for processing (13)
apps/geolibre-desktop/src/components/layout/BoundsRestrictionIndicator.tsxapps/geolibre-desktop/src/components/layout/CollaborationStatusBadge.tsxapps/geolibre-desktop/src/components/layout/MapGrid.tsxapps/geolibre-desktop/src/components/layout/MapModeBanner.tsxapps/geolibre-desktop/src/components/layout/QuickAnalysisBanner.tsxapps/geolibre-desktop/src/components/legend/MapLegendPanel.tsxapps/geolibre-desktop/src/components/panels/FlightSimulatorPanel.tsxapps/geolibre-desktop/src/components/panels/RouteAnimationPanel.tsxapps/geolibre-desktop/src/components/panels/SunPanel.tsxapps/geolibre-desktop/src/components/storymap/StoryMapComposeBar.tsxapps/geolibre-desktop/src/components/storymap/StoryMapPresenter.tsxapps/geolibre-desktop/src/index.csspackages/ui/src/globals.css
- Move .map-glass into Tailwind's utilities layer (@Utility) instead of leaving it unlayered. Unlayered CSS beats every layered declaration for the same property, and cascade-layer precedence resolves before specificity, so the unlayered rule also beat `hover:bg-accent` — killing the hover fill on MapGrid's two pane buttons and the CollaborationStatusBadge toggle. Confirmed dead in the built app before the fix (idle == hovered on all three), and restored after (idle rgba(17,21,28,0.85) -> hover rgb(46,53,66) = --accent). The @supports fallback is nested inside the utility so it stays in the layer; hoisting it out would reintroduce the same bug where backdrop-filter is unsupported. - Switch MapGrid's layers trigger from Button variant="outline" to "ghost". Inside the utilities layer, outline's own bg-background ties with map-glass on specificity and won on order, so that button rendered opaque beside its glass siblings. ghost sets no base background and still supplies hover:bg-accent; the border is restored explicitly. (bg-transparent was tried first and is worse — tailwind-merge drops bg-background, but bg-transparent then beats map-glass and the button renders fully see-through.) - Add --shadow-xs and bare --shadow to the elevation scale. Only sm/md/lg/xl were redefined, but shadow-xs is what @geolibre/ui's form primitives use (input, textarea, select, color-field, color-ramp-select) plus 7 app sites, so every input, textarea, select and colour field in the app was still on the invisible-in-dark 10% black this scale exists to fix. Verified: a real input's shadow now resolves to rgba(0,0,0,0.4) in dark mode. shadow-2xl and shadow-inner are unused and left out, with a note to extend if that changes.
Code reviewBugs
Security
Performance
Quality
CLAUDE.md
Inline comments posted:
|
🔍 GitHub Pages PR preview
|
- Exempt indeterminate progress indicators from the reduced-motion guard. The blanket `animation-iteration-count: 1` gave every spinner a single 0.01ms iteration and then froze it mid-rotation, so during a long raster/segmentation/ conversion job the UI would read as hung — a worse outcome for the same user than the abrupt transitions the guard exists to fix. 70 animate-spin sites and 6 animate-pulse were affected. Slowed rather than stopped (1.5s / 3s, still infinite): the preference targets large vestibular-triggering motion, which a small in-place spinner is not, so this keeps the "still working" signal while calming it. Verified under emulated prefers-reduced-motion: animate-spin resolves to 1.5s/infinite and animate-pulse to 3s/infinite, while a decorative `animate-in fade-in-0` is still suppressed to 0.00001s/1. - Bump Dialog from shadow-lg to shadow-xl. The scale documents shadow-xl as "modal dialog", but the shared primitive every modal renders through was still on shadow-lg — the same step as "panel floating over the map" — so the ladder's one-step-per-surface invariant was false for the one component that most literally is a modal dialog. Bumping the primitive makes the documented mapping true rather than aspirational.
|
Good, self-contained. I've reviewed all 14 changed files (component class-name migrations to Code reviewBugs: None found. The Security: No user input, no injection surface — this is CSS/className-only churn. (Confidence: high) Performance: The PR itself flags the untested Quality: Pre-existing CLAUDE.md: No violations — no i18n strings added, no RTL-sensitive physical positioning introduced, no Overall this is a clean, well-reasoned, self-documenting CSS/design-token refactor; I don't have findings that warrant inline comments. |
Follow-up to #1754 (Tier 2 + Tier 3 of the UI modernization). Everything here works through tokens rather than per-component styling, so most of it lands without editing components at all.
1. Map-floating chrome — "instrument glass"
I pitched this as adding a signature effect. That was wrong, and worth correcting: the treatment already existed in 11 components. What was missing was consistency. The recipes had drifted:
main/95,/90,/70backdrop-blur-sm×8,-md×1, barebackdrop-blur×2shadow-lg,-md,-smSo this collapses all of them into one
.map-glassclass, and encodes the rule the app was already half-following:Where it was deliberately not applied, since the boundary is the interesting part:
PixelTimeSeriesControl— that's content (a chart), not chrome. 85% translucency over busy imagery would fight the data it exists to show.DesktopShell's file-drop scrim — a full-cover dim, not a panel; keeps its own/70.Why a plain class instead of
@utility: it's applied next to components that already emit abg-*utility (MapGrid'sButton variant="outline"carriesbg-background). As a utility, both would land in the same cascade layer and the winner would depend on Tailwind's internal sort rather than intent. Unlayered CSS beats the utility layer outright. Trade-off: no variants (hover:,md:), which nothing needs.Also added an
@supportsfallback to 97% opaque — at 85% with no blur, panels over live imagery are unreadable.2. Theme-aware shadow scale
Tailwind v4 bakes a 10%-black colour into its shadow utilities. On dark surfaces that's all but invisible — meaning all ~65 shadow sites in the app did nothing in dark mode, and elevation rested entirely on the surface ladder from #1754.
Redefining
--shadow-sm/md/lg/xlagainst per-theme--elev-penumbra/--elev-umbrafixes every one of those sites without touching a single component. Each step now maps to a surface rather than being picked by eye:Two implementation notes worth a reviewer's eye:
--tw-shadow-colorwithinherits: false, so the obvious approach — retinting shadows by setting that on:root— silently does nothing.var(--tw-shadow-color, …)when generating the utility, soshadow-<color>overrides keep working. Writing the wrapper by hand just nests the fallback twice.3. Radius and reduced motion
--radius0.5rem→0.375rem. A dense tool UI reads sharper and the derived md/sm steps stay positive. This is the most subjective change in the PR — easy to drop if you disagree.prefers-reduced-motionguard. Two components respected the preference against ~53 transition sites. Motion is reduced to a near-instant cross-fade rather than removed outright, since an abrupt swap is its own accessibility problem.I also drafted motion tokens (
--duration-fast,--ease-out-quart) and removed them before committing: Tailwind only emits utilities actually used in source, so with no consumer they generated zero CSS. Unused tokens that look like a system but aren't are worse than none.Verification
npm run buildclean;npm run test:frontend5473 pass, 0 fail;pre-commiton changed files passes..map-glasspanels render asrgba(17,21,28,0.85)withblur(12px) saturate(1.4)at radius4px; shadows resolve torgba(0,0,0,0.4)/rgba(0,0,0,0.55)instead of the old invisible 10% black. In light mode--elev-umbraswitches to the cool-navy value and the toolbar staysrgb(255,255,255)— opaque docked chrome, glass floating chrome, as intended.Not verified — please check on real hardware
backdrop-filtercost under GPU compositing. I tried to measure it and the measurement is not trustworthy: the test browser runs SwiftShader (software rendering), where blur-on vs blur-off medians came out identical (66.8 vs 66.7 ms) at a software-bound ~15fps. That says nothing about GPU behaviour.This matters because the blur radius went from
backdrop-blur-sm(4px) to 12px plus asaturate(), so the per-frame compositing cost is genuinely higher than onmain— the wider blur is what keeps text legible at the lower 85% opacity. Worth a pan/zoom sanity check on a real GPU before merge; if it bites, the radius is a one-line dial inindex.css.Summary by CodeRabbit