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

feature: localization control suggestion #2039

Merged
merged 12 commits into from
Oct 11, 2024
Merged

Conversation

Grammostola
Copy link
Contributor

@Grammostola Grammostola commented Sep 6, 2024

Aims to resolve #900

This is the result of activity 2.10 in this year's management plan and the discussions we've had in our dev meetings.
It introduces a way of localizing visible strings and numbers. As previously agreed the map needs to reload in order to set a new language. (Some day we might have a separate discussion regarding the components ability to refresh the the dom or parts of it.)

It is a draft because the method is a suggestion. I do want feedback still : )

..And the way to configure it is as such, in the map config json:

  {
    "name": "localization",
    "options":{
      "localeId": "sv-SE",
      "fallbackLocaleId": "en-US",
      "showLocMenu": true
    }
  }

where the fallbackLocaleId is a potentially more filled out locale.json and what the string lookups will target if there's no match in the localeId.

The localeId sets the map locale at start, though if a query param specifies a different locale then the different locale will load. The query param is set by the gui language switcher. It is loc=sv-SE for instance.

It provides:

  • A menu item to select a new locale (or not - is configurable)
  • The current locale can be set via a query param
  • The string lookup is a function
  • The api or a plugin can be utilized to add new locales (for a session)
  • 2024-09-27: A plugin can utilize the localization component in order to localize itself by first adding its language file to the corresponding Origo locale

Developer / translator experience:

  • The locale definitions are kept as json files, one per locale
  • The json structure is partially employed on key lookup - the target key and its parent can be specified
  • All controls receive the localization component as an option
  • The hardcoded strings in the code can be replaced gradually, perhaps control by control
  • Once a single string is converted the localization control needs to be present

animated gifs

  • The measure control (numbers and strings)
    measure_sm

  • Adding a locale
    add_locale_sm

Missing:

  • Sharemap. Save a new query param (besides "mapStateId")
  • Polish on the gui bit. 💊 . Nothing else scrolls when the window gets smaller and this dropdown currently only scrolls if the language list is long enough.

What are your thoughts?

@tonnyandersson
Copy link
Collaborator

Great stuff, @Grammostola!

Two minor things:

  • Origo throws a fit if the localization control hasn't been added to the config.
  • The arrow in the menu is a bit misaligned. I suggest you drop the 'align-self': 'flex-end' bit.

@Grammostola
Copy link
Contributor Author

Thanks for the feedback!
The arrow looks better like you suggested.

As for item number one, it rhymes with

Once a single string is converted the localization control needs to be present

I reasoned it might be a bit like other needed parts of the map config, like the map menu.
The Origo client could be more defensive and assume some default config (because once introduced (and other controls start depending on it) the control cannot leave its host without the host expiring) if the control config is missing.

@Grammostola
Copy link
Contributor Author

It should no longer crash if config for the Localization control is left out and is now a part of (and first of) the default controls. I guess we have a precedent in Fullscreen (default control that is documented because configurable).

@Grammostola Grammostola marked this pull request as ready for review September 19, 2024 13:52
@Grammostola
Copy link
Contributor Author

Grammostola commented Sep 23, 2024

I have been thinking of how to add language support for plugins. Origo loads first, then plugins, so it should be possible with an append-operation:

  function addPluginToLocale(locId, additionObj) {
    if (getLocaleExists(locId)) {
      locales[locId].plugins = { ...locales[locId].plugins, ...additionObj };
      return true;
    } return false;
  }

However plugins typically create their UI (including text) as part of their onInit() method and the reference of the viewer variable is typically not defined until their onAdd() method (for viewer.getControlByName()).

The Origo object defined in origo.js exposes import * as origoControls from './src/controls'; but I'm not sure that that helps me in this instance since we need the specific localizationControl object that Origo instantiates on start.

Any ideas?

Edit. Thanks @steff-o who helped me think outside of my garden:

origo.on('load', function(viewer) {
  const barebone = Barebone({
    buttonText: 'Click this!',
    content: 'Just nonsense',
    viewer
 });

and the plugin can then add its strings to the corresponding language object of the localization component (whenever), and then utilize said component's getStringByKeys method (usually in its onInit method). Seems to work, I shall test a bit more and then push something.

Indeed:
If for instance the example plugin is slightly revised to receive the viewer it can add its language file to the matching locale of Origo like so

import svLocale from './loc/sv_SE.json';

const Barebone = function Barebone(options = {}) {
  const {
    buttonText = 'Default text',
    content = 'Default content',
    viewer
  } = options;

  const icon = '#fa-pencil';
  let bareboneButton;

  let target;
  let modal;

  const localization = viewer.getControlByName('localization');
  localization.addPluginToLocale('sv-SE', svLocale);

and then use it:
(part of a new Origo ui button declared in its init method)

tooltipText: localization.getStringByKeys({ targetParentKey: 'barebone', targetKey: 'tooltipText' }),

where svLocale.js is:

{
    "barebone": {
        "tooltipText": "En långsam låt"
    }
}

@steff-o
Copy link
Contributor

steff-o commented Sep 30, 2024

Seems like a solid implementation. I haven't tested plugins, but pretty much everything else without crashing or behaving like not expected.

Some thoughts:

  • I don't think an additional param is needed for sharemap. Localization is a user setting and should not be a part of a sharemap link.
  • Would it be possible to add the preferred locale id to localStorage and thus make it a personal setting across sessions? If present it would override default locale and be set when user manually changes locale.
  • In the locale files there is an empty entry for "styleTemplate". I can't find any usage of that.
  • The recursive search makes structure redundant and imposes a unique name constraint on targetParentKey and targetKey between controls, plugin and style.
  • Roundtripping custom locales in sessionStorage slows down custom locales as page is reloaded.
    • Workaround is to add custom locales to sessionStorage before Origo is initialized, which may or may not be an intentional usage, or add in source and maintain a forked repo.
    • Would it be a good idea to be able to add additional locales or ovrrides in the localization control's config (in index.json)?
  • Adding a custom locale does not override the existing hard coded unless adding directly to sessionStorage.
  • To be nitpicky, the currlently active language is not highlighted in the menu.
  • Each control could create a utility function to avoid having to call localization.getStringByKeys() with the same targetParentKey each time. In a more object oriented world it could have been a method on Component using the name of the actual implementation class.
  • Maybe the localization Control is so central it should have its own quick access getter in the Viewer like featureInfo instead of calling getControlByName(). When used by other controls, the usage is more like that of a service.

@Grammostola
Copy link
Contributor Author

Grammostola commented Oct 1, 2024

Thanks for the feedback.

  • I don't think an additional param is needed for sharemap. Localization is a user setting and should not be a part of a sharemap link.

Sharing a map set to Arabic should have no bearing on what language the recipient opens it in, I take it. Makes a certain sense. I will not attempt to develop that unless someone can argue for it

  • Would it be possible to add the preferred locale id to localStorage and thus make it a personal setting across sessions? If present it would override default locale and be set when user manually changes locale.

implemented. The query param is ignored if there's the key in localStorage. (In order to get rid of it, clear website history/data). If it specifies an unknown locale id it is ignored

  • In the locale files there is an empty entry for "styleTemplate". I can't find any usage of that.

Will cleanup the language files

  • The recursive search makes structure redundant and imposes a unique name constraint on targetParentKey and targetKey between controls, plugin and style.

As noted in the dev exp section of the op: "The json structure is partially employed on key lookup". At first there was no lookup function, only specified object paths. At that time the json structure was fully employed. Then a request was made to turn it into a function, and a function could take a key value and the language files could be flat lists of unique strings. I think such files are harder to read than json structures though. So to somewhat use the structure the key and its parent must be specified because certain generic key names might occur multiple times. And this is the unique name constraint mentioned. Do you have a different strategy in mind?

  • Roundtripping custom locales in sessionStorage slows down custom locales as page is reloaded.
    Workaround is to add custom locales to sessionStorage before Origo is initialized, which may or may not be an intentional usage, or add in source and maintain a forked repo.

What? :)

  • Would it be a good idea to be able to add additional locales or ovrrides in the localization control's config (in index.json)?

I don't know. What opinions are there? If there's a wish for being able to replace specific strings in a locale then potentially there needs to be a discussion on the locale file structre

  • Adding a custom locale does not override the existing hard coded unless adding directly to sessionStorage.

fixed. In the sense that if you bring a locale that already exists, it had better contain all the strings that you want to see. This relates to the above answer

  • To be nitpicky, the currlently active language is not highlighted in the menu.

It should be and is for me, like in the gifs. Are you referring to something else or is the functionality glitchy?

  • Each control could create a utility function to avoid having to call localization.getStringByKeys() with the same targetParentKey each time. In a more object oriented world it could have been a method on Component using the name of the actual implementation class.

implemented. drawStyles.js is special since merely a collection of functions

  • Maybe the localization Control is so central it should have its own quick access getter in the Viewer like featureInfo instead of calling getControlByName(). When used by other controls, the usage is more like that of a service.

The controls are inited before the Viewer. Quicker access via the viewer would benefit plugins, I guess..?

@steff-o
Copy link
Contributor

steff-o commented Oct 2, 2024

  • The missing indication of active language is because the css file is not imported in origo.scss. Forgot to commit that file?
@import 'localization';
  • I really like the persisting of selected language in localStorage, but if takes precedence over the query param it kind of makes the query param unnecessary. Maybe query param should take precedence, so it is possible to save links that includes locale?

Roundtripping custom locales in sessionStorage slows down custom locales as page is reloaded.
Workaround is to add custom locales to sessionStorage before Origo is initialized, which may or may not be an intentional usage, or add in source and maintain a forked repo.
What? :)

Well, it seemed to me that when adding a custom locale the application is reloaded after the local is added which would the code to start origo and add a custom locale look like:

var origo = Origo('index.json');
// locobj initilized here
origo.api().getControlByName('localization').addLocales([locobj])

The call to addLocales would cause the page to reload, so each time the application is started, it is actually started twice. I guess that is the way it has to be as the origo object does not exists until the Origo function has been called. It could however be avoided if the locale is added directly to localStorage without using an origo api call:

// locobj initilized here
localStorage.setItem(locobj, loc.id);
var origo = Origo('index.json');

But that is probably harder to maintain as an official way of doing it. The other solution to avoid the page reload would be avoid adding custom locales and instead hard code them in the source. Or maybe I'm totally wrong.

@Grammostola
Copy link
Contributor Author

Grammostola commented Oct 2, 2024

I really like the persisting of selected language in localStorage, but if takes precedence over the query param it kind of makes the query param unnecessary. Maybe query param should take precedence, so it is possible to save links that includes locale?

I like it too, your original statement that this is a user config thing rather than a part of a shared map makes sense to me.
Though yes, the only use for the query param presently is in a browser without a origoSelectedLocale key.

The call to addLocales would cause the page to reload, so each time the application is started, it is actually started twice. I guess that is the way it has to be as the origo object does not exists until the Origo function has been called.

The reason for the sessionStorage and rebooting the map upon having added a locale is to change the UI of the localization component i.e change the component's and its resp dom element state. The whole shebang would need to re-render during runtime and I haven't seen any great way of doing that, though a good component way may have just gone 'swoosh' over my head : )

Was also considering whether to allow changing individual keys (parent-target-specified) in an existing locale and that the addLocale function would if its id matched an existing locale rather than replace that traverse the proposed locale and find parent-target-keys and replace these in the target locale. (The latter would be easier with a flat list of unique keys)

@Grammostola
Copy link
Contributor Author

Grammostola commented Oct 3, 2024

Have added the print control. Somewhat annoyingly the ol/control/ScaleLine class does not support manually defining locales for the number format. It picks whatever the browser is set to. If we were to extend the method in that class responsible for this in order to accomodate specifying a locale then this would be a maintenance issue forever that we probably do not want. Better then to create an issue for OL for this if one does not already exist.

I can next:

  • Try to update the loc control UI (dropdown list) without rebooting the map when a locale is added. Spontaneously I would think this involves changing props for components, calling their render to get a html string, then recreating their resp dom elements. Or something like that.
  • Try to implement overriding specific keys and/or that replacing a whole locale would only replace what keys the replacement carries

@steff-o
Copy link
Contributor

steff-o commented Oct 3, 2024

Try to update the loc control UI (dropdown list) without rebooting the map when a locale is added. Spontaneously I would think this involves changing props for components, calling their render to get a html string, then recreating their resp dom elements. Or something like that.

I think being able to add new locales without rebooting would be great. Just fixing the loc control is probably pretty easy. Problem arises when the newly added locale is set to default or active from previous session. Could probably be solved by having all (participating) components listen to some event from loc control. It quickly adds to the complexity to convert a control to locale aware. But with that in place, it would also be possible to change locale on the fly without rebooting.

Alternatively, could it be possible to find a way to add locales before controls are rendered?

  • Index,json?
    • Messy to maintain if there are many applications.
  • New property in options argument to Origo()?
    • Can't think of any drawbacks except that it might offend someone who thinks it is not logical to have all configuration in index.json but localization configuration has its own configuration.
  • "Static" Origo-function that adds to sessionStorage before Origo() is called?
    • If we ever gets to the point when is possible to actually instantiate several origo instances in the same page they will share locale settings, which of course would be to expected behaviour, but could in theory go against some thoughts of separation of scopes.
  • Save to localStorage instead and take one reboot first time application is started.
    • Possibly tricky to handle updates

Try to implement overriding specific keys and/or that replacing a whole locale would only replace what keys the replacement carries

I don't really think that this is necessary, but it is of course nice to have if someone is displeased with only a couple of strings.

@Grammostola
Copy link
Contributor Author

I think being able to add new locales without rebooting would be great. Just fixing the loc control is probably pretty easy. Problem arises when the newly added locale is set to default or active from previous session.

The localization control should be able to handle a config, query param or localStorage key wanting to set a locale that does not exist (i.e ignore / fallback). It would appear logical to save added locales in the localStorage together with the last selected locale id, instead of like now where the preference is localStorage and added locales sessionStorage.

Could probably be solved by having all (participating) components listen to some event from loc control

While I see how events from the localization control being listened to by components that somehow changed their DOM counterparts is a plausible high level description of a scenario that involves not having to reload the map when changing a language (or adding one), I don't quite see how that has to do with the above query. It is a (to me) somewhat lofty goal that might be considered worthwhile (for the project). It would be nice to have I think.

New property in options argument to Origo()?

Can't think of any drawbacks except that it might offend someone who thinks it is not logical to have all configuration in index.json but localization configuration has its own configuration.

To replace the index.json config or merely in order to add locales like so:

const origo = Origo('index.json', {locales: [
    "id": "sv-SE",
    "title": "Svenska",
    "menuTitle": "Språk",
    "controls": {
        "measure": {
            "mainButtonTooltip": "Mäta",
            "startMeasureTooltip": "Klicka för att börja mäta"
	}
    }
]}
);

?

That would perhaps solve not having to reboot the map once a locale had been added (if not choosing the listener way listed above)

@steff-o
Copy link
Contributor

steff-o commented Oct 4, 2024

While I see how events from the localization control being listened to by components that somehow changed their DOM counterparts is a plausible high level description of a scenario that involves not having to reload the map when changing a language (or adding one), I don't quite see how that has to do with the above query. It is a (to me) somewhat lofty goal that might be considered worthwhile (for the project). It would be nice to have I think.

I was just trying to understand what your abition with "Try to update the loc control UI (dropdown list) without rebooting the map when a locale is added. " was by describing what I thought would have to be done when locale is added via api, as it will (as currently implemented) cause a reboot.

To replace the index.json config or merely in order to add locales like so:

const origo = Origo('index.json', {locales: [
"id": "sv-SE",
"title": "Svenska",
"menuTitle": "Språk",
"controls": {
"measure": {
"mainButtonTooltip": "Mäta",
"startMeasureTooltip": "Klicka för att börja mäta"
}
}
]}
);

?

Yes just like the example, but of course in real life the actual json string would probably not be hard coded and instead fetched from a server to re use locale files between applications. But that is up to the implementer of index.html to provide.

That way of setting additional configuration could in general by useful to add other configuration that is not suitable for index.json, for example function pointers for callbacks and plugin hooks that has to be configured before component initialization.

@Grammostola
Copy link
Contributor Author

Grammostola commented Oct 7, 2024

Right, I meant to quote

Problem arises when the newly added locale is set to default or active from previous session

But maybe that is less of a problem now.

The locmenu is now recreated on the spot when addLocales() is used. mapMenu doesn't have a 'removeMenuItem' method but looking at its add method I think I'm proposing a realistic way.

(Had to ignore the no-use-before-define because I couldn't seem to rearrange things to avoid that rule and besides since function declarations are hoisted its complaint is functionally unwarranted)

Having a look at how the same thing might be done for measure (redraw itself) it seems to me, as you indicated, that there's added complexity in that it needs to be rewritten to a degree to allow for "redrawing" or "rerendering and updating its dom elements" easier, and preferrably in a between controls somewhat uniform way. Maybe that's a ui/component thing. It's probably beyond the scope of this PR (and the budget) but it's a nice "second phase" in that I don't think much of what this PR brings would need to be redone.

Copy link
Contributor

@steff-o steff-o left a comment

Choose a reason for hiding this comment

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

Nice work! We better merge this before the merge conflicts stack up.

Sorry for being so obsessed about the possibility to add custom locales, but I'm worried about possible upcoming requirements regarding minority languages that may not concern everybody. However, there is nothing in this PR that has to be rewritten in a non-backwards compatible way to improve the possibilities to add them later. And as it is possible now, I say let get this show on the road!

@Grammostola
Copy link
Contributor Author

@steff-o It's cool and cheers for all of the feedback. Indeed I hope we can improve this control along the way, as well as get all visible numbers and strings inventoried and localized and with that a relatively complete lang.json structure to use as a template for all langs.

@Grammostola Grammostola merged commit 5452190 into master Oct 11, 2024
4 checks passed
@Grammostola Grammostola deleted the localization_feature branch October 11, 2024 12:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Origo could have an translation object/config
3 participants