-
Notifications
You must be signed in to change notification settings - Fork 29
feat: add Satellite of the Day spotlight card #148
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,5 @@ | ||
| import React, { useEffect, useRef, useState, useCallback } from 'react'; | ||
| import React, { useEffect, useRef, useState, useCallback, useImperativeHandle, forwardRef } from 'react'; | ||
| import { prefersReducedMotion } from './SatelliteSpotlight/GlowEffect'; | ||
| import { useUIStore } from '@/store/uiStore'; | ||
| import { MaterialIcon } from './MaterialIcon'; | ||
| import { SpotlightManager } from './SatelliteSpotlight/SpotlightManager'; | ||
|
|
@@ -110,7 +111,12 @@ const LegendCardSkeleton = () => ( | |
| <div className="min-w-[200px] glass-panel p-4 animate-pulse bg-surface-container/40 h-48" /> | ||
| ); | ||
|
|
||
| export const EarthTwin: React.FC = () => { | ||
| export interface EarthTwinHandle { | ||
| /** Flies the camera to the given NORAD catalog number and opens its info panel. */ | ||
| flyToSatellite: (catalogNumber: string) => void; | ||
| } | ||
|
|
||
| export const EarthTwin = forwardRef<EarthTwinHandle>((_props, ref) => { | ||
| const containerRef = useRef<HTMLDivElement>(null); | ||
| const viewerRef = useRef<Cesium.Viewer | null>(null); | ||
| const entitiesRef = useRef<Map<string, Cesium.Entity>>(new Map()); | ||
|
|
@@ -119,6 +125,12 @@ export const EarthTwin: React.FC = () => { | |
| const tooltipRef = useRef<HTMLDivElement>(null); | ||
| const { activeSector, setSelectedSatelliteId } = useUIStore(); | ||
| const [useFallback, setUseFallback] = useState(true); | ||
| // Set by flyToSatellite(), consumed by SpotlightManager to lock its | ||
| // selection/info-card onto a satellite chosen from outside the globe | ||
| // (e.g. the Satellite of the Day card's "View on Globe" button). The | ||
| // nonce forces the effect to re-fire even if the same satellite is | ||
| // requested twice in a row. | ||
| const [focusRequest, setFocusRequest] = useState<{ catalogNumber: string; nonce: number } | null>(null); | ||
| const [hoveredObject, setHoveredObject] = useState<CatalogObject | null>(null); | ||
| const [tooltipPos, setTooltipPos] = useState({ x: 0, y: 0 }); | ||
| const [viewerInstance, setViewerInstance] = useState<Cesium.Viewer | null>(null); | ||
|
|
@@ -333,6 +345,27 @@ export const EarthTwin: React.FC = () => { | |
| } | ||
| }, []); | ||
|
|
||
| // Mirrors the double-click fly-to in useSatelliteSelection.ts, but | ||
| // triggerable from outside the globe (e.g. a dashboard card) instead of | ||
| // from a Cesium pick event. | ||
| const flyToSatellite = useCallback((catalogNumber: string) => { | ||
| const viewer = viewerRef.current; | ||
| const obj = catalogMapRef.current.get(catalogNumber); | ||
| if (!viewer || viewer.isDestroyed() || !obj) return; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When the direct NORAD lookup succeeds for an object outside EarthTwin's first 500 catalog entries, the button is enabled but Prompt To Fix With AIThis is a comment left during a code review.
Path: frontend/src/components/EarthTwin.tsx
Line: 354
Comment:
**Catalog scope breaks globe navigation**
When the direct NORAD lookup succeeds for an object outside EarthTwin's first 500 catalog entries, the button is enabled but `flyToSatellite` silently returns because the object is absent from `catalogMapRef`, causing no camera movement, selection, panel, or error feedback.
How can I resolve this? If you propose a fix, please make it concise. |
||
|
|
||
| const pos = keplerToLatLonAlt(obj); | ||
| if (pos) { | ||
| const destination = Cesium.Cartesian3.fromDegrees(pos.lon, pos.lat, pos.alt * 1000 + 2000000); | ||
| viewer.camera.flyTo({ destination, duration: prefersReducedMotion() ? 0 : 1.5 }); | ||
| } | ||
|
Comment on lines
+356
to
+360
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggestion: Severity Level: Major
|
||
|
|
||
| // SpotlightManager owns actual selection state; this just tells it | ||
| // which satellite to lock onto and open the info panel for. | ||
| setFocusRequest({ catalogNumber, nonce: Date.now() }); | ||
| }, []); | ||
|
Comment on lines
+351
to
+365
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win Queue focus requests until the globe catalog is ready.
🤖 Prompt for AI Agents |
||
|
|
||
| useImperativeHandle(ref, () => ({ flyToSatellite }), [flyToSatellite]); | ||
|
|
||
| return ( | ||
| <div className="relative w-full h-full overflow-hidden bg-bg-deep-space border-b border-border-panel"> | ||
| {/* 3D Cesium Container */} | ||
|
|
@@ -372,6 +405,7 @@ export const EarthTwin: React.FC = () => { | |
| collisionSetRef={collisionSetRef} | ||
| datasetVersion={datasetVersion} | ||
| toLatLonAlt={keplerToLatLonAlt} | ||
| focusRequest={focusRequest} | ||
| onHoverChange={(obj, pos) => { | ||
| setHoveredObject(obj); | ||
| setTooltipPos(pos); | ||
|
|
@@ -576,5 +610,7 @@ export const EarthTwin: React.FC = () => { | |
| `}</style> | ||
| </div> | ||
| ); | ||
| }; | ||
| export default EarthTwin; | ||
| }); | ||
|
|
||
| EarthTwin.displayName = 'EarthTwin'; | ||
| export default EarthTwin; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,179 @@ | ||
| import React from 'react'; | ||
| import { useQuery } from '@tanstack/react-query'; | ||
| import { MagicCard } from '@/components/ui/magic-card'; | ||
| import { MaterialIcon } from '@/components/MaterialIcon'; | ||
| import { api, type SpaceObject } from '@/services/api'; | ||
| import { useSatelliteOfTheDay } from '@/hooks/useSatelliteOfTheDay'; | ||
| import type { SatelliteStatus } from '@/constants/spotlightSatellites'; | ||
|
|
||
| const EARTH_RADIUS_KM = 6371; | ||
|
|
||
| interface OrbitSummary { | ||
| orbitType: 'LEO' | 'MEO' | 'GEO' | 'HEO'; | ||
| altitudeKm: number; | ||
| inclinationDeg: number; | ||
| periodMin: number; | ||
| } | ||
|
|
||
| /** Classifies orbit regime from semimajor axis, mirroring common LEO/MEO/GEO/HEO bands. */ | ||
| function summarizeOrbit(obj: SpaceObject): OrbitSummary | null { | ||
| if (obj.semimajor_axis == null || obj.inclination == null || obj.period == null) return null; | ||
| const altitudeKm = obj.semimajor_axis - EARTH_RADIUS_KM; | ||
|
|
||
| let orbitType: OrbitSummary['orbitType'] = 'LEO'; | ||
| if (altitudeKm > 2000 && altitudeKm < 35000) orbitType = 'MEO'; | ||
| else if (altitudeKm >= 35000 && altitudeKm <= 36500) orbitType = 'GEO'; | ||
| else if (altitudeKm > 36500) orbitType = 'HEO'; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggestion: Orbit type cannot be determined from altitude alone: every object with altitude above 36,500 km is labeled Severity Level: Major
|
||
|
|
||
| return { | ||
| orbitType, | ||
| altitudeKm: Math.round(altitudeKm), | ||
| inclinationDeg: Math.round(obj.inclination * 10) / 10, | ||
| periodMin: Math.round(obj.period), | ||
| }; | ||
| } | ||
|
|
||
| const statusColor: Record<SatelliteStatus, string> = { | ||
| Active: 'text-status-success border-status-success', | ||
| Inactive: 'text-status-warning border-status-warning', | ||
| Retired: 'text-on-surface-variant border-border-panel', | ||
| }; | ||
|
|
||
| const CardSkeleton = () => ( | ||
| <div className="p-4 space-y-3 animate-pulse"> | ||
| <div className="h-5 bg-surface-container-high rounded w-2/3" /> | ||
| <div className="h-3 bg-surface-container-high rounded w-full" /> | ||
| <div className="h-3 bg-surface-container-high rounded w-5/6" /> | ||
| <div className="grid grid-cols-2 gap-2 pt-2"> | ||
| {[0, 1, 2, 3].map(i => ( | ||
| <div key={i} className="h-10 bg-surface-container-high rounded" /> | ||
| ))} | ||
| </div> | ||
| </div> | ||
| ); | ||
|
|
||
| interface SpotlightCardProps { | ||
| /** Called with the real NORAD catalog number when "View on Globe" is clicked. */ | ||
| onViewOnGlobe: (catalogNumber: string) => void; | ||
| } | ||
|
|
||
| export const SpotlightCard: React.FC<SpotlightCardProps> = ({ onViewOnGlobe }) => { | ||
| const { satellite, isManualPick, showAnother, resetToToday } = useSatelliteOfTheDay(); | ||
|
|
||
| const liveQuery = useQuery({ | ||
| queryKey: ['catalog', 'byNorad', satellite.catalog_number], | ||
| queryFn: () => api.getCatalogObjectByNorad(satellite.catalog_number), | ||
| retry: false, | ||
| }); | ||
|
|
||
| const liveObject = liveQuery.data?.data; | ||
| const orbit = liveObject ? summarizeOrbit(liveObject) : null; | ||
| const canViewOnGlobe = liveQuery.isSuccess && orbit !== null; | ||
|
Comment on lines
+69
to
+71
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggestion: The button is enabled solely from the per-object lookup, but Severity Level: Critical 🚨- ❌ View-on-globe action silently does nothing for unloaded catalog objects.
- ⚠️ Spotlight navigation depends on an unrelated first-page limit.Steps of Reproduction ✅1. Open the dashboard, where `Dashboard.tsx:128` connects the card's `onViewOnGlobe`
callback to `earthTwinRef.current?.flyToSatellite(catalogNumber)`.
2. Let `SpotlightCard` query the selected NORAD ID through `api.getCatalogObjectByNorad()`
at `SpotlightCard.tsx:63-66`; a successful response with valid orbital fields makes
`canViewOnGlobe` true at `SpotlightCard.tsx:69-71`.
3. The globe loads only `size=500&page=1` at `EarthTwin.tsx:98-101`, then builds
`catalogMapRef` from those returned objects at `EarthTwin.tsx:144-145`. Select a valid
spotlight object that is outside this first page while its direct lookup still succeeds.
4. Click `VIEW ON GLOBE` at `SpotlightCard.tsx:164-167`. `EarthTwin.flyToSatellite()`
looks up the object at `EarthTwin.tsx:252-255` and returns when it is absent, so no camera
flight occurs and no focus request reaches `SpotlightManager`; its external selection
effect at `SpotlightManager.tsx:80-85` consequently cannot open the info panel.(Use Cmd/Ctrl + Click for best experience) Prompt for AI Agent 🤖This is a comment left during a code review.
**Path:** frontend/src/components/SatelliteOfTheDay/SpotlightCard.tsx
**Line:** 69:71
**Comment:**
*Api Mismatch: The button is enabled solely from the per-object lookup, but `EarthTwin` builds `catalogMapRef` from only the first 500 catalog entries and `flyToSatellite` returns without action when the requested object is absent. Consequently, a successful spotlight lookup can expose an enabled “View on Globe” button that neither flies the camera nor opens the info panel. Ensure the globe can resolve the object directly or disable/report the action when it is not present in the globe dataset.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix |
||
|
|
||
| return ( | ||
| <MagicCard | ||
| className="border border-border-panel rounded-xl overflow-hidden" | ||
| fillClassName="bg-surface-container/60" | ||
| gradientFrom="#00e5ff" | ||
| gradientTo="#0088ff" | ||
| > | ||
| <div className="p-4 md:p-5"> | ||
| <div className="flex items-center justify-between mb-3"> | ||
| <span className="flex items-center gap-1.5 font-label-caps text-[10px] text-primary-container"> | ||
| <MaterialIcon name="stars" className="text-sm" /> | ||
| SATELLITE OF THE DAY | ||
| </span> | ||
| <button | ||
| onClick={isManualPick ? resetToToday : showAnother} | ||
| className="px-3 py-1 border border-border-panel font-label-caps text-[10px] hover:bg-surface-container-high/50 cursor-pointer rounded" | ||
| > | ||
| {isManualPick ? "TODAY'S PICK" : 'SHOW ANOTHER'} | ||
| </button> | ||
| </div> | ||
|
|
||
| {liveQuery.isLoading ? ( | ||
| <CardSkeleton /> | ||
| ) : ( | ||
| <> | ||
| <div className="flex flex-col md:flex-row gap-4"> | ||
| <div className="w-full md:w-28 h-28 flex-shrink-0 rounded-lg overflow-hidden bg-surface-container-high flex items-center justify-center"> | ||
| {satellite.image ? ( | ||
| <img | ||
| src={satellite.image} | ||
| alt={satellite.name} | ||
| className="w-full h-full object-cover" | ||
| onError={e => { (e.currentTarget as HTMLImageElement).style.display = 'none'; }} | ||
| /> | ||
| ) : ( | ||
| <MaterialIcon name="satellite_alt" className="text-4xl text-primary-container/40" /> | ||
| )} | ||
| </div> | ||
|
|
||
| <div className="flex-1 min-w-0"> | ||
| <div className="flex flex-wrap items-center gap-2 mb-1"> | ||
| <h3 className="font-display-lg text-lg text-on-surface truncate">{satellite.name}</h3> | ||
| <span className={`px-2 py-0.5 border rounded font-label-caps text-[9px] ${statusColor[satellite.status]}`}> | ||
| {satellite.status.toUpperCase()} | ||
| </span> | ||
| </div> | ||
| <p className="text-[11px] text-on-surface-variant mb-2"> | ||
| {satellite.operator} · launched {new Date(satellite.launch_date).toLocaleDateString('en-US', { year: 'numeric', month: 'short', day: 'numeric' })} | ||
| </p> | ||
| <p className="text-[12px] text-on-surface-variant leading-relaxed"> | ||
| {satellite.mission_summary} | ||
| </p> | ||
| </div> | ||
| </div> | ||
|
|
||
| <div className="grid grid-cols-2 md:grid-cols-4 gap-2 mt-4"> | ||
| <div className="glass-panel p-2 rounded-md"> | ||
| <div className="font-label-caps text-[9px] text-on-surface-variant">ORBIT</div> | ||
| <div className="font-technical-data text-[13px] text-primary-container">{orbit?.orbitType ?? '\u2014'}</div> | ||
| </div> | ||
| <div className="glass-panel p-2 rounded-md"> | ||
| <div className="font-label-caps text-[9px] text-on-surface-variant">ALTITUDE</div> | ||
| <div className="font-technical-data text-[13px] text-primary-container">{orbit ? `${orbit.altitudeKm.toLocaleString()} km` : '\u2014'}</div> | ||
| </div> | ||
| <div className="glass-panel p-2 rounded-md"> | ||
| <div className="font-label-caps text-[9px] text-on-surface-variant">INCLINATION</div> | ||
| <div className="font-technical-data text-[13px] text-primary-container">{orbit ? `${orbit.inclinationDeg}\u00b0` : '\u2014'}</div> | ||
| </div> | ||
| <div className="glass-panel p-2 rounded-md"> | ||
| <div className="font-label-caps text-[9px] text-on-surface-variant">PERIOD</div> | ||
| <div className="font-technical-data text-[13px] text-primary-container">{orbit ? `${orbit.periodMin} min` : '\u2014'}</div> | ||
| </div> | ||
| </div> | ||
|
|
||
| {!canViewOnGlobe && !liveQuery.isLoading && ( | ||
| <p className="text-[10px] text-status-warning mt-2"> | ||
| Live orbital data isn't currently available for this object, it may have re-entered or dropped out of the tracked catalog. | ||
| </p> | ||
| )} | ||
|
Comment on lines
+147
to
+151
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win Do not report API failures as a re-entry. This branch treats 🤖 Prompt for AI Agents |
||
|
|
||
| {satellite.facts.length > 0 && ( | ||
| <ul className="mt-3 space-y-1"> | ||
| {satellite.facts.map((fact, i) => ( | ||
| <li key={i} className="flex gap-1.5 text-[11px] text-on-surface-variant"> | ||
| <MaterialIcon name="auto_awesome" className="text-[12px] text-primary-container/60 mt-0.5" /> | ||
| <span>{fact}</span> | ||
| </li> | ||
| ))} | ||
| </ul> | ||
| )} | ||
|
|
||
| <button | ||
| onClick={() => canViewOnGlobe && onViewOnGlobe(satellite.catalog_number)} | ||
| disabled={!canViewOnGlobe} | ||
| className="mt-4 w-full px-3 py-2 border border-primary-container/40 text-primary-container font-label-caps text-[11px] rounded-md hover:bg-primary-container/10 disabled:opacity-40 disabled:cursor-not-allowed cursor-pointer flex items-center justify-center gap-1.5" | ||
| > | ||
| <MaterialIcon name="public" className="text-sm" /> | ||
| VIEW ON GLOBE | ||
| </button> | ||
| </> | ||
| )} | ||
| </div> | ||
| </MagicCard> | ||
| ); | ||
| }; | ||
|
|
||
| export default SpotlightCard; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,6 +16,8 @@ interface SpotlightManagerProps { | |
| toLatLonAlt: (obj: CatalogObject) => { lat: number; lon: number; alt: number } | null; | ||
| onHoverChange?: (obj: CatalogObject | null, pos: { x: number; y: number }) => void; | ||
| onSelectionChange?: (id: string | null) => void; | ||
| /** Set to lock selection onto a satellite from outside the globe (e.g. a dashboard card). */ | ||
| focusRequest?: { catalogNumber: string; nonce: number } | null; | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -36,6 +38,7 @@ export const SpotlightManager: React.FC<SpotlightManagerProps> = ({ | |
| toLatLonAlt, | ||
| onHoverChange, | ||
| onSelectionChange, | ||
| focusRequest, | ||
| }) => { | ||
| const handlerRef = useRef<Cesium.ScreenSpaceEventHandler | null>(null); | ||
| const [handler, setHandler] = useState<Cesium.ScreenSpaceEventHandler | null>(null); | ||
|
|
@@ -71,6 +74,16 @@ export const SpotlightManager: React.FC<SpotlightManagerProps> = ({ | |
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| }, [hoveredObject, tooltipPos]); | ||
|
|
||
| // External fly-to request (e.g. Satellite of the Day's "View on Globe") | ||
| // — lock selection the same way a click would, keyed off `nonce` so the | ||
| // same satellite can be re-requested. | ||
| useEffect(() => { | ||
| if (!focusRequest) return; | ||
| const obj = catalogMapRef.current?.get(focusRequest.catalogNumber) ?? null; | ||
| if (obj) selectSatellite(obj); | ||
|
Comment on lines
+82
to
+83
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggestion: The external request selects any object found in Severity Level: Major
|
||
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| }, [focusRequest]); | ||
|
|
||
| // Keyboard focus takes priority over mouse hover for spotlight targeting, | ||
| // since it reflects deliberate keyboard navigation. | ||
| const effectiveHoverId = keyboardFocusId ?? hoveredId; | ||
|
|
@@ -149,4 +162,4 @@ export const SpotlightManager: React.FC<SpotlightManagerProps> = ({ | |
| ); | ||
| }; | ||
|
|
||
| export default SpotlightManager; | ||
| export default SpotlightManager; | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion: When the card is clicked before Cesium finishes loading its catalog, this early return discards the request entirely. Since
focusRequestis only set after this check and the later catalog refresh does not replay it, the same valid click silently does nothing and cannot select the satellite once the data becomes available. Record the request before checking readiness, then consume it when the viewer and catalog are ready. [race condition]Severity Level: Major⚠️
Steps of Reproduction ✅
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖