Leaflet / Leaflet

πŸƒ JavaScript library for mobile-friendly interactive maps πŸ‡ΊπŸ‡¦
https://leafletjs.com
BSD 2-Clause "Simplified" License
40.17k stars 5.75k forks source link

Add i18n helpers in core #9281

Open yohanboniface opened 2 months ago

yohanboniface commented 2 months ago

I need a module way of importing the translate function in uMap. So either I modernize Leaflet.i18n, either I contribute here, which may a better move. So let's try something here :)

Trying to summarize what I think we agreed on (cf #9092):

Two points that still needs discussion and decision:

About the first point, I'm not convinced this is more than a edge case. Leaflet.i18n does not deal with this, and has been used for years without nobody asking for it. I guess adding the feature into Leaflet will give it more visibility and usage, so this may arrise, but let's focus on actual need. Keeping things simple is often a better choice in the long term, so I've made this choice for this PR. Given that the order we call registerLocale is important, one can still make sure the preferred translations are loaded last to be sure they override any possible collision.

About the second point, Leaflet.i18n exposes a script to collect the translatable strings among the code base and put it in a json file, that one can then easily push to Transifex or such service. This comes with some conventions (eg. a json file name lang.json), so maybe in a first step it would be better to keep this outside of Leaflet, and maybe rename Leaflet.i18n to something like leaflet-collect-i18n, so people wanting this feature would integrate it in their workflow.

Translation is a whole world per se, and the API we need to elect is not just about how to make a string translatable in the code. We need to keep things simple even for people using many plugins, people wanting to mix leaflet plugins with their own Javascript code that includes other translatable strings, people wanting to add more languages for their site using Leaflet and plugins, and needing to coordinate all those string in a Transifex like service, etc.

Also, one thing to keep in mind: translate is not lazy, so it cannot be used for example for options in a class definition, or any situation where the function will be executed at load time (unless the registerLocale has been used before of course, which is not the case for Leaflet core but can be for plugins).

When (if!) we are ready to merge, I'll add a tutorial for a better overview on how to set up translations.

As an exercise and a demonstrator, I've used translate internally in this PR for Leaflet strings ('zoom in'…), but I'm not sure it actually makes sense if we do not have the translations alongside. And certainly at this point we should only make sure it's easy and documented to override the internal strings through options. Or maybe it makes sense if those translations could be in a separate package (ping @nfreear) ?

Thanks for your feedback! :)

Falke-Design commented 2 months ago

@jonkoops @mourner @IvanSanchez I would be nice to have your input too but if you have no time to review, please give your OK that we implement translation in Leaflet core.

jonkoops commented 2 months ago

I'll try to get a review in for this, can't tell exactly when that will be.