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

Some comments to localization #989

Closed
PavelVsl opened this issue Dec 9, 2020 · 17 comments
Closed

Some comments to localization #989

PavelVsl opened this issue Dec 9, 2020 · 17 comments

Comments

@PavelVsl
Copy link
Contributor

PavelVsl commented Dec 9, 2020

@hishamco, great job with localization.

I know than it's work in progress and some of this points are planned or obsolete but:

My recommendation in this topic:

  1. Allow use Language Switcher as standalone control in skin. Take it out from Control panel or allow hide it with control panel parameter. I'm not able without modification of Oqtane source switch off Language Switcher in skin.
  2. Make default language and supported languages property of site, add this information to settings similarly to SMTP settings.
  3. Allow set only one language for site without switcher (I have different sites for different languages where content is not 1:1 translation)

New settings for site:
SupportedLanguages string;
DefaultLanguage string;
ShowSwitcher bool;

.

@hishamco
Copy link
Contributor

hishamco commented Dec 9, 2020

Hi @chlupac nice to see you again, regarding localization there are some features still in progress or not planned yet

  1. LanguageSwitcher is already has a Visible property as requested by @sbwalker so you can toggle the visibility programmatically also it's located in Skins if I'm not wrong

  2. Localization options are stored in appsettings.json it could not be in a setting because localization middleware runs early in the pipeline, but we could add it to the settings but change them required to restart the app

  3. Let me think about localization options per tenant

@sbwalker
Copy link
Member

sbwalker commented Dec 9, 2020

There were some comments in #967 which I will move here so they are easier to find:

  • The LanguageSwitcher component was added to the ControlPanel. The benefit to this approach is that every theme will get access to this feature without having to explicitly add the new component. I think this is the correct approach ( as opposed to creating a new LanguageSwitcher component which would need to be explicitly added to every theme ). The only thing I suggest is a property on the LanguageSwitcher to enable/disable its display in a theme. I realize that the language switcher is only visible when multiple supported cultures are defined but there may be cases where someone wants to hide it altogether.

  • There should be an option in the Control Panel for Manage Cultures which would allow the Site Administrator to enable cultures for the current site and specify which culture is the default, The default culture would be used for anonymous users. The current method of storing this information in appsettings.json was a short term solution and does not satisfy our multi-tenancy needs.

  • An authenticated user be able to specify their preferred culture in their user profile. When a user logs in to the system it would use the culture they specified in their profile rather than the site default culture. This allows a site administrator to interact with the site in their native language even if the site caters to users who speak other languages.

@hishamco
Copy link
Contributor

hishamco commented Dec 9, 2020

We could open a separated issues for each so I can closed it when it's done or we could add a checklist here so it could be easy for tracking

Regarding the multitenancy is there already a settings that can stored per tenant, so we can support this feature?

@PavelVsl
Copy link
Contributor Author

PavelVsl commented Dec 9, 2020

  1. Control panel should have parameter ShowLangSwitcher="true" or this should be option for site.
  2. Languages for site can be subset from appsettings.json
  3. Localization options per site, not per tenant, please, mechanism for site settings is already here (SMTP)

@sbwalker
Copy link
Member

sbwalker commented Dec 9, 2020

  1. @chlupac you are correct - based on the goal I described above for allowing users to hide the language switcher component within their theme, the current implementation is not correct. The property should not have been added to the LanguageSwitcher component - it should have been added to the ControlPanel component.

  2. As a general rule it is preferable to keep settings to a bare minimum in appsettings.json. The main technical reason being that Oqtane is a dynamic framework and we want to avoid requiring people to do a deployment in order to change framework configuration. It is much better to encapsulate these settings in the application - even if it requires more effort to implement. One potential option is for the framework to auto-discover the list of languages during startup based on the actual satellite assemblies which exist - then we are not maintaining a list which could get out of sync with reality. As I mentioned above, I instructed hishamco to use appsettings.json as a temporary solution so that he could focus on implementing localization throughout the framework - but it is not the optimal solution.

  3. I agree that we need localization settings per site. I mentioned multi-tenancy because the support for multiple sites is a form of multi-tenancy but I understand how that could be confusing since Oqtane contains a Tenant concept as well. There are many implementation options for allowing sites to manage languages... there could be a new SiteLanguage table introduced if we want to do this in a more relational manner ( more work ) or we could just store a comma-delimited list of languages and a default language in the existing SiteSettings ( less work ). The overarching requirement is that an admin can dynamically specify the languages and default for a site. However, what I was describing in the item above was the ability for a User to be able to specify their own default language as well.

@hishamco
Copy link
Contributor

hishamco commented Dec 9, 2020

@chlupac you are correct - based on the goal I described above for allowing users to hide the language switcher component within their theme, the current implementation is not correct. The property should not have been added to the LanguageSwitcher component - it should have been added to the ControlPanel component.

Fixed in #990

@TomaszGrzmilas
Copy link

Hi,

I want to add Polish translation for oqtane. But I can't see any *.resx files :(

Coult you tell me how to do the translations ?

@sbwalker
Copy link
Member

@TomaszGrzmilas there is a translations repo which was created last week:

https://github.com/oqtane/oqtane.translations

I believe @hishamco is in the process of setting it up for contributions.

@hishamco
Copy link
Contributor

Hi @TomaszGrzmilas as @sbwalker mentioned a new repo is created, I will try to start publish RESX files there, so people can now what should be localized

I was suggested to create a localization extractor tool to simplifiy the process, but if you are ready to contribute in Polish culture,please have a look to oqtane/oqtane.translations#1 so you can start localize your components, controllers .. etc

@sbwalker
Copy link
Member

sbwalker commented Jan 5, 2021

@hishamco in order to complete the implementation of Static Localization do you plan on implementing the options described above:

  1. There should be an option in the Admin Dashboard for Language Management which would allow the Site Administrator to enable languages for the current site and specify which language is the default, The default language would be used for anonymous users.

are you saying above that it is impossible to move away from appsettings.json and use the database to get the default language from the database for a site? It appears the Orchard Core was able to to do this https://github.com/OrchardCMS/OrchardCore/blob/dc1c2016e5fbbafd7033928ebd124d85faf79f37/src/OrchardCore.Modules/OrchardCore.Localization/Startup.cs

  1. An authenticated user be able to specify their preferred culture in their user profile. When a user logs in to the system it would use the culture they specified in their profile rather than the site default culture. This allows a site administrator to interact with the site in their native language even if the site caters to users who speak other languages.

@hishamco
Copy link
Contributor

hishamco commented Jan 5, 2021

There should be an option in the Admin Dashboard for Language Management which would allow the Site Administrator to enable languages for the current site and specify which language is the default, The default language would be used for anonymous users.

This mean we are moving from appsettings.json to database

are you saying that it is impossible to move away from appsettings.json and use the database to get the default language from the database for a site?

Nope, but we should :

  • Call the database before the localization middleware take place

  • Changing the default language from the admin will not take place until the app is restarted - this is a limitation of ASP.NET Core Localization -

An authenticated user be able to specify their preferred culture in their user profile. When a user logs in to the system it would use the culture they specified in their profile rather than the site default culture. This allows a site administrator to interact with the site in their native language even if the site caters to users who speak other languages.

This may be a last step because it requires a new tracking for a user profile culture, so we don't need to rely on ASP.NET Localization cookie here to avoid the conflict that may occurs between the site and admin cultures

@hishamco
Copy link
Contributor

hishamco commented Jan 5, 2021

You can assign this issue to me, I hope to start with soon

@hishamco
Copy link
Contributor

Thinking back about profile language, the issue that is raise in my mind is even I choose another cookie to track the profile language, UICulture need to be set so the localization resources can aware about the selected culture. In this case changing the profile culture will affect the website culture

@sbwalker how you handled this in DNN before?

@sbwalker
Copy link
Member

@hishamco DNN is a traditional WebForms application... so every interaction is based on a request to an ASPX page which is then processed by the ASP.NET pipeline. It uses an HTTP Module to set the culture for the current thread:

https://github.com/dnnsoftware/Dnn.Platform/blob/develop/DNN%20Platform/HttpModules/Localization/LocalizationModule.cs

        /// <summary>
        /// The PreRequestHandlerExecute event happens after AuthenticateRequest. This means this code will
        /// run after we know the identity of the user. This allows for the accurate retrieval of the locale
        /// which depends not just on the site and page locale, but also on the user's preference.
        /// </summary>
        private static void Context_PreRequestHandlerExecute(object sender, EventArgs e)

            // The portalSettings should always be correct at this point, but in case we can't find a portal
            // we need to insure we're not setting the thread locale.
            var portalSettings = PortalController.Instance.GetCurrentSettings();
            if (portalSettings != null)
            {
                Localization.SetThreadCultures(Localization.GetPageLocale(portalSettings), portalSettings);
            }

The Localization class handles the fallback logic for the various levels where the culture can be specified:

https://github.com/dnnsoftware/Dnn.Platform/blob/develop/DNN%20Platform/Library/Services/Localization/Localization.cs

public static CultureInfo GetPageLocale(IPortalSettings portalSettings)

        /// The order in which the language is being detect is:
        ///         1. QueryString
        ///         2. Cookie
        ///         3. User profile (if request is authenticated)
        ///         4. Browser preference (if portal has this option enabled)
        ///         5. Portal default
        ///         6. System default (en-US)

All pages inherit from PageBase which contains a property for PageCulture which is accessible to all modules on a page

https://github.com/dnnsoftware/Dnn.Platform/blob/develop/DNN%20Platform/Library/Framework/PageBase.cs

        public CultureInfo PageCulture
        {
            get
            {
                return this._pageCulture ?? (this._pageCulture = Localization.GetPageLocale(this.PortalSettings));
            }
        }

@hishamco
Copy link
Contributor

Ya it's a WebForms, I knew DNN from version 3 if I'm not wrong, if we looked to the line https://github.com/dnnsoftware/Dnn.Platform/blob/develop/DNN%20Platform/HttpModules/Localization/LocalizationModule.cs#L53 it set the current cultures, this will affect both front-end and back-end cultures if I'm not wrong

@sbwalker
Copy link
Member

sbwalker commented Feb 8, 2021

I believe all of the Localization scenarios above are addressed now. The only thing remaining is populating the localization repo with a full set of RESX files so that people can see what is expected to localize Oqtane.

@sbwalker sbwalker closed this as completed Feb 8, 2021
@hishamco
Copy link
Contributor

hishamco commented Feb 8, 2021

That's what expected to be done in upcoming days ...

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

No branches or pull requests

4 participants