Skip to content

refactor(geo-map): externalize theme control and remove style indirection#145

Open
petekp wants to merge 3 commits intomainfrom
codex/geo-map-inherit-theme
Open

refactor(geo-map): externalize theme control and remove style indirection#145
petekp wants to merge 3 commits intomainfrom
codex/geo-map-inherit-theme

Conversation

@petekp
Copy link
Collaborator

@petekp petekp commented Feb 26, 2026

Summary

  • remove internal GeoMap auto-theme resolution and require explicit light | dark theme input
  • simplify default behavior to light and document that apps should pass their already-resolved theme
  • remove geo-map-styles.ts indirection by inlining popup/tooltip class hooks
  • clean stale GeoMap README references to removed internals

Why

Tool UI components are copy-paste surfaces, so this keeps GeoMap lighter and easier to reason about:

  • no internal theme hook state/logic
  • fewer files and imports
  • clearer ownership: app-level theme resolution stays outside the component

Validation

  • pnpm test lib/tests/tool-ui/geo-map (35 passing)
  • pre-push checks passed (pnpm lint, pnpm lint:format, pnpm registry:check)
  • note: local pnpm typecheck currently reports unrelated missing module types under packages/agent on this checkout (@clack/prompts, tsup)

Context

Follow-up after merged PR #144 to land the lightweight/theming polish commits that were not included in main.

@vercel
Copy link

vercel bot commented Feb 26, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
tool-ui Ready Ready Preview, Comment Feb 26, 2026 4:34am

Request Review

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 751a869551

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

viewport,
showZoomControl = true,
theme = "auto",
theme = "light",

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Keep dark-mode tiles when theme prop is omitted

Changing the default to theme = "light" causes every existing GeoMap usage that omits theme to render light tiles in dark mode. I checked app/docs/gallery/page.tsx (<GeoMap id="gallery-geo-map" {...geoMapPresets.fleet.data} />) and lib/docs/preview-config.tsx (<DynamicGeoMap id={...} {...geoMapData} />), and both paths now lose dark-mode tile behavior that previously came from auto theme resolution. Please either preserve automatic fallback when theme is undefined or update these consumers to pass the app-resolved theme explicitly.

Useful? React with 👍 / 👎.

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