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

Translation support #2314

Closed
3 tasks
maxokorokov opened this issue Apr 13, 2018 · 5 comments
Closed
3 tasks

Translation support #2314

maxokorokov opened this issue Apr 13, 2018 · 5 comments
Milestone

Comments

@maxokorokov
Copy link
Member

maxokorokov commented Apr 13, 2018

We need to make sure that people can translate strings in ng-bootstrap widget templates. Things to do:

  • list widgets and their parts which might require tranlations
  • select a set of supported translation solutions
  • make the necessary changes to the existing templates and code

The current thinking is that we will support Angular translations so the majority of the process should boil down to adding i18n attributes in the templates of widgets needing translations

@jnizet
Copy link
Member

jnizet commented Apr 15, 2018

Here's a list of labels to mark for i18n:

  • alert.ts: Close
  • carousel.ts: Previous, Next
  • datepicker-navigation.ts: {{ i18n.getMonthFullName(month.number) }} {{ month.year }} (not sure about this, but the order of these two elements could change depending on the locale)
  • pagination.ts: ««, «, (current), », »», First, Previous, Next, Last
  • progressbar.ts: {{getPercentValue()}}% (because some locales might want those in a different order, or with a space between the value and the percent symbol)
  • rating.ts: ★, ☆ (not sure here, but it could be useful to use other symbols by default, without having to provide a specific star template)
  • timepicker.ts: Increment hours, HH, Hours, Decrement hours, Increment minutes, MM, Minutes, Decrement minutes, Increment seconds, SS, Seconds, Decrement seconds, PM, AM

I would also add the following to the things to do:

  • decide about a naming convention for i18n IDs, and apply this convention for each i18n.

My proposal would be to use ngb.<widget-name>.label-in-dash-case. For example, the label "Decrement hours" would be marked using

<span class="sr-only" i18n="@@ngb.timepicker.decrement-hours">Decrement hours</span>

@maxokorokov
Copy link
Member Author

Am I understanding correctly that with this approach:

  • all ng-bootstrap strings will end up in user translation files with default values whether they want it or not?
  • it won't be possible to, say, have 2 pagination widgets with different translations for next/prev buttons?
  • we need to have a separate solution for dynamically translated messages, like for rating's 3 out of 5 message?

@jnizet
Copy link
Member

jnizet commented Apr 27, 2018

@maxokorokov

all ng-bootstrap strings will end up in user translation files with default values whether they want it or not?

Yes, indeed. It would be strange, though, not to translate ng-bootstrap widgets if you translate the remaining of the application, wouldn't it? But indeed, if you only use alert and datepicker (for example), the trans-units for all the other widgets will also be extracted. I guess you can just not translate them.

it won't be possible to, say, have 2 pagination widgets with different translations for next/prev buttons?

No, indeed. I don't see any way of achieving that other then inventing our own translation mechanism (as we sort of did for datepicker month/day labels), using a service that could be provided at the component level. It's currently not possible to change the text, so it's at least a step in the right direction. I know the Angular team is working on a runtime translation mechanism (as opposed to the the current compile-time mechanism), but I don't know if they plan to support such a use-case (i.e. a contextual translation of the same component)

we need to have a separate solution for dynamically translated messages, like for rating's 3 out of 5 message?

I haven't found such a text in the rating's template, but if it existed, it could be translated. Suppose the template contains <span>{{ rate }} out of {{ max }}</span>, we can add the i18n tag on the span (or in any other containing element, including ng-container), and the translator will just have to translate <x id="INTERPOLATION" equiv-text="{{ rate }}"/> out of <x id="INTERPOLATION_1" equiv-text="{{ max }}"/>to (for example, in French) <x id="INTERPOLATION" equiv-text="{{ rate }}"/> sur <x id="INTERPOLATION_1" equiv-text="{{ max }}"/>

@pkozlowski-opensource
Copy link
Member

@maxokorokov I feel like what you are listing are general problems with / limitations of the current Angular system and not something specific to ng-bootstrap use-case. I'm not saying that those problems don't exist, just saying that those should be solved at the framework level rather than on this library level.

As such I would vote for merging @jnizet PR as at least it allows people using Angular built-in method to to translate things. It doesn't mean that we shouldn't try to fix the current translation system. And we should not stop looking for a better translation support. But hey, baby steps :-)

@maxokorokov
Copy link
Member Author

maxokorokov commented Apr 27, 2018

@pkozlowski-opensource @jnizet, oh I have nothing against the PR, on the contrary actually!
Was just trying to think of use cases that default solution doesn't cover and see the impact.

For instance, for angular/material they've decided to have a translation service, ex: https://material.angular.io/components/datepicker/api#MatDatepickerIntl And with the changes: Subject<void> to refresh... Haven't researched why, but most likely real life use cases?

Anyway we shouldn't invent our own translation system right now. And services like in the link above could be added on top / in parallel later if necessary.

@jnizet

I haven't found such a text in the rating's template

Yep, because it's actually in one of methods; But I like your idea as a workaround, I think it should work.

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

Successfully merging a pull request may close this issue.

3 participants