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.

Sign up for GitHub

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

Jump to bottom

Fix datetime translations #309

Open
wants to merge 10 commits into
base: dev
Choose a base branch
from
Open

Fix datetime translations #309

wants to merge 10 commits into from

Conversation

KevinTriplett
Copy link
Contributor

This fixes some of the translation problems with posts.

Tight review of i18n and TextHelpers, to make sure notifications are okay with the changes.

  • for i18n, we need to load the namespaces in order to use the translation outside of react components
  • for TextHelpers, it was a bit of a mess when luxon could solve the problem AND provide translation win!
  • BUT -- there's not an easy way to remove the "ago" from all the translations. And "short" may not be good enough for English everywhere.

@KevinTriplett
Copy link
Contributor Author

This commit "add setLocale as needed" has touched a number of files. I made an executive decision to change the format for locales that don't use English ordering of date (month-day-year) -- so check especially Invite and Join request formatted dates, to make sure it doesn't mess up any automation.

@KevinTriplett
Copy link
Contributor Author

I removed reviews, since translation is probably not a priority.

tibetsprague
tibetsprague requested changes Feb 28, 2025
View reviewed changes
Copy link
Collaborator

@tibetsprague tibetsprague left a comment

Choose a reason for hiding this comment

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

great! generally looks good. will require lots of testing across front and backend since TextHelper is used in both places

packages/shared/src/TextHelpers.js
@@ -1,3 +1,5 @@
import i18n from '../../../apps/web/src/i18n.mjs'
Copy link
Collaborator

Choose a reason for hiding this comment

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

this cant be right. we probably should just move this file to shared? hmm...

packages/shared/src/TextHelpers.js
@@ -1,3 +1,5 @@
import i18n from '../../../apps/web/src/i18n.mjs'
import { getLocaleAsString } from '../../../apps/web/src/components/Calendar/calendar-util'
Copy link
Collaborator

Choose a reason for hiding this comment

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

we shouldnt be loading stuff from web in shared, so move this here?

Copy link
Contributor Author

@KevinTriplett KevinTriplett Feb 28, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

After the launch, let's re-visit this PR and potential need for re-organization.

I want to create DateTimeHelper functions, there are many repeating patterns and we can change the library more easily if they're centralized.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tibetsprague tibetsprague tibetsprague requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@KevinTriplett @tibetsprague