Decouple TimeControl and TimeUI via event bus - #217
Open
sandesh-sp wants to merge 8 commits into
Open
Conversation
Replace the direct TimeControl <-> TimeUI calls with mmgisAPI events (time:userChanged, time:setRequested, time:layersReloaded, time:change) and move shared time parsing into timeUtils. TimeUI now renders only in the classic layout; modern layout uses the Timeline tool.
… management - Decouple time control events from UI updates to prevent redundant resyncs - Enhance time initialization logic to derive start/end times from committed state - Update time utility functions to handle null/undefined inputs gracefully - Introduce unit tests for time parsing utilities
The event-bus decoupling removed TimeControl.timeUI but left consumers referencing it: QueryURL threw on every share-URL generation for time-enabled missions, the Draw templater's set-time buttons threw on click, and AnimationTool's TimeUI integration and UserInterfaceDefault_'s popover realignment silently no-op'd behind guards. Point them all at TimeUI directly, guarded on the widget actually being mounted (TimeUI.startTempus) where they touch its DOM. Also: - Clear TimeUI._lastEmitted once its echo is consumed so a later external commit of identical times isn't mistaken for an echo and skipped - Validate dates in TimeControl.setTime and return false instead of throwing RangeError mid-mutation on invalid input - Implement TimeControl.toggleTimeUI (mmgisAPI.toggleTimeUI has always referenced it, but it never existed) by delegating to the Coordinates-owned '#toggleTimeUI' button - Replace 'Invalid Date' string-coercion checks with isNaN(getTime()) - Deduplicate the legacy-TimeUI layout condition in essence.js into usesLegacyTimeUI() Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Sqp49fHnucNf9uvAT6QfYn
CarsonDavis
reviewed
Jul 28, 2026
| _initialStart: null, | ||
| _initialEnd: null, | ||
| // Payload of the last emitted time:changeRequested — used to recognize | ||
| // the committed echo on time:change and skip a redundant resync |
Collaborator
There was a problem hiding this comment.
time:change -> time:changeCompleted
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Decouples
TimeControl(time state + layer reloading) fromTimeUI(the legacy bottom time slider) so they communicate over themmgisAPIevent bus instead of direct method calls.TimeControl's directimport TimeUIand theTimeControl.timeUIreference; migrates all formerTimeControl.timeUIconsumers (QueryURL, AnimationTool, DrawTool_Templater, UserInterfaceDefault_) to useTimeUIdirectly, guarded on the widget being mountedTimeUIinstantiation out ofTimeControl.init()intoessence.js, gated on!mobile && !modernMode && time.enabled(usesLegacyTimeUI())time:changeRequested(widgets → TimeControl),time:change(TimeControl → widgets),time:layersReloadedinit()(no reload — layers aren't on the map yet), performs the single initial reload infina(); TimeUI mirrors committed state and recognizes echoes of its own requests (_lastEmitted, consumed on match;_suppressChangeEmit) so nothing reloads twiceparseTimeWithOffset/parseTimeToSecondsinto a sharedtimeUtils.jswith unit testsTimeControl.setTime(returnsfalseon invalid input instead of throwing mid-mutation)TimeControl.toggleTimeUI—mmgisAPI.toggleTimeUIhas always referenced it but it never existed; it now delegates to the Coordinates-owned#toggleTimeUIbuttonFiles changed
src/essence/Basics/TimeControl_/TimeControl.jssrc/essence/Basics/TimeControl_/TimeUI.jssrc/essence/Basics/TimeControl_/timeUtils.js(new)src/essence/essence.jssrc/essence/Ancillary/QueryURL.jssrc/essence/Tools/Animation/AnimationTool.jssrc/essence/Tools/Draw/DrawTool_Templater.jssrc/essence/Basics/UserInterface_/UserInterfaceDefault_.jstests/unit/timeUtils.spec.js(new)Test plan
TimeUI) renders and drives time changes whentime.enabledand not mobile/modern modemmgisAPItime events (time:change,time:layersReloaded) fire as expectedstartTime/endTime/live)mmgisAPI.setTimewith invalid dates returnsfalsewithout breaking state🤖 Generated with Claude Code
https://claude.ai/code/session_01Sqp49fHnucNf9uvAT6QfYn