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

Support localization for Label component #771

Closed
hishamco opened this issue Oct 1, 2020 · 26 comments · Fixed by #796
Closed

Support localization for Label component #771

hishamco opened this issue Oct 1, 2020 · 26 comments · Fixed by #796

Comments

@hishamco
Copy link
Contributor

hishamco commented Oct 1, 2020

As @sbwalker mentioned earlier

@hishamco DNN has a custom Label control which handles the display of form labels and help text. The way that the label control handles localization is via an attribute named "ResourceKey". Essentially the value provided for this attribute was used as a prefix when retrieving localized values. So if ResourceKey="FirstName" then it would expect by convention that there would be localization name/value pairs for "FirstName.Text" ( the label ) and "FirstName.HelpText" ( the help text ). This made it very easy for developers to wire up localization - without having to write any code. Oqtane has a similar Label component which is used consistently throughout the admin UI. It would be great if the Label component had a parameter named "ResourceKey" which could be used for the same purpose.

Now we should think about the challenges and use cases to make this happen, hopefully I will share my thought soon ...

@hishamco
Copy link
Contributor Author

hishamco commented Oct 1, 2020

IMHO the localization support can go with two way:

  1. The Standard way that Microsoft are proposed by injecting IStringLocalizer, this to meet Oqtane philosophy

  2. Localize oqtane components, coz all the existing controls may need localization such as alerts, dialogs ... etc, so we can apply the same principle that you proposed for the other controls

I will start a WIP PR and share my thought with you guys

@hishamco
Copy link
Contributor Author

hishamco commented Oct 1, 2020

The initial proposal for the components could be something like:

<Label ResourceKey="FirstName" ResourceType="@typeof(TLocalizationType)">This is Label</Label>

So, there should be localized keys such as FirstName.Text, FirstName.HelpText and so on, according to the component localized properties. TLocalizationType is the type that been used in IStringLocalizer<T>

@sbwalker your thoughts

@sbwalker
Copy link
Member

sbwalker commented Oct 1, 2020

My preference is that a developer should not have to specify the ResourceType property explicitly as it is extra work and seems redundant. The framework knows the component being rendered so it should be possible to internalize the implementation. Perhaps by adding an additional property for ComponentType to the Module model which is what the ModuleState CascadingParameter is based on. Set ComponentType in the router so that it is available to all downstream components.

@hishamco
Copy link
Contributor Author

hishamco commented Oct 1, 2020

My preference is that a developer should not have to specify the ResourceType property explicitly as it is extra work and seems redundant

That's what I pointed to earlier in the discussion, agree with you

Set ComponentType in the router so that it is available to all downstream components.

+1

@hishamco
Copy link
Contributor Author

hishamco commented Oct 4, 2020

@sbwalker I'm able to access module type not the page type, so can we set ComponentType?

@sbwalker
Copy link
Member

sbwalker commented Oct 4, 2020

Can you explain how the type is used? My assumption is that it is the type of the module component being localized? So if a module ( ie. MyModule ) had an index.razor component and it had a bunch of Labels on it, then the type which would be needed for localization is MyModule.Index. Correct? I can’t really answer your question unless I understand the requirements.

@hishamco
Copy link
Contributor Author

hishamco commented Oct 4, 2020

Correct?

Right

@sbwalker
Copy link
Member

sbwalker commented Oct 4, 2020

ModuleState is a cascading parameter which is already available in the Label component ( through ModuleBase ) and it has a property for ModuleType which provides the type of the related module component.

@hishamco
Copy link
Contributor Author

hishamco commented Oct 5, 2020

The type of the module or the parent component that owns the label?

@sbwalker
Copy link
Member

sbwalker commented Oct 5, 2020

The type of the parent component that owns the label

@hishamco
Copy link
Contributor Author

hishamco commented Oct 8, 2020

@sbwalker the issue that i posted here is related to the label, when I try to print out the ModuleStatus ModuleState and ModuleType in the label itself the status is null, to give a little context here, what I did basically is add label component in Default page component

@sbwalker
Copy link
Member

sbwalker commented Oct 8, 2020

what is ModuleStatus - there is no property named ModuleStatus. And I am not sure how it relates to localization?

@hishamco
Copy link
Contributor Author

hishamco commented Oct 8, 2020

Sorry ModuleState it's typo ;)

@hishamco
Copy link
Contributor Author

hishamco commented Oct 8, 2020

And I am not sure how it relates to localization?

To capture the module type as you mentioned above

@sbwalker
Copy link
Member

sbwalker commented Oct 12, 2020

Set a breakpoint in OnParamaterSet() in Oqtane.Client\Modules\Controls\Label.razor
Run Oqtane on Blazor Server
Navigate to Admin Dashboard ( ie. login, click gear icon at top right of UI )
Choose Site Settings
Your breakpoint will be hit
Look at the Locals windows in Visual Studio
Scroll down to ModuleState and open the object
Look for property ModuleType and it will contain the value "Oqtane.Modules.Admin.Site.Index, Oqtane.Client"
This is the fully qualified type name of the parent component

The reason why ModuleState was null for you is because you tried to use the Label component outside of module - which is not a valid use case. The whole idea in Oqtane is that your build modules, and all of the generic components such as Label, etc.. assume that they are being used within an Oqtane module which allows them to access the cascading parameters of PageState and ModuleState which provides access to all contextual properties.

@hishamco
Copy link
Contributor Author

Thanks @sbwalker I will try what you said, but the localization should work with pages too without need hosting a label into a module, Am I right?

@sbwalker
Copy link
Member

I am not sure if I am understanding your question correctly, so please forgive me if I repeat some of the most basic concepts of Oqtane. Oqtane is a "modular framework" so its entire development model is based on modules - not pages. A developer should NEVER create a razor component with an @page directive ( ie, a standard razor page component ). "Pages" are a virtual concept in Oqtane - they are essentially metadata stored in a database which defines a route and which modules should be dynamically rendered when a user navigates to that route. So to answer your question, the Oqtane Label component should NEVER be used anywhere except for within a module.

@hishamco
Copy link
Contributor Author

hishamco commented Oct 12, 2020

the Oqtane Label component should NEVER be used anywhere except for within a module.

If your intend in Oqtane is to use the components inside a module, it's fine, I'm working on the PR now, hope to share a draft soon

I'm struggling with reflection little bit coz you know IStringLocalizer<T> will need a type not a string, the idea is to generate a type and passed as original type to be localized

@sbwalker
Copy link
Member

sbwalker commented Oct 12, 2020

@hishamco I am a bit concerned based on your comments in this thread. Have you created a module for Oqtane? The reason I said the Label component should NEVER be used anywhere except for within a module is because there is no other supported place in Oqtane for using a component.

Type.GetType(ModuleState.ModuleType) will provide you with a type.

@hishamco
Copy link
Contributor Author

hishamco commented Oct 12, 2020

Have you created a module for Oqtane?

Nope, but I used the Oqtane.Blog

The reason I said the Label component should NEVER be used anywhere except for within a module is because there is no other supported place in Oqtane for using a component.

Frankly I tested the Label component when I filed the issue on Default.razor then I realize from your last comment that the label should be place into a module

Type.GetType(ModuleState.ModuleType) will provide you with a type.

I already know that, but I will let you know in my PR why this will may not fit

@hishamco
Copy link
Contributor Author

I think the issue because the generic param should be a Type not an object of type Type

@sbwalker
Copy link
Member

I have no code to look at so I cannot really understand or comment.

@hishamco
Copy link
Contributor Author

No worry I will handle it and share the PR with you

Thanks a lot

@hishamco
Copy link
Contributor Author

@sbwalker can we add localization support to other components or shall we do it after 2.0.0?

@sbwalker
Copy link
Member

It would definitely be useful to implement the ResourceKey concept on other Controls as it makes it much easier for developers to localize their modules.

@hishamco
Copy link
Contributor Author

Ok, I will start some PRs tomorrow to make this happen

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

Successfully merging a pull request may close this issue.

2 participants