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

refactor(angularjs): remove angular-moment #2154

Merged
merged 1 commit into from
Jun 14, 2021

Conversation

LoneRifle
Copy link
Collaborator

@LoneRifle LoneRifle commented Jun 14, 2021

Problem

As we prepare for the React migration, we should minimise the number of
AngularJS-specific libraries that are currently in use.

Solution

As we have no need for the AngularJS filters and directives that
angular-moment provides, remove it completely, referencing moment
as a CommonJS module instead.

Testing

Verify that date formatting remains correct for:

  • Admin dashboard form list
  • Form builder heading
  • Viewing responses in storage mode
  • On form submission

As we have no need for the AngularJS filters and directives that
angular-moment provides, remove it completely, referencing moment
as a CommonJS module instead
@mantariksh
Copy link
Contributor

@LoneRifle could you explain more about this? I’m actually not sure this is helpful for React migration because it doesn’t give us any code which we can battle-test in production and then reuse in React. regardless of whether we use moment-timezone or angular-moment in AngularJS, we're going to throw out the code anyway.

@LoneRifle
Copy link
Collaborator Author

LoneRifle commented Jun 14, 2021

@LoneRifle could you explain more about this? I’m actually not sure this is helpful for React migration because it doesn’t give us any code which we can battle-test in production and then reuse in React. regardless of whether we use moment-timezone or angular-moment in AngularJS, we're going to throw out the code anyway.

I agree - however, I think that we ought to throw out code earlier if we think we can, so that there is less to figure out later on. This PR would also help to better identify dependencies on moment-timezone within components so that moving these out of AngularJS would be easier.

@mantariksh
Copy link
Contributor

This PR would also help to better identify dependencies on moment-timezone within components so that moving these out of AngularJS would be easier.

ok! agree here

@mantariksh
Copy link
Contributor

mantariksh commented Jun 14, 2021

Manual tests

  • Modify a form and check that the "last modified" under the form title in the admin dashboard is formatted the same way as in production
  • Check that the "last modified" date for forms in the form dashboard is formatted the same way as in production
  • Storage mode CSV download works correctly
  • Formatting of submission date in end page (page after submitting form) works correctly

@mantariksh mantariksh merged commit 29bbd26 into develop Jun 14, 2021
@LoneRifle LoneRifle deleted the refactor/angular-moment/remove branch June 14, 2021 09:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants