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

Used luxon instead of moment #1vy5ryh #29

Merged
merged 16 commits into from
Feb 4, 2022
Merged

Conversation

disha1202
Copy link
Contributor

No description provided.

@disha1202 disha1202 changed the title #1vy5ryh Used luxon instead of moment #1vy5ryh Jan 12, 2022
src/App.vue Outdated Show resolved Hide resolved
src/views/TimezoneModal.vue Outdated Show resolved Hide resolved
src/views/TimezoneModal.vue Outdated Show resolved Hide resolved
@@ -120,9 +116,10 @@ export default defineComponent({
// Currently backend API returns some of the legacy timezones which are not found in moment list
// Due to which if we set them we get an error
// Filtered them out till it is fixed at backend
this.timeZones = resp.data.filter((timeZone: any) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why we have commented this out?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed the commented code and comments because we are not using moment anymore.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think here we are checking if the server timezone belongs to moment as well. We should also handle it for luxon. Check if we are similar method in Luxon

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Handled it for luxon as well

src/App.vue Outdated
text: this.$t("Dismiss"),
role: 'cancel',
cssClass: 'secondary'
{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please revert all the unrelated indentation changes

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reverted indentation changes.

@@ -120,9 +116,10 @@ export default defineComponent({
// Currently backend API returns some of the legacy timezones which are not found in moment list
// Due to which if we set them we get an error
// Filtered them out till it is fixed at backend
this.timeZones = resp.data.filter((timeZone: any) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think here we are checking if the server timezone belongs to moment as well. We should also handle it for luxon. Check if we are similar method in Luxon

}
}
],
{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Revert unrelated indentation changes

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reverted unrelated indentation changes

@adityasharma7 adityasharma7 merged commit d585e8e into hotwax:main Feb 4, 2022
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.

3 participants