Skip to content
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

Remove time context switching from views #4226

Closed
akhenry opened this issue Sep 17, 2021 · 7 comments · Fixed by #4556 or #4581
Closed

Remove time context switching from views #4226

akhenry opened this issue Sep 17, 2021 · 7 comments · Fixed by #4556 or #4581

Comments

@akhenry
Copy link
Contributor

akhenry commented Sep 17, 2021

Is your feature request related to a problem? Please describe.
Open MCT now supports independent time control in some views. Yay! The change that enables this introduces the concept of "time contexts". A view can be following either an independent time context (that is configured for the specific view) or the global time context (which is controlled by the time conductor).

The current implementation requires views to handle switching between independent and global time contexts when it occurs.

Views should not need to care whether they are following an independent or global time context if all they care about is what the current time bounds are. Some views may want to know this information, but we should not be pushing a requirement on views to track this.

Describe the solution you'd like
When a view requests a time context, they should receive a context that is valid for the lifetime of the view. They should not need to listen for context changes, because this is additional boilerplate that all views will need to add. It adds risk of memory leaks if listeners are not cleaned up, and it also introduces risk that the view could get out of sync with the active time context.

Instead, the independent time context could switch between tracking its own state internally, and tracking the global time context. So, in the case that the view is following the global time context (which is the default case), then the independent time context is basically just a thin wrapper over the global time context.

Describe alternatives you've considered
A clear and concise description of any alternative solutions or features you've considered.

Additional context
Add any other context or screenshots about the feature request here.

@shefalijoshi
Copy link
Contributor

shefalijoshi commented Dec 2, 2021

Listeners (for bounds changes etc) are registered on this initial context object assigned to a view.
When the time context for a view needs to change (either from indepedent -> global or global -> independent), so if we don't want views to handle time context switches when they happen, then we need to have a mechanism where we can pass through events from the new/changed context to the initial context object. (Hope that makes sense?)

For example:
View1
-> View 2 (nested)

independentTimeContexts: []
contextsByView: {view1: view1Context->global, view2: view2Context->global}

Listeners are registered on these context objects

View2 -> add independent time context
independentTimeContexts: [independentTimeContext2]
Check all views to see if they should use this independent context
contextsByView: {view1: viewContext1->globalTimeContext, view2: viewContext2->independentTimeContext2}

View1 -> add independent time context
independentTimeContexts: [independentTimeContext2, independentTimeContext1]
Check all views to see if they should use this independent context
contextsByView: {view1: viewContext1->independentTimeContext1, view2: viewContext2->independentTimeContext1}

@shefalijoshi
Copy link
Contributor

shefalijoshi commented Dec 9, 2021

Testing instructions:
Ensure independent time conductor works as before.
Create a Time Strip domain object and all some plots/plans to it.
Enable the independent time context and change the bounds etc.
Turn it off and ensure that the global clock is followed.
OR
In code, Use independent context in a new domain object and not have to listen for a 'timeContext' event from the timeAPI.

@charlesh88
Copy link
Contributor

Testathon 12-13-21 NOT fixed, using a Time Strip view with this plan: https://banner.ndc.nasa.gov/testathon/#/browse/couch-search:couch-search/ab3471f6-928d-4858-a114-1e7c1a981cc5?tc.mode=local&tc.startDelta=900000&tc.endDelta=15000&tc.timeSystem=utc&view=plan.view

  • If global TC is in real-time mode, the settings in the ITC have no effect, but should.
  • Start and End date pickers in the ITC are selecting to values in the global TC, but should be pointed at those in the ITC.

@nikhilmandlik
Copy link
Contributor

  • When click on 'calendar button' on independent time conductor it opens calendar with value of popup synced with main time conductor instead of independent time conductor
  • when in local clock mode, click on independent time conductor 'Fixed Timestamp' button opens popup in wrong position.

@michaelrogers
Copy link
Contributor

michaelrogers commented Dec 16, 2021

Screen Shot 2021-12-16 at 3 26 32 PM

The local clock mode popup position still appears to overlap the button in testathon 2021-12-16. Otherwise the above fixed / real-time issues appear to have been resolved in #4581.

Fix verified in Testathon 2021-12-16.

@unlikelyzero
Copy link
Collaborator

Closed. @michaelrogers can you file a visual bug for this one

@nikhilmandlik
Copy link
Contributor

Verified Fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants