feat(arc): add great circle path mode for arcs - #73
Conversation
Add a `path` prop to render arcs as true great circles, document the greatCircle mode with a globe example, and reuse the Basic Arc routes in that example.
|
@jabahm is attempting to deploy a commit to the mapcn oss program Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthrough
ChangesMapArc great-circle paths
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant MapArc
participant buildArcCoordinates
participant GeoJSON
MapArc->>buildArcCoordinates: pass path and arc endpoints
buildArcCoordinates->>GeoJSON: generate Bézier or great-circle coordinates
MapArc->>GeoJSON: update geometry when path changes
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
src/registry/map.tsx (1)
1712-1714: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse a single-line guard.
Use a concise conditional for this single return statement.
As per coding guidelines, “Avoid unnecessary curly braces in conditionals; use concise syntax for simple statements.”
🤖 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 `@src/registry/map.tsx` around lines 1712 - 1714, Replace the multi-line greatCircle conditional in the surrounding coordinate-building logic with a single-line guard that directly returns buildGreatCircleCoordinates(from, to, samples), preserving the existing condition and behavior.Source: Coding guidelines
src/app/(main)/docs/_components/examples/great-circle-arc-example.tsx (2)
9-23: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePlace the exported component before static content.
Move
GreatCircleArcExamplebefore the static data declarations. Keep the static data after helpers.As per coding guidelines, “Structure files: exported component → subcomponents → helpers → static content → types.”
🤖 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 `@src/app/`(main)/docs/_components/examples/great-circle-arc-example.tsx around lines 9 - 23, Move the exported GreatCircleArcExample component declaration before the static hub, destinations, and arcs declarations, while keeping those static data declarations after any helper functions in the file. Preserve the component’s existing behavior and references.Source: Coding guidelines
25-53: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the required visual tokens and responsive rules.
Replace the hardcoded orange and white colors with semantic monochrome tokens. Use a
0.625remradius for the label. Replace the fixed height with mobile-first height utilities. Add the requiredfade-up,fade-in, orscale-inanimation with 100 ms marker delays.As per coding guidelines, “Use base radius of
0.625remwith computed variants”, “Use animations:fade-up,fade-in,scale-inwith staggered delays (100ms intervals)”, and “Use theme tokens defined as CSS custom properties inglobals.css(oklch color space) — no hardcoded colors, always use semantic tokens.”🤖 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 `@src/app/`(main)/docs/_components/examples/great-circle-arc-example.tsx around lines 25 - 53, Update the great-circle example’s Map, MapArc, and MapMarker elements to replace hardcoded orange/white colors with semantic monochrome theme tokens, apply the 0.625rem base radius to labels, and change the fixed 420px container height to mobile-first responsive height utilities. Add the required fade-up, fade-in, or scale-in animation and stagger marker animations with 100ms delays, including the hub and mapped destinations.Source: Coding guidelines
🤖 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 `@src/app/`(main)/docs/arcs/page.tsx:
- Around line 70-75: Correct the prose accompanying the DocsCode example for
path="greatCircle": describe it as a spherical shortest-path approximation with
poleward curvature, without claiming it models an aircraft’s actual route or
passes over Greenland. Keep the example’s technical behavior unchanged.
In `@src/registry/map.tsx`:
- Around line 1667-1673: Normalize samples through a shared finite,
maximum-bounded segment-count helper before generating coordinates, and reuse
that count in both path-generation modes so Infinity and NaN cannot produce
non-terminating or one-coordinate results. Apply the source change around the
visible segment calculation and regenerate the embedded registry source at
src/registry/map.tsx lines 1667-1673 and public/r/map.json line 17 with the same
normalizer.
---
Nitpick comments:
In `@src/app/`(main)/docs/_components/examples/great-circle-arc-example.tsx:
- Around line 9-23: Move the exported GreatCircleArcExample component
declaration before the static hub, destinations, and arcs declarations, while
keeping those static data declarations after any helper functions in the file.
Preserve the component’s existing behavior and references.
- Around line 25-53: Update the great-circle example’s Map, MapArc, and
MapMarker elements to replace hardcoded orange/white colors with semantic
monochrome theme tokens, apply the 0.625rem base radius to labels, and change
the fixed 420px container height to mobile-first responsive height utilities.
Add the required fade-up, fade-in, or scale-in animation and stagger marker
animations with 100ms delays, including the hub and mapped destinations.
In `@src/registry/map.tsx`:
- Around line 1712-1714: Replace the multi-line greatCircle conditional in the
surrounding coordinate-building logic with a single-line guard that directly
returns buildGreatCircleCoordinates(from, to, samples), preserving the existing
condition and behavior.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: e03c5f95-dd14-475b-9fbb-ba97bb8394eb
📒 Files selected for processing (5)
public/r/map.jsonsrc/app/(main)/docs/_components/examples/great-circle-arc-example.tsxsrc/app/(main)/docs/api-reference/page.tsxsrc/app/(main)/docs/arcs/page.tsxsrc/registry/map.tsx
| <DocsCode>path="greatCircle"</DocsCode> to render the true | ||
| shortest path over the sphere (the orthodrome) instead. This is the | ||
| route an aircraft actually flies, so long-haul connections bend | ||
| toward the poles — the same London hub as above, but a London → New | ||
| York flight now arcs up over Greenland instead of straight across the | ||
| Atlantic. In this mode{" "} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Correct the aircraft-route description.
path="greatCircle" renders a spherical shortest-path approximation. It does not model the route an aircraft actually flies. The London-to-New York great circle does not pass over Greenland. Describe the path as poleward curvature, or use a route that demonstrates the stated geography.
🤖 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 `@src/app/`(main)/docs/arcs/page.tsx around lines 70 - 75, Correct the prose
accompanying the DocsCode example for path="greatCircle": describe it as a
spherical shortest-path approximation with poleward curvature, without claiming
it models an aircraft’s actual route or passes over Greenland. Keep the
example’s technical behavior unchanged.
| const segments = Math.max(2, Math.floor(samples)); | ||
|
|
||
| // Degenerate cases: coincident points (d ~ 0) or near-antipodal points | ||
| // (sin(d) ~ 0, great circle not unique). Slerp is undefined here, so fall | ||
| // back to the unwrapped straight segment. Guarantees no NaN. | ||
| if (d < 1e-9 || sinD < 1e-9) { | ||
| return [from, [lng2, lat2]]; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Normalize samples before generating coordinates.
Infinity makes segments infinite, so the coordinate loop never terminates. NaN skips the loop and returns a one-coordinate LineString. Normalize to a finite, bounded segment count and reuse it for both path modes.
src/registry/map.tsx#L1667-L1673: add a shared finite and maximum-bounded segment normalizer, then use it in both coordinate generators.public/r/map.json#L17-L17: regenerate the embedded registry source with the same normalizer.
Proposed fix
+const MAX_ARC_SAMPLES = 512;
+
+function getArcSegments(samples: number) {
+ const finiteSamples = Number.isFinite(samples)
+ ? samples
+ : DEFAULT_ARC_SAMPLES;
+ return Math.min(MAX_ARC_SAMPLES, Math.max(2, Math.floor(finiteSamples)));
+}
+
- const segments = Math.max(2, Math.floor(samples));
+ const segments = getArcSegments(samples);📍 Affects 2 files
src/registry/map.tsx#L1667-L1673(this comment)public/r/map.json#L17-L17
🤖 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 `@src/registry/map.tsx` around lines 1667 - 1673, Normalize samples through a
shared finite, maximum-bounded segment-count helper before generating
coordinates, and reuse that count in both path-generation modes so Infinity and
NaN cannot produce non-terminating or one-coordinate results. Apply the source
change around the visible segment calculation and regenerate the embedded
registry source at src/registry/map.tsx lines 1667-1673 and public/r/map.json
line 17 with the same normalizer.
Summary
Adds a
pathprop toMapArcso arcs can be rendered as true great circles (the shortest path over the globe), alongside the existing curved style.pathprop onMapArcwith agreatCirclemode/docs/arcswith a globe example (reusing the Basic Arc routes)pathpropTest plan
/docs/arcsrenders the Great Circle example, no console errorsgreatCirclepath modeSummary by CodeRabbit
New Features
Documentation
MapArcAPI reference with the newpathoption and behavior details.