diff --git a/CLAUDE.md b/CLAUDE.md index 89042df..e9d641a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -37,6 +37,8 @@ SceneryStack **first-person planetarium** for the night sky. Observer location, **Gotchas** - **Ctrl-drag** and **Ctrl+arrow keys** advance **sidereal time** (stars move 1:1 with gesture); civil time advances ~1/1.0027 as fast. +- Civil time is bounded by `CIVIL_TIME_MS_RANGE` (derived from `CIVIL_YEAR_RANGE`, 1900–2100). Route every write that could leave that span through `setCivilTimeMs()`; the `date` query parameter rejects out-of-range epochs rather than clamping them. +- `epochPresetProperty` only flips to `CUSTOM` once civil time is a full minute off the preset — the timer starts playing, so a tighter test would mark it CUSTOM on the first frame and the combo could never show a preset. - Reset All restores model Properties but **not** preference-backed overlays (`showStarLabels`, `showConstellations`, `showPlanetLabels`, `deepStarCatalog`). - Planet positions use `astronomy-engine` only through `PlanetEphemeris.ts`; equatorial↔horizontal transforms are intentionally hand-rolled (see [doc/astronomy-engine.md](doc/astronomy-engine.md)). - Default sky: Boulder (40° N, 105° W), 2024-06-21 18:00 UTC, look south 30° alt, 140° FOV. @@ -48,7 +50,9 @@ Follows the shared [OpenPhysics accessibility convention](https://github.com/Ope ## Compliance carve-outs -- **Hardcoded colors:** `#ffffff` pin stroke in `ObserverLocationNode.ts` — fixed white ring for map legibility on both land and ocean fills; not a profile theme token. +- **Hardcoded colors:** `#ffffff` pin stroke in `ObserverLocationNode.ts` — fixed white ring for map legibility on both land and ocean fills; not a profile theme token. Invisible hit targets use `Color.TRANSPARENT`, not a literal `rgba(0,0,0,0)`. + +**Color pairing gotcha:** `LIGHT_SURFACE_TEXT_FILL` (`controlSurfaceText`, near-black) is only for text on the *white* control surfaces — combo items, flat-button labels, editable field values. Anything drawn on the dark `panelBackground` fill takes `ZenithColors.textColorProperty`. Mixing these up yields ~1:1 contrast and is invisible rather than merely ugly (see the `ObjectNameSearch` result rows). ## Testing diff --git a/README.md b/README.md index 42f60d3..6ce05c5 100644 --- a/README.md +++ b/README.md @@ -18,7 +18,11 @@ Vite 8, TypeScript 7, and Biome 2. - Sun, Moon, and Mercury–Neptune via `astronomy-engine` ephemerides (angularly correct Sun/Moon discs; optional true-scale planets) - Observer latitude/longitude, civil UTC time, and derived local sidereal time with play / pause / speed - English, Spanish, and French localization via `StringManager` -- Deep-link startup via query params (`lat`, `lon`, `date`, `fov`, `magLimit`) +- Deep-link startup via query params (`lat`, `lon`, `date` — 1900–2100, `fov`, `magLimit`) +- Optional "Use my location": browser geolocation, falling back to a third-party IP lookup + (`get.geojs.io`, then `ipapi.co`) when the browser cannot answer. An explicit permission + denial is honoured with no fallback, and the button is the only thing that makes either + request — nothing is sent at startup, and the rest of the sim runs fully offline. - Default (night-sky) and projector color profiles - Progressive Web App (installable, offline-capable) - Git hooks for Biome pre-commit checks diff --git a/doc/model.md b/doc/model.md index 1b624b4..adfd8db 100644 --- a/doc/model.md +++ b/doc/model.md @@ -68,7 +68,9 @@ works across many longitudes): - June / December solstices Year / month / day / hour controls jump to any UTC civil moment in range -(1900–2100). +(1900–2100). That range bounds civil time everywhere: playback stops at either +end rather than running past it, and a `date` deep link outside it falls back to +the default epoch instead of being silently clamped to the boundary. ### Default sky (reset / first load) @@ -101,7 +103,10 @@ south at a modest altitude — a natural outdoor “looking up” pose. - **Object name search** (top-center) — type-ahead over **47 named entries** (38 curated bright stars + 9 solar-system bodies). Enter selects and enables tracking. -- **"Use my location"** — browser geolocation with IP fallback. +- **"Use my location"** — browser geolocation, with a coarse third-party IP-address + lookup as a fallback when the browser cannot answer. Pressing the button is what + triggers the request; declining the browser permission prompt stops there, with no + IP fallback. Worth knowing before using it on a classroom machine. - **N / P** — cycle only among named stars and planets currently in the field of view. ### Coordinate systems (overlays) @@ -192,7 +197,7 @@ You can open the sim with a prepared observer and epoch in the URL, for example: `?lat=-33.9&lon=151.2&date=2024-12-21T10:00:00Z&fov=60&magLimit=4` -Useful parameters: `lat`, `lon`, `date` (ISO-8601 UTC), `fov`, `magLimit`, plus +Useful parameters: `lat`, `lon`, `date` (ISO-8601 UTC, 1900–2100), `fov`, `magLimit`, plus optional toggles for star names, constellation lines, planet names, and the deeper star catalog. Full list: [implementation-notes.md](./implementation-notes.md#deep-link-query-parameters). diff --git a/src/ZenithConstants.ts b/src/ZenithConstants.ts index a80f543..c8d495f 100644 --- a/src/ZenithConstants.ts +++ b/src/ZenithConstants.ts @@ -71,12 +71,6 @@ export const LONGITUDE_RANGE = new Range(-180, 180); /** Arrow-key nudge (degrees) when dragging the observer pin on the location map. */ export const LOCATION_STEP_DEGREES = 5; -/** - * Default local sidereal time is derived from civil time + longitude at reset; - * this constant is only used as a fallback before the first sync. - */ -export const DEFAULT_LOCAL_SIDEREAL_TIME_HOURS = 0; - /** * Hours of civil (and roughly sidereal) time advanced per second of simulation * clock at the base `1×` rate. Real-time: one wall-clock second advances civil @@ -114,6 +108,18 @@ export const CIVIL_DAY_RANGE = new Range(1, 31); /** UTC hour-of-day range for the civil date jump UI. */ export const CIVIL_HOUR_RANGE = new Range(0, 23); +/** + * Supported span of civil time in UTC milliseconds, derived from + * {@link CIVIL_YEAR_RANGE}. This is the authoritative bound: the date-jump + * spinners, the `date` deep-link parameter, and clock playback are all held + * inside it, so the Year spinner can never display a value outside its own + * range and a shared link can never resolve to an epoch the UI cannot express. + */ +export const CIVIL_TIME_MS_RANGE = new Range( + Date.UTC(CIVIL_YEAR_RANGE.min, 0, 1, 0, 0, 0), + Date.UTC(CIVIL_YEAR_RANGE.max, 11, 31, 23, 0, 0), +); + /** * Discrete time-rate ladder: each value multiplies the base educational rate * {@link CIVIL_HOURS_PER_SIM_SECOND}. Symmetric with no zero, so stepping the @@ -306,11 +312,11 @@ ZenithNamespace.register("ZenithConstants", { CONTROL_PANEL_WIDTH, DEFAULT_LATITUDE_DEG, DEFAULT_LONGITUDE_DEG, - DEFAULT_LOCAL_SIDEREAL_TIME_HOURS, CIVIL_HOURS_PER_SIM_SECOND, HOURS_PER_SIDEREAL_DAY, DEFAULT_CIVIL_TIME_MS, CIVIL_YEAR_RANGE, + CIVIL_TIME_MS_RANGE, CIVIL_MONTH_RANGE, CIVIL_DAY_RANGE, CIVIL_HOUR_RANGE, diff --git a/src/i18n/StringManager.ts b/src/i18n/StringManager.ts index e1a12a7..cd4c05c 100644 --- a/src/i18n/StringManager.ts +++ b/src/i18n/StringManager.ts @@ -10,10 +10,17 @@ import stringsEn from "./strings_en.json"; import stringsEs from "./strings_es.json"; import stringsFr from "./strings_fr.json"; +// Compile-time locale parity: every locale must carry exactly the same key set. +// Both directions are asserted for each pair, so a key missing from — or added to — +// any one locale is a build error (`npm run check`), not a runtime fallback. // biome-ignore lint/complexity/noVoid: intentional compile-time type assertion void (stringsEn satisfies typeof stringsFr); // biome-ignore lint/complexity/noVoid: intentional compile-time type assertion void (stringsFr satisfies typeof stringsEn); +// biome-ignore lint/complexity/noVoid: intentional compile-time type assertion +void (stringsEn satisfies typeof stringsEs); +// biome-ignore lint/complexity/noVoid: intentional compile-time type assertion +void (stringsEs satisfies typeof stringsEn); const stringProperties = LocalizedString.getNestedStringProperties({ en: stringsEn, diff --git a/src/main.ts b/src/main.ts index 774a42e..60759e9 100644 --- a/src/main.ts +++ b/src/main.ts @@ -64,13 +64,9 @@ onReadyToLaunch(() => { }, }), - // Optional: fill in credits shown in Help → About - credits: { - leadDesign: "", - softwareDevelopment: "", - team: "", - qualityAssurance: "", - }, + // `credits` is deliberately omitted: joist renders a row for every key it is + // given, so empty strings produce blank labelled rows in Help → About. Add + // the keys back only with real names. Project attribution lives in CREDITS.md. }); sim.start(); diff --git a/src/preferences/ZenithPreferencesModel.ts b/src/preferences/ZenithPreferencesModel.ts index c3c2cc9..cce7b89 100644 --- a/src/preferences/ZenithPreferencesModel.ts +++ b/src/preferences/ZenithPreferencesModel.ts @@ -42,12 +42,9 @@ export class ZenithPreferencesModel { }); } - public reset(): void { - this.showStarLabelsProperty.reset(); - this.showConstellationsProperty.reset(); - this.showPlanetLabelsProperty.reset(); - this.deepStarCatalogProperty.reset(); - } + // Deliberately no reset(): these overlay toggles are preference-backed and + // outlive Reset All (see ZenithModel.reset). An unused reset() here would + // invite wiring it into Reset All and silently breaking that contract. } ZenithNamespace.register("ZenithPreferencesModel", ZenithPreferencesModel); diff --git a/src/preferences/zenithQueryParameters.ts b/src/preferences/zenithQueryParameters.ts index dcfb479..12210fe 100644 --- a/src/preferences/zenithQueryParameters.ts +++ b/src/preferences/zenithQueryParameters.ts @@ -17,6 +17,7 @@ import { logGlobal } from "scenerystack/phet-core"; import { QueryStringMachine } from "scenerystack/query-string-machine"; import { + CIVIL_TIME_MS_RANGE, DEFAULT_CIVIL_TIME_MS, DEFAULT_DEEP_STAR_CATALOG, DEFAULT_FIELD_OF_VIEW_DEG, @@ -34,7 +35,14 @@ import { import ZenithNamespace from "../ZenithNamespace.js"; /** - * Returns true when `value` is empty/null (use sim default) or a parseable civil date. + * Returns true when `value` is empty/null (use sim default), or a parseable + * civil date that falls inside the span the simulation can actually represent + * ({@link CIVIL_TIME_MS_RANGE}, i.e. the `CIVIL_YEAR_RANGE` the date-jump + * spinners cover). Parseability alone is not enough: an out-of-range epoch would + * be silently clamped to the range boundary further down, so a shared link would + * quietly render a different sky than the one it names. Rejecting it here makes + * QueryStringMachine warn and fall back to the documented default instead. + * * Used by QueryStringMachine `isValidValue` for the `date` parameter. * `StringType` from QueryStringMachine includes `null`, so accept that here. */ @@ -42,18 +50,20 @@ export function isValidCivilDateQueryParam(value: string | null): boolean { if (value === null || value === "") { return true; } - return !Number.isNaN(Date.parse(value)); + const ms = Date.parse(value); + return !Number.isNaN(ms) && CIVIL_TIME_MS_RANGE.contains(ms); } /** - * Parses a civil-date query string to UTC ms, or null when empty / invalid. + * Parses a civil-date query string to UTC ms, or null when empty, unparseable, + * or outside {@link CIVIL_TIME_MS_RANGE}. */ export function parseCivilDateQueryParam(value: string | null): number | null { if (value === null || value === "") { return null; } const ms = Date.parse(value); - return Number.isNaN(ms) ? null : ms; + return Number.isNaN(ms) || !CIVIL_TIME_MS_RANGE.contains(ms) ? null : ms; } /** @@ -86,8 +96,9 @@ const zenithQueryParameters = QueryStringMachine.getAll({ }, /** - * Civil UTC timestamp (`Date.parse` / ISO-8601). Empty string uses the sim default. - * Example: `?date=2024-12-21T10:00:00Z`. + * Civil UTC timestamp (`Date.parse` / ISO-8601), within the supported civil + * year range (1900–2100). Empty string — or a date outside that range — uses + * the sim default. Example: `?date=2024-12-21T10:00:00Z`. */ date: { type: "string", diff --git a/src/zenith-screen/model/ZenithModel.ts b/src/zenith-screen/model/ZenithModel.ts index 38a4129..2a9186d 100644 --- a/src/zenith-screen/model/ZenithModel.ts +++ b/src/zenith-screen/model/ZenithModel.ts @@ -34,6 +34,7 @@ import type { ZenithPreferencesModel } from "../../preferences/ZenithPreferences import zenithQueryParameters, { resolveCivilTimeMsFromQuery } from "../../preferences/zenithQueryParameters.js"; import { CIVIL_HOURS_PER_SIM_SECOND, + CIVIL_TIME_MS_RANGE, DEFAULT_CIVIL_TIME_MS, DEFAULT_LATITUDE_DEG, DEFAULT_LONGITUDE_DEG, @@ -65,6 +66,7 @@ import { DEFAULT_LOCATION_PRESET, LOCATION_PRESET_COORDS, LocationPreset } from import type { SelectedSkyObject } from "./SelectedSkyObject.js"; const MS_PER_HOUR = 3600 * 1000; +const MS_PER_MINUTE = 60 * 1000; /** * Single instantaneous ephemeris for the whole solar system at one civil time / @@ -269,7 +271,7 @@ export class ZenithModel implements TModel { this.longitudeProperty = new NumberProperty(startLon, { range: LONGITUDE_RANGE, }); - this.civilTimeMsProperty = new NumberProperty(startCivilMs); + this.civilTimeMsProperty = new NumberProperty(CIVIL_TIME_MS_RANGE.constrainValue(startCivilMs)); this.localSiderealTimeHoursProperty = new NumberProperty(localSiderealTimeHours(startCivilMs, startLon)); this.lookAzimuthDegProperty = new NumberProperty(DEFAULT_LOOK_AZIMUTH_DEG); this.lookAltitudeDegProperty = new NumberProperty(DEFAULT_LOOK_ALTITUDE_DEG, { @@ -391,10 +393,24 @@ export class ZenithModel implements TModel { this.disposers.push(() => this.latitudeProperty.unlink(markLocationCustom)); this.disposers.push(() => this.longitudeProperty.unlink(markLocationCustom)); - const markEpochCustom = (): void => { - if (!this.applyingPreset) { - this.epochPresetProperty.value = EpochPreset.CUSTOM; + // Leaving a named epoch marks it CUSTOM — but only once the clock has moved + // far enough for the UTC readout (minute precision) to disagree with the + // preset. Without that tolerance the timer, which starts playing, advances + // civil time by a few milliseconds on the very first step and flips the combo + // to "Custom time" while the readout still shows the preset epoch — so the + // selector could never display a preset, not even right after one is chosen. + const markEpochCustom = (civilMs: number): void => { + if (this.applyingPreset) { + return; + } + const preset = this.epochPresetProperty.value; + if (preset !== EpochPreset.CUSTOM) { + const presetMs = EPOCH_PRESET_CIVIL_MS.get(preset); + if (presetMs !== undefined && Math.abs(civilMs - presetMs) < MS_PER_MINUTE) { + return; + } } + this.epochPresetProperty.value = EpochPreset.CUSTOM; }; this.civilTimeMsProperty.lazyLink(markEpochCustom); this.disposers.push(() => this.civilTimeMsProperty.unlink(markEpochCustom)); @@ -456,7 +472,17 @@ export class ZenithModel implements TModel { /** Jumps civil time to the observer's real-world current instant ("Now"). */ public setToNow(): void { - this.civilTimeMsProperty.value = Date.now(); + this.setCivilTimeMs(Date.now()); + } + + /** + * Sets civil time, held inside {@link CIVIL_TIME_MS_RANGE}. Every write that + * can leave the supported span (playback, scrubbing, "Now") goes through here, + * so the clock stops at the range edge rather than drifting to an epoch the + * date-jump spinners cannot represent. + */ + public setCivilTimeMs(civilTimeMs: number): void { + this.civilTimeMsProperty.value = CIVIL_TIME_MS_RANGE.constrainValue(civilTimeMs); } /** @@ -503,7 +529,7 @@ export class ZenithModel implements TModel { /** Advances civil time by `hours` (educational scrub / Ctrl-drag). */ public advanceCivilTimeHours(hours: number): void { - this.civilTimeMsProperty.value += hours * MS_PER_HOUR; + this.setCivilTimeMs(this.civilTimeMsProperty.value + hours * MS_PER_HOUR); this.syncLocalSiderealTime(); } diff --git a/src/zenith-screen/view/ObjectNameSearch.ts b/src/zenith-screen/view/ObjectNameSearch.ts index c732bb4..36fd6c5 100644 --- a/src/zenith-screen/view/ObjectNameSearch.ts +++ b/src/zenith-screen/view/ObjectNameSearch.ts @@ -14,7 +14,7 @@ import { DerivedProperty, Multilink, PatternStringProperty, Property, type TReadOnlyProperty } from "scenerystack/axon"; import type { OneKeyStroke } from "scenerystack/scenery"; -import { KeyboardListener, Node, Rectangle, Text, VBox } from "scenerystack/scenery"; +import { Color, KeyboardListener, Node, Rectangle, Text, VBox } from "scenerystack/scenery"; import { PhetFont } from "scenerystack/scenery-phet"; import { LIGHT_SURFACE_TEXT_FILL } from "../../common/ZenithButtonOptions.js"; import { ZenithPanel } from "../../common/ZenithPanel.js"; @@ -151,14 +151,19 @@ export class ObjectNameSearch extends Node { // ── Result rows ───────────────────────────────────────────────────────────── const rowFont = new PhetFont(CONTROL_FONT_SIZE); + // Result rows sit directly on the dark panel fill, not on the white field + // surface, so they take the panel text color. Using LIGHT_SURFACE_TEXT_FILL + // here (near-black on deep indigo) renders them at ~1:1 contrast. The + // highlighted row inverts: dark text on the light accent fill. const createRow = (entry: Entry, active: boolean): Node => { const background = new Rectangle(0, 0, ROW_WIDTH, ROW_HEIGHT, { cornerRadius: 3, - fill: active ? ZenithColors.accentColorProperty : "rgba(0,0,0,0)", + // Transparent (not null) so the whole row stays a pointer hit target. + fill: active ? ZenithColors.accentColorProperty : Color.TRANSPARENT, }); const label = new Text(entry.nameProperty, { font: rowFont, - fill: active ? ZenithColors.controlSurfaceColorProperty : LIGHT_SURFACE_TEXT_FILL, + fill: active ? LIGHT_SURFACE_TEXT_FILL : ZenithColors.textColorProperty, maxWidth: ROW_WIDTH - 12, }); label.left = 6; @@ -174,9 +179,10 @@ export class ObjectNameSearch extends Node { ); // ── "No matches" line + list visibility ──────────────────────────────────── + // Also on the dark panel fill, not the white field — panel text color. const statusText = new Text(controls.searchNoMatchesStringProperty, { font: rowFont, - fill: LIGHT_SURFACE_TEXT_FILL, + fill: ZenithColors.textColorProperty, maxWidth: FIELD_WIDTH, }); Multilink.multilink([focusedProperty, queryProperty, matchesProperty], (isFocused, q, matches) => { diff --git a/src/zenith-screen/view/ObserverLocationNode.ts b/src/zenith-screen/view/ObserverLocationNode.ts index 08a09df..842ee3e 100644 --- a/src/zenith-screen/view/ObserverLocationNode.ts +++ b/src/zenith-screen/view/ObserverLocationNode.ts @@ -20,6 +20,7 @@ import { Shape } from "scenerystack/kite"; import { ModelViewTransform2 } from "scenerystack/phetcommon"; import { Circle, + Color, DragListener, KeyboardListener, Line, @@ -215,7 +216,8 @@ export class ObserverLocationNode extends Node { const pin = new Node({ cursor: "grab", children: [ - new Circle(12, { fill: "rgba(0,0,0,0)" }), + // Transparent (not null) so the disk still hit-tests as a grab target. + new Circle(12, { fill: Color.TRANSPARENT }), new Line(-9, 0, 9, 0, { stroke: ZenithColors.locationPinColorProperty, lineWidth: 1.5 }), new Line(0, -9, 0, 9, { stroke: ZenithColors.locationPinColorProperty, lineWidth: 1.5 }), new Circle(3.5, { fill: ZenithColors.locationPinColorProperty, stroke: "#ffffff", lineWidth: 0.75 }), diff --git a/src/zenith-screen/view/SelectedObjectReadout.ts b/src/zenith-screen/view/SelectedObjectReadout.ts index de95243..5783193 100644 --- a/src/zenith-screen/view/SelectedObjectReadout.ts +++ b/src/zenith-screen/view/SelectedObjectReadout.ts @@ -163,6 +163,15 @@ const buildVisibility = ( }; export class SelectedObjectReadout extends Node { + /** + * Teardown steps for the axon graph this node builds, in the order they must + * run: internal links first, then the Text/Checkbox subtree, then the pattern + * Properties they read, then the DerivedProperties those patterns depend on. + * `DerivedProperty.dispose()` throws while it still has listeners, so the order + * matters — each stage releases the subscribers of the next. + */ + private readonly disposers: (() => void)[] = []; + public constructor(model: ZenithModel) { super(); @@ -176,9 +185,12 @@ export class SelectedObjectReadout extends Node { const nameProperty = new Property( objectName(model.selectedObjectProperty.value, stars, controls.selectedNoneStringProperty.value), ); - Multilink.multilink([model.selectedObjectProperty, controls.selectedNoneStringProperty], (selected, noneLabel) => { - nameProperty.value = objectName(selected, stars, noneLabel); - }); + const nameMultilink = Multilink.multilink( + [model.selectedObjectProperty, controls.selectedNoneStringProperty], + (selected, noneLabel) => { + nameProperty.value = objectName(selected, stars, noneLabel); + }, + ); const coordsProperty = new DerivedProperty( [ @@ -258,8 +270,16 @@ export class SelectedObjectReadout extends Node { const elongDegProperty = new DerivedProperty([elongationProperty], (e) => (e ? e.deg : "—")); const elongDirProperty = new DerivedProperty([elongationProperty], (e) => (e ? e.dir : "")); + // Pattern Properties are hoisted (rather than built inline in each Text) so + // dispose() can release them after the Texts that read them are torn down. + const patterns: { dispose: () => void }[] = []; + const pattern = void }>(property: T): T => { + patterns.push(property); + return property; + }; + const nameText = new Text( - new PatternStringProperty(controls.selectedObjectStringProperty, { name: nameProperty }), + pattern(new PatternStringProperty(controls.selectedObjectStringProperty, { name: nameProperty })), { font: labelFont, fill: ZenithColors.accentColorProperty, maxWidth }, ); const noneText = new Text(controls.selectedNoneStringProperty, { @@ -267,27 +287,34 @@ export class SelectedObjectReadout extends Node { fill: ZenithColors.textColorProperty, maxWidth, }); - const magText = new Text(new PatternStringProperty(controls.selectedMagStringProperty, { mag: magProperty }), { - font: labelFont, - fill: ZenithColors.textColorProperty, - maxWidth, - }); + const magText = new Text( + pattern(new PatternStringProperty(controls.selectedMagStringProperty, { mag: magProperty })), + { + font: labelFont, + fill: ZenithColors.textColorProperty, + maxWidth, + }, + ); const eqText = new Text( - new PatternStringProperty(controls.selectedEquatorialStringProperty, { - ra: raProperty, - dec: decProperty, - }), + pattern( + new PatternStringProperty(controls.selectedEquatorialStringProperty, { + ra: raProperty, + dec: decProperty, + }), + ), { font: labelFont, fill: ZenithColors.textColorProperty, maxWidth }, ); const hzText = new Text( - new PatternStringProperty(controls.selectedHorizontalStringProperty, { - alt: altProperty, - az: azProperty, - }), + pattern( + new PatternStringProperty(controls.selectedHorizontalStringProperty, { + alt: altProperty, + az: azProperty, + }), + ), { font: labelFont, fill: ZenithColors.textColorProperty, maxWidth }, ); const typeText = new Text( - new PatternStringProperty(controls.selectedTypeStringProperty, { type: typeNameProperty }), + pattern(new PatternStringProperty(controls.selectedTypeStringProperty, { type: typeNameProperty })), { font: labelFont, fill: ZenithColors.textColorProperty, @@ -295,16 +322,20 @@ export class SelectedObjectReadout extends Node { }, ); const constellationText = new Text( - new PatternStringProperty(controls.selectedConstellationStringProperty, { - constellation: constellationNameProperty, - }), + pattern( + new PatternStringProperty(controls.selectedConstellationStringProperty, { + constellation: constellationNameProperty, + }), + ), { font: labelFont, fill: ZenithColors.textColorProperty, maxWidth }, ); const elongationText = new Text( - new PatternStringProperty(controls.selectedElongationStringProperty, { - deg: elongDegProperty, - dir: elongDirProperty, - }), + pattern( + new PatternStringProperty(controls.selectedElongationStringProperty, { + deg: elongDegProperty, + dir: elongDirProperty, + }), + ), { font: labelFont, fill: ZenithColors.textColorProperty, maxWidth }, ); @@ -333,41 +364,48 @@ export class SelectedObjectReadout extends Node { const eventTextOptions = { font: labelFont, fill: ZenithColors.textColorProperty, maxWidth }; const riseText = new Text( - new PatternStringProperty(controls.selectedRiseStringProperty, { - time: riseTimeProperty, - clock: riseClockProperty, - az: riseAzProperty, - }), + pattern( + new PatternStringProperty(controls.selectedRiseStringProperty, { + time: riseTimeProperty, + clock: riseClockProperty, + az: riseAzProperty, + }), + ), eventTextOptions, ); const setText = new Text( - new PatternStringProperty(controls.selectedSetStringProperty, { - time: setTimeProperty, - clock: setClockProperty, - az: setAzProperty, - }), + pattern( + new PatternStringProperty(controls.selectedSetStringProperty, { + time: setTimeProperty, + clock: setClockProperty, + az: setAzProperty, + }), + ), eventTextOptions, ); const transitText = new Text( - new PatternStringProperty(controls.selectedTransitStringProperty, { - time: transitTimeProperty, - clock: transitClockProperty, - alt: transitAltProperty, - }), + pattern( + new PatternStringProperty(controls.selectedTransitStringProperty, { + time: transitTimeProperty, + clock: transitClockProperty, + alt: transitAltProperty, + }), + ), eventTextOptions, ); const circumpolarText = new Text(controls.selectedCircumpolarStringProperty, eventTextOptions); const neverRisesText = new Text(controls.selectedNeverRisesStringProperty, eventTextOptions); - visibilityProperty.link((v) => { + const onVisibility = (v: Visibility): void => { riseText.visible = v.kind === "risesSets"; setText.visible = v.kind === "risesSets"; transitText.visible = v.kind === "risesSets" || v.kind === "circumpolar"; circumpolarText.visible = v.kind === "circumpolar"; neverRisesText.visible = v.kind === "neverRises"; - }); + }; + visibilityProperty.link(onVisibility); - coordsProperty.link((coords) => { + const onCoords = (coords: SelectionCoords): void => { const hasSelection = coords.kind !== "none"; noneText.visible = !hasSelection; nameText.visible = hasSelection; @@ -376,14 +414,17 @@ export class SelectedObjectReadout extends Node { magText.visible = hasSelection; eqText.visible = hasSelection; hzText.visible = hasSelection; - }); + }; + coordsProperty.link(onCoords); // Elongation only applies to planets (and the Moon), never a bare star or the Sun. - elongationProperty.link((elong) => { + const onElongation = (elong: { deg: string; dir: string } | null): void => { elongationText.visible = elong !== null; - }); + }; + elongationProperty.link(onElongation); // ── Track toggle (enabled only when an object is selected) ──────────────── + const trackEnabledProperty = new DerivedProperty([model.selectedObjectProperty], (s) => s !== null); const trackCheckbox = new Checkbox( model.trackSelectedObjectProperty, new Text(controls.trackSelectedStringProperty, { @@ -393,7 +434,9 @@ export class SelectedObjectReadout extends Node { }), { ...ZENITH_CHECKBOX_OPTIONS, - enabledProperty: new DerivedProperty([model.selectedObjectProperty], (s) => s !== null), + // Not owned by the Checkbox (EnabledComponent only disposes an + // enabledProperty it created), so dispose() below releases it. + enabledProperty: trackEnabledProperty, accessibleName: a11y.controls.trackSelectedStringProperty, }, ); @@ -420,5 +463,78 @@ export class SelectedObjectReadout extends Node { ], }), ); + + // ── Teardown, in dependency order ───────────────────────────────────────── + // 1. Internal listeners on Properties we are about to dispose. + this.disposers.push(() => { + visibilityProperty.unlink(onVisibility); + coordsProperty.unlink(onCoords); + elongationProperty.unlink(onElongation); + nameMultilink.dispose(); + }); + // 2. Pattern Properties (their Text readers are disposed by dispose() first). + this.disposers.push(() => { + for (const patternProperty of patterns) { + patternProperty.dispose(); + } + }); + // 3. Derived Properties that fed those patterns, leaf-most first… + this.disposers.push(() => { + for (const property of [ + magProperty, + raProperty, + decProperty, + altProperty, + azProperty, + typeNameProperty, + constellationNameProperty, + constellationSourceProperty, + elongDegProperty, + elongDirProperty, + riseTimeProperty, + riseAzProperty, + riseClockProperty, + setTimeProperty, + setAzProperty, + setClockProperty, + transitTimeProperty, + transitAltProperty, + transitClockProperty, + ]) { + property.dispose(); + } + }); + // 4. …then the bases they derived from, and the standalone Properties. + this.disposers.push(() => { + coordsProperty.dispose(); + visibilityProperty.dispose(); + elongationProperty.dispose(); + trackEnabledProperty.dispose(); + nameProperty.dispose(); + }); + } + + /** + * Releases this panel's axon graph. `Node.dispose()` only detaches children, so + * the readout's Texts, its pattern Properties, and the ~20 DerivedProperties + * behind them would otherwise stay subscribed to the model and the shared + * StringManager after the screen is torn down. Children are disposed between + * stages 1 and 2 so no Text is still reading a pattern when it is released. + * Idempotent — the disposer list is drained on the first call. + */ + public override dispose(): void { + const [detachListeners, disposePatterns, ...rest] = this.disposers.splice(0); + detachListeners?.(); + + const children = this.children; + super.dispose(); + for (const child of children) { + child.disposeSubtree(); + } + + disposePatterns?.(); + for (const disposeStage of rest) { + disposeStage(); + } } } diff --git a/src/zenith-screen/view/ZenithScreenView.ts b/src/zenith-screen/view/ZenithScreenView.ts index afd68b9..48006b6 100644 --- a/src/zenith-screen/view/ZenithScreenView.ts +++ b/src/zenith-screen/view/ZenithScreenView.ts @@ -62,6 +62,7 @@ const SELECTION_PANEL_BOTTOM_CLEARANCE = 52; export class ZenithScreenView extends ScreenView { private readonly skyNode: PlanetariumSkyNode; + private readonly selectedReadout: SelectedObjectReadout; private readonly searchNode: ObjectNameSearch; private readonly controlPanel: AccordionBox; private readonly timePanel: TimeControlPanel; @@ -184,7 +185,7 @@ export class ZenithScreenView extends ScreenView { }, ); - const selectedReadout = new SelectedObjectReadout(model); + this.selectedReadout = new SelectedObjectReadout(model); const checkbox = ( property: typeof model.showGridProperty, @@ -419,7 +420,7 @@ export class ZenithScreenView extends ScreenView { this.addChild(this.skyNode); // Selection readout sits over the sky (bottom-left). - const selectionPanel = new ZenithPanel(selectedReadout, { + const selectionPanel = new ZenithPanel(this.selectedReadout, { xMargin: 8, yMargin: 6, }); @@ -500,8 +501,24 @@ export class ZenithScreenView extends ScreenView { this.searchNode.centerX = visibleBounds.centerX; this.searchNode.top = visibleBounds.minY + SCREEN_VIEW_MARGIN; - selectionPanel.left = visibleBounds.minX + SELECTION_PANEL_INSET; + // Selection readout: bottom-left over the sky. The left column (Location + + // Time) can reach the bottom of the window at common sizes, which would + // leave the readout painted over by the Time panel — so when they would + // overlap, slide it clear to the right of that column instead. + // + // `left` is computed once and assigned once: this runs re-entrantly from + // selectionPanel.boundsProperty, so writing it twice (default, then the + // shifted value) would notify back into here forever. Anchoring `bottom` + // first is safe — the overlap test reads `top`, which depends on `bottom` + // and the panel height, never on `left`. selectionPanel.bottom = safeBottom - SELECTION_PANEL_BOTTOM_CLEARANCE; + const selectionClearsLeftColumn = selectionPanel.top >= this.timePanel.bottom + RESET_ALL_PANEL_GAP; + selectionPanel.left = selectionClearsLeftColumn + ? visibleBounds.minX + SELECTION_PANEL_INSET + : Math.min( + this.timePanel.right + RESET_ALL_PANEL_GAP, + visibleBounds.maxX - SELECTION_PANEL_INSET - selectionPanel.width, + ); resetAllButton.right = visibleBounds.maxX - SCREEN_VIEW_MARGIN; resetAllButton.bottom = safeBottom - SCREEN_VIEW_MARGIN; @@ -577,6 +594,7 @@ export class ZenithScreenView extends ScreenView { dispose(); } this.skyNode.dispose(); + this.selectedReadout.dispose(); } public override step(_dt: number): void { diff --git a/tests/ZenithModel.test.ts b/tests/ZenithModel.test.ts index 3ed0e96..468fce6 100644 --- a/tests/ZenithModel.test.ts +++ b/tests/ZenithModel.test.ts @@ -10,6 +10,7 @@ import { equatorialToHorizontal } from "../src/common/sky/SkyCoordinates.js"; import { ZenithPreferencesModel } from "../src/preferences/ZenithPreferencesModel.js"; import { CIVIL_HOURS_PER_SIM_SECOND, + CIVIL_TIME_MS_RANGE, DEFAULT_CIVIL_TIME_MS, DEFAULT_FIELD_OF_VIEW_DEG, DEFAULT_LATITUDE_DEG, @@ -287,4 +288,33 @@ describe("ZenithModel", () => { expect(model.epochPresetProperty.value).toBe(EpochPreset.CUSTOM); expect(model.civilTimeMsProperty.value).toBe(Date.UTC(2020, 0, 1, 3, 0, 0)); }); + + it("keeps the epoch preset selected while the clock ticks within the readout's precision", () => { + // The timer starts playing, so the first step advances civil time by a few + // milliseconds. That must not flip the combo to CUSTOM while the UTC readout + // (minute precision) still shows the preset epoch. + model.epochPresetProperty.value = EpochPreset.DECEMBER_SOLSTICE; + model.step(1 / 60); + expect(model.epochPresetProperty.value).toBe(EpochPreset.DECEMBER_SOLSTICE); + }); + + it("marks epoch CUSTOM once the clock has moved a full minute off the preset", () => { + model.epochPresetProperty.value = EpochPreset.DECEMBER_SOLSTICE; + model.advanceCivilTimeHours(1 / 60); + expect(model.epochPresetProperty.value).toBe(EpochPreset.CUSTOM); + }); + + it("holds civil time inside the supported year range", () => { + model.setCivilTimeMs(CIVIL_TIME_MS_RANGE.max + 5 * 365 * 24 * 3600 * 1000); + expect(model.civilTimeMsProperty.value).toBe(CIVIL_TIME_MS_RANGE.max); + + model.setCivilTimeMs(CIVIL_TIME_MS_RANGE.min - 5 * 365 * 24 * 3600 * 1000); + expect(model.civilTimeMsProperty.value).toBe(CIVIL_TIME_MS_RANGE.min); + }); + + it("stops the clock at the range edge instead of playing past it", () => { + model.setCivilTimeMs(CIVIL_TIME_MS_RANGE.max); + model.advanceCivilTimeHours(48); + expect(model.civilTimeMsProperty.value).toBe(CIVIL_TIME_MS_RANGE.max); + }); }); diff --git a/tests/memory-leak.test.ts b/tests/memory-leak.test.ts index b4214da..5aa4a53 100644 --- a/tests/memory-leak.test.ts +++ b/tests/memory-leak.test.ts @@ -9,6 +9,7 @@ import { TimeModel } from "../src/common/TimeModel.js"; import { ZenithPreferencesModel } from "../src/preferences/ZenithPreferencesModel.js"; import { ZenithModel } from "../src/zenith-screen/model/ZenithModel.js"; import { PlanetariumSkyNode } from "../src/zenith-screen/view/PlanetariumSkyNode.js"; +import { SelectedObjectReadout } from "../src/zenith-screen/view/SelectedObjectReadout.js"; import { ZenithScreenView } from "../src/zenith-screen/view/ZenithScreenView.js"; async function forceGC(earlyExitRef?: WeakRef): Promise { @@ -156,6 +157,50 @@ describe("Memory leak regression", () => { }); }); + describe("SelectedObjectReadout", () => { + it("is collected after dispose", async () => { + const ref = (() => { + const preferences = new ZenithPreferencesModel(); + const model = new ZenithModel(preferences); + model.timer.isPlayingProperty.value = false; + const readout = new SelectedObjectReadout(model); + const weak = new WeakRef(readout); + readout.dispose(); + model.dispose(); + return weak; + })(); + await forceGC(ref); + expect(ref.deref()).toBeUndefined(); + }); + + it("stops observing the model after dispose", () => { + const preferences = new ZenithPreferencesModel(); + const model = new ZenithModel(preferences); + model.timer.isPlayingProperty.value = false; + const readout = new SelectedObjectReadout(model); + readout.dispose(); + + // The readout derives coordinates, rise/set times and the constellation + // name from these; after dispose none of that may still be listening. + expect(() => { + model.civilTimeMsProperty.value += 3_600_000; + model.latitudeProperty.value = -33.9; + model.selectedObjectProperty.value = null; + }).not.toThrow(); + + model.dispose(); + }); + + it("double dispose() does not throw", () => { + const preferences = new ZenithPreferencesModel(); + const model = new ZenithModel(preferences); + const readout = new SelectedObjectReadout(model); + readout.dispose(); + expect(() => readout.dispose()).not.toThrow(); + model.dispose(); + }); + }); + describe("ZenithScreenView", () => { // Full ScreenView GC is not asserted here: joist `ScreenView` plus sun // ComboBox / NumberControl / localized string Properties retain a graph that diff --git a/tests/zenithQueryParameters.test.ts b/tests/zenithQueryParameters.test.ts index 622d1fc..12e42df 100644 --- a/tests/zenithQueryParameters.test.ts +++ b/tests/zenithQueryParameters.test.ts @@ -10,7 +10,7 @@ import { parseCivilDateQueryParam, resolveCivilTimeMsFromQuery, } from "../src/preferences/zenithQueryParameters.js"; -import { DEFAULT_CIVIL_TIME_MS } from "../src/ZenithConstants.js"; +import { CIVIL_TIME_MS_RANGE, DEFAULT_CIVIL_TIME_MS } from "../src/ZenithConstants.js"; describe("zenithQueryParameters date helpers", () => { it("treats empty or null date as the sim default civil epoch", () => { @@ -33,4 +33,22 @@ describe("zenithQueryParameters date helpers", () => { expect(isValidCivilDateQueryParam("not-a-date")).toBe(false); expect(parseCivilDateQueryParam("not-a-date")).toBeNull(); }); + + it("rejects parseable dates outside the supported civil year range", () => { + // Accepting these would let the date-jump spinners clamp them to the range + // boundary, so a shared link would silently render a different sky. + for (const outOfRange of ["1850-01-01T00:00:00Z", "2150-06-01T00:00:00Z"]) { + expect(isValidCivilDateQueryParam(outOfRange)).toBe(false); + expect(parseCivilDateQueryParam(outOfRange)).toBeNull(); + expect(resolveCivilTimeMsFromQuery(outOfRange)).toBe(DEFAULT_CIVIL_TIME_MS); + } + }); + + it("accepts dates at both ends of the supported range", () => { + for (const ms of [CIVIL_TIME_MS_RANGE.min, CIVIL_TIME_MS_RANGE.max]) { + const iso = new Date(ms).toISOString(); + expect(isValidCivilDateQueryParam(iso)).toBe(true); + expect(resolveCivilTimeMsFromQuery(iso)).toBe(ms); + } + }); });