Attempt to localize mapbox UI

[dmytro-gokun]

This is second attempt to implement UI localization, as described here: #8010

@andrewharvey reviewed my previous PR and suggested a better idea, which i'm trying to implement here. For starter, I only implemented localization for FullscreenControl. If the overall approach is approved by contributors, I'll work to add localization for the rest of controls. Also, I'm able to add Russian, Ukrainian & Danish translation (but those should, probably, go as a separate PR or even separate NPM packages).

Any feedback is welcome.

[andrewharvey]

These two could be combined. If an object is passed to uiLanguage it acts as the language string lookup, or if just a string it's the language code.

On the other hand you could say how it is now is clearer 💭 . Interested to see what other people think.

[dmytro-gokun]

@andrewharvey Yes, you are right having it as one of a string or an object might be an option as well. The reasoning behind the route I've taken was that it allows the following scenarios:

1. Only change the default translation for some strings. So, say i specify Swedish as a language and then only "patch" the "Exit fullcsreen" tooltip. If I had to provide the entire list of strings, that would make my code more brittle and susceptible to bugs when new controls/strings added or existing default text is changed. With this "patch" approach i can either change all the strings (thus, providing support for a completely new language/translation) or only few of them.

2. By allowing to "patch" the dictionary, it is easier to add localization support for custom controls. So, ideally, when developing a custom control, one would expose language "packs" for it (and perform the actual translation through this._map._translateUiString('') . Later, a consumer of such control could pass that language "pack" as "uiLanguagePatch" objects. Moreover, if you consume several such controls, you may merge their translation packs, while preserving translation for the default controls.

So, this was my reasoning. May be it is too complicated and a bit simpler approach has to be taken. For example, we could have a central 'localization' repository + some API to patch it. Then, custom controls will have a chance to install their supported languages.

I will wait for some more feedback.

[vakila]

I agree with @andrewharvey that it would be nice to keep this a single option, but I also agree with the rationale for 1&2 here.

What if we had a single option, which always expects an object with a language string and optionally a strings object with the string mappings? Defaulting to { language: 'en' }

In that case I could pass { language: 'uk' } to use the built-in Ukrainian translations, without any overrides, or something like this to provide a partial or full translation patch for a given language:

{ language: 'en', strings: { 'FullscreenControl.ExitTooltip': 'Minimize', ... } }

In that case perhaps renaming the option uiLocalization or something would avoid the awkwardly repetitive uiLanguage: { language: ... }.

Thoughts?

[vakila]

Hi @dmytro-gokun, thanks so much for your work on this! Thanks also for your patience with us taking a while to get back to you. I'm new on the team but will be happy to work with you to move this forward - Really excited to see the project become more internationalized. 😍

In general, I think your approach sounds great - leaving a few comments below on the particulars.

Look forward to hearing your thoughts!

[vakila]

Ultimately I think we should follow what I understand to be the general best practice of pulling all the default (en) strings out of the code and into an en resource module analogous to the uk one you've created. Then ._translateUiString() would become something more like ._getUiString() and would be responsible for returning the default-or-localized version.

[vakila]

As there aren't a lot of strings to deal with (yet), what about putting all the localizations in a single module, as mapbox-gl-geocoder has done?

Importing that one module would then give us the equivalent of the SupportedUiLanguages object here, and in the future we wouldn't have to touch this file to add new imports every time we add a language.

[vakila]

I agree with @andrewharvey that it would be nice to keep this a single option, but I also agree with the rationale for 1&2 here.

What if we had a single option, which always expects an object with a language string and optionally a strings object with the string mappings? Defaulting to { language: 'en' }

In that case I could pass { language: 'uk' } to use the built-in Ukrainian translations, without any overrides, or something like this to provide a partial or full translation patch for a given language:

{ language: 'en', strings: { 'FullscreenControl.ExitTooltip': 'Minimize', ... } }

In that case perhaps renaming the option uiLocalization or something would avoid the awkwardly repetitive uiLanguage: { language: ... }.

Thoughts?

[mourner]

Personally I think we don't have that many localized strings to implement a full-fledged internationalization system. Instead, we could just extract the strings as control options, just like Leaflet does (e.g. here). This would be much simpler, and cover most use cases.

[1ec5]

Personally I think we don't have that many localized strings to implement a full-fledged internationalization system. Instead, we could just extract the strings as control options, just like Leaflet does (e.g. here). This would be much simpler, and cover most use cases.

An approach that includes built-in translations has the advantage of not requiring the developer to obtain (read: possibly pay for) translations into each of the languages they want to fully support. It avoids duplication of effort.

One disadvantage would be the potential for translations to take up more space as the number of languages and translatable strings increases. But GL JS is far from that, as the check-size build would probably show. (It isn’t running on CI because this branch lives on a fork.) Meanwhile, combining the translations into a single module could address the concern about complexity, but anyways most of the PR’s logic pertains to the override options, not the vended translations.

A full-fledged internationalization system would include gl-language and also pull strings in from Transifex like the native SDKs do. 😉 This PR doesn’t go quite that far and doesn’t need to, but maybe someday it might be nice to use Transifex, so that translators can more easily keep translations in sync between the different platform SDKs.

[dmytro-gokun]

@1ec5 Thank you for your feedback. Honestly, i did not update this PR because i was a bit confused about the path that folks prefer to take. Do you think it makes sense to implement what @vakila suggests here: #8095 (comment)?

[interDist]

@dmytro-gokun , @1ec5 , @andrewharvey I hope this PR moves forward soon! The English-only tooltips are an annoying UI bug.

[mourner]

@dmytro-gokun hey, sorry for the delay with responses and thanks a lot for taking on this! Even though localization doesn't change a lot of code, we have to take time coming up with the right direction because committing to a particular approach will have major long-term implications for maintaining the project.

Here are my thoughts personally (other team members might have different opinions):

• We're probably not ready to commit to maintaining a set of official translations for different languages as a part of this repo. But we can set up the API in a way so that it will be easy to plug in community translations some time later — for example, maintaining them in a different repo.
• Since the UI strings are not a critical part of the map experience and we don't have that many to translate, I'd strive for the most minimal and simple API / code possible, avoiding sophisticated solutions.
• The simplest API I can imagine that seeminly still has enough flexibility (e.g. supports partial overrides) and convenience (e.g. all strings in one place) is having a single object with all the UI strings as a map option — e.g. like this:

const map = new mapboxgl.Map({
  ...
  locale: {
    zoomIn: 'Приблизити',
    zoomOut: 'Віддалити'
  }
});

Then we'd merge those over the default English strings like we do for options in general. Then we can support the use case while not having to commit to a particular translation maintenance model right now (given limited bandwidth). Will be happy to hear your thoughts on this.

[vakila]

Hi @dmytro-gokun, thanks again for this work and sorry again for our slow & at times confusing responses - as @mourner wrote we are excited about seeing this get over the finish line, and we've been discussing as a team how we can best support a localization strategy going forward.

Thanks @mourner for capturing the team discussion! I - and I believe the rest of the team - have come to agree that an API like the one above, i.e. where the map has no internal "knowledge" of locale codes or their translations, but only receives a locale object with strings to patch, will be the best path forward. At some point after we get this merged, we can tackle the longer-term question of how to manage translations going forward - noting some of the team's suggestions below for future reference.

@dmytro-gokun and anyone else following along who'd like to weigh in - does that sound reasonable to you? If so, are you still available to work on moving this in that direction, so we can get it merged in?

---

plug in community translations some time later — for example, maintaining them in a different repo.

Some of the ideas suggested so far:

• Do not maintain translations in a centralized place, and merely allow/require developers to pass in the strings they want at map creation
  • This will be the initial situation once we merge this PR, but as @1ec5 pointed out above, in the long run we'd like a better solution to avoid reduplication of effort and support translations being shared and reused in the community. Balancing that with our team's limited ability to support sourcing & maintaining translations will be the trick.
• A plugin or separate repo collecting string patch objects for various locales (e.g. import { ru } from "mapbox-gl-ui-locales"). We'd need to determine whether that would be maintained by Mapbox or the community.
• Separate community-maintained modules for each locale, which users are able to pick and choose from.

[dmytro-gokun]

@vakila @dmytro-gokun Thanks for the update, guys.

To summarize what have to be done now with this PR:

1. Add a new optional property to Map's options named "locale". It is expected to hold a string map.
2. If "locale" is supplied, it will be used. If it is not supplied (or a particular key is missing), the default locale (english, hardcoded within mapbox-gl-js) will be consulted.

If everyone is okay with this, i'll go ahead and update the PR accordingly. Please let me know.

[mourner]

If it is not supplied (or a particular key is missing), the default locale (english, hardcoded within mapbox-gl-js) will be consulted.

@dmytro-gokun the supplied keys will override the default ones, like this:

this._locale = extend({}, defaultLocale, options.locale);

[dmytro-gokun]

@mourner Got it. Thanks.

[dmytro-gokun]

@mourner @vakila Please check it. Is that like you wanted it to be? If it's okay, i'll go ahead and do this for the rest of control (or should that be done in separate PR(s)?)

[mourner]

This is exactly what I was thinking about, thank you for working on this! Added a few nits.

[mourner]

Nit: is there a case where we want key to be returned if the input is supposed to be an id rather than a final string? If not, perhaps we should rename to _getUIString, and throw an error if this._locale[key] is not defined.

[vakila]

Agreed that _getUIString would be a better name, and erroring on a key not defined on the default local seems reasonable.

[mourner]

Nit: do we need to introduce a new folder if it will only contain a single file? Maybe worth moving to just src/ui/default_locale.js.

[mourner]

Also wondering whether we need long namespaced ids if there are not that many strings to define, but it will be clearer when we define them for all controls.

[vakila]

I think the namespaced IDs are OK, and in line with what I understand to be string management best practices.

[mourner]

i'll go ahead and do this for the rest of control (or should that be done in separate PR(s)?

Let's do it in one sweep!

[vakila]

Thanks for the continued work on this @dmytro-gokun ! I generally agree with everything @mourner wrote, and just had a few more questions/thoughts noted below.

[vakila]

I think the namespaced IDs are OK, and in line with what I understand to be string management best practices.

[vakila]

Agreed that _getUIString would be a better name, and erroring on a key not defined on the default local seems reasonable.

[mourner]

Looks awesome to me now, apart from minor linting/flow issues! 🎉🚢 Leaving for @vakila to take a final pass too just in case.

[dmytro-gokun]

Thanks, @mourner! I've now fixed lint errors, but it appear that 'test-render' fails:

failed text-variable-anchor/pitched-with-map

I'm not if this might have been caused by this commit or if it's unrelated..

[mourner]

@dmytro-gokun it's unrelated, just fails sometimes (see #8679). I've restarted the test, should be green soon.

[vakila]

Looks great to me! Thanks so much for all your work on this @dmytro-gokun ❤️ - so excited to see this get merged in! 🎉 🙌

[dmytro-gokun]

Thanks, guys