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

fix: support "week", "bi-week" and months in period translations #3269

Merged
merged 5 commits into from
Jun 26, 2024

Conversation

BRaimbault
Copy link
Collaborator

@BRaimbault BRaimbault commented Jun 25, 2024

Part of the fix for DHIS2-16904

Requires dhis2/analytics#1685


Key features

  • bump analytics dependency
  • pass periodsSettings to @dhis2/analytics utility function: getFixedPeriodsOptionsById

Description

The actual fix is in @dhis2/multi-calendar-dates which is a dependency of analytics and it's used in the PeriodSelector component the analytics apps use.
See also the PR in analytics.

For maps-app, the locale and calendar specs where not passed to the @dhis2/analytics utility function, so this was implemented as well.
Note: this has not been implemented for selectors limited to years.


Screenshots

  • Translations don't work for week/bi-week:

    • Before:
      image
      -After:
      image
  • Translations don't work for months:

    • Before:
      image
    • After:
      image

@dhis2-bot
Copy link
Contributor

dhis2-bot commented Jun 25, 2024

🚀 Deployed on https://pr-3269--dhis2-maps.netlify.app

@dhis2-bot dhis2-bot temporarily deployed to netlify June 25, 2024 14:21 Inactive
@dhis2-bot dhis2-bot temporarily deployed to netlify June 26, 2024 08:37 Inactive
@dhis2-bot dhis2-bot temporarily deployed to netlify June 26, 2024 08:51 Inactive
@dhis2-bot dhis2-bot temporarily deployed to netlify June 26, 2024 08:57 Inactive
@BRaimbault BRaimbault requested a review from edoardo June 26, 2024 09:03
@BRaimbault BRaimbault changed the title fix: bump analytics to 26.7.5 for period translations fix: support "week", "bi-week" and months in period translations Jun 26, 2024
@@ -27,6 +27,15 @@ export const appQueries = {
key: SYSTEM_SETTINGS,
},
},
systemInfo: {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think you can get the systemInfo using the useConfig hook - this would make use of the settings already in the provider rather than making an extra API call.

import { useConfig } from '@dhis2/app-runtime'

const { systemInfo } = useConfig()
const { calendar = 'gregory' } = systemInfo

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I get your point.
I find it a bit more intuitive to have both requests comming from the same place (calendar + locale).
Down the road we are going to implement the standard periods selector so this will have to change.
Unless you think it is definitely better to make this change now, I would argue in favor of keeping the current version.

Copy link
Collaborator

@kabaros kabaros left a comment

Choose a reason for hiding this comment

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

lgtm - just the one comment/suggestion

@dhis2-bot dhis2-bot temporarily deployed to netlify June 26, 2024 13:42 Inactive
@BRaimbault BRaimbault merged commit 977e075 into master Jun 26, 2024
21 checks passed
@BRaimbault BRaimbault deleted the fix/DHIS2-16904 branch June 26, 2024 14:09
dhis2-bot added a commit that referenced this pull request Jun 26, 2024
## [100.5.6](v100.5.5...v100.5.6) (2024-06-26)

### Bug Fixes

* support "week", "bi-week" and months in period translations ([#3269](#3269)) ([977e075](977e075))
@dhis2-bot
Copy link
Contributor

🎉 This PR is included in version 100.5.6 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

4 participants