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: translation strings (DHIS2-10630) #199

Merged
merged 1 commit into from
Apr 1, 2021
Merged

Conversation

KaiVandivier
Copy link
Contributor

@KaiVandivier KaiVandivier commented Mar 17, 2021

Relates to https://jira.dhis2.org/browse/DHIS2-10630 and https://jira.dhis2.org/browse/DHIS2-10664, I believe

Adds translation strings for page headers and 'no apps installed' messages

Controversial: adds legacy i18n to translate '** search **' in the search bar from d2-ui Sidebar (see the second commit). If this one string isn't worth the legacy i18n, especially since Médi is working on the overhaul of this app, I can remove it :)

@KaiVandivier KaiVandivier requested review from HendrikThePendric and a team March 17, 2021 17:15
Copy link
Contributor

@HendrikThePendric HendrikThePendric left a comment

Choose a reason for hiding this comment

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

Since this app is still relying on, and has a working implementation of, the legacy i18n system, I think we should make sure these strings are fixed too. So I don't think anything needs to be removed from this PR.

I do think that we may have missed a few strings, for example this seems to result in hardcoded strings:

const actions = {
// App management actions
installApp: Action.create('Install App'),
uninstallApp: Action.create('Uninstall App'),
refreshApps: Action.create('Refresh Apps'),
appInstalled: Action.create('An app was installed'),
// App store actions
loadAppHub: Action.create('Load DHIS2 App Hub'),
installAppVersion: Action.create('Install App Version from DHIS2 App Hub'),
// Snackbar
showSnackbarMessage: Action.create('Show Snackbar message'),
}

I know it is possible to apply i18n to these Actions. At first glance it seems you could wrap the action name in i18n.t(), like so:

 installApp: Action.create(i18n.t('Install App')),

If this is not providing the expected results, I think you could check either the user-app or the maintenance app to see how they've implemented things.

And quite likely this is ending up as hardcoded text in the UI too:

const appManagementAppName = 'App Management'

I'm not sure we want to translate the app-name, but if we do, this needs to be wrapped in a call to i18n.t()

@KaiVandivier
Copy link
Contributor Author

Ah thanks @HendrikThePendric, I wasn't aware of those :) I'll fix those up too

@KaiVandivier
Copy link
Contributor Author

Added translations for those actions.

It looks like 'appManagementAppName' is used to find the app on the app hub, and is never displayed, so it doesn't need a translation 👍

const appManagementApp = appHub.apps.find(
app =>
app.developer.organisation === appManagementAppOrg &&
app.name === appManagementAppName
)

@KaiVandivier KaiVandivier merged commit bc58ad2 into master Apr 1, 2021
@KaiVandivier KaiVandivier deleted the fix-translations branch April 1, 2021 10:24
dhis2-bot added a commit that referenced this pull request Apr 1, 2021
## [28.2.6](v28.2.5...v28.2.6) (2021-04-01)

### Bug Fixes

* translate headers, menu actions, searchbar, and 'missing' messages ([#199](#199)) ([bc58ad2](bc58ad2))
@dhis2-bot
Copy link
Contributor

🎉 This PR is included in version 28.2.6 🎉

The release is available on GitHub release

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.

3 participants