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

Re-implement Clock object and indicator in Vue.js #4042

Closed
akhenry opened this issue Jul 19, 2021 · 7 comments · Fixed by #4106
Closed

Re-implement Clock object and indicator in Vue.js #4042

akhenry opened this issue Jul 19, 2021 · 7 comments · Fixed by #4106

Comments

@akhenry
Copy link
Contributor

akhenry commented Jul 19, 2021

The existing Clock object and the associated indicator are currently implemented as a legacy bundle and written in Angular.js . They should be re-implemented as a plugin under /src/plugins using our current API, with the UI migrated to Vue.

The Timer should not be re-implemented just yet, just the clock object and clock indicator. Re-implementing the timer should be addressed separately as its functionality is a little more nuanced and we need to decide on the correct design approach.

A followup issue has also been created to enhance the clock object and indicator to support remote clocks, but that can be addressed in a future PR.

Any legacy code associated with the legacy Clock object and indicator should be removed from the code-base.

@akhenry akhenry changed the title Re-implement Clock object in Vue.js Re-implement Clock object and indicator in Vue.js Jul 19, 2021
@kobe1104 kobe1104 self-assigned this Jul 19, 2021
@davetsay davetsay assigned davetsay and unassigned kobe1104 Jul 23, 2021
@davetsay
Copy link
Contributor

@khalidadil , can you do this one before the dialog one please? thanks.

@akhenry
Copy link
Contributor Author

akhenry commented Jul 29, 2021

The legacy clock allowed the user to select a time zone and a clock format. Re-implementing that functionality is blocked by the ongoing form refactoring work, so for now it's ok for new clocks to be created with a hard coded time zone (UTC) and format. The new clock object should still support user-specified time zones and display formats though, in order to retain support for legacy clock objects, and in preparation for our new forms and create action.

@khalidadil
Copy link
Contributor

Testing instructions

  1. If possible, create a clock before the change and then change to the branch with the change. Ensure that the clock continues to display properly and that editing the properties works.
  2. Create a clock object and choose some values in the creation dialog. Ensure that the time is displayed and is in the appropriate format based on the options chosen (proper timezone, format, 12/24 hours).
  3. Edit the properties to modify the display format and ensure the clock updates with the correct time.
  4. Edit the properties to modify the 12/24 hours selection and ensure the clock updates with the correct time.
  5. Edit the properties to modify the timezone and ensure the clock updates with the correct time.
  6. Check that the clock indicator is displayed when the option enableClockIndicator is set to true in index.html and displays time in UTC.

It would be useful to make sure the timer still works properly as well since code around it was removed for this change.

@charlesh88
Copy link
Contributor

Testathon 9-20-21: Clock object not available to create and not displayed in the Status area. Unverified.

@davetsay
Copy link
Contributor

Since this was taken out of the angular bundles and installed as a plugin it needs to be manually installed in deployments. @khalidadil . We can retest after we install in VIPER repo.

@unlikelyzero
Copy link
Contributor

testable after change made on viper repo

@jvigliotta
Copy link
Contributor

Looks good to me! I tested the timer as well and didn't see any issues.

Verified Fixed - Testathon 9/28

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

Successfully merging a pull request may close this issue.

7 participants