search

OrchardCMS / Orchard

Orchard is a free, open source, community-focused Content Management System built on the ASP.NET MVC platform.
https://orchardproject.net
BSD 3-Clause "New" or "Revised" License
2.38k stars 1.12k forks source link

There is no localizable DisplayText property for most of elements #5853

Open armanforghani opened 9 years ago

armanforghani commented 9 years ago

Thanks to @sfmskywalker for adding localizable DisplayText and Description property for Element base class. But most of elements does not override these properties so they are not localizable. Only 6 elements in Orchard.Layouts overrides these properties. Interestingly, no element in Orchard.DynamicForms overrides these properties. I think a main module such as Orchard.Layouts should be localizable.

Dear Orchard developer: Please don't use Type, Id, ... as display text. Please consider localization. Now this is one of the big challenges for non-English users. We should keep in mind that English is not the only language in the world.

armanforghani commented 8 years ago

/cc @sfmskywalker, @sebastienros, @Piedone Orchard 1.9.2 has been released but this problem still remains. I can send PR to fix this issue.

Piedone commented 8 years ago

Yes please!

sfmskywalker commented 8 years ago

I'm not sure I understand. The base Element class implements the DisplayText property which is of type LocalizedString, and the implementation of that property invokes the T delegate, so even though the display name is based on the .NET type of the element by default, that string should be localizable. Unless I'm missing something?

armanforghani commented 8 years ago

@sfmskywalker Now in Orchard there are cases that we don't have such DisplayText property to use for localization. But if we just want to talk about Orchard.Layouts, Yes. There is DisplayText in Element base class but I had no success to localize this property when inherited class does not override it with explicit string. Also i cannot find an entry for these elements in PO files. (for example for Paragraph) This may be for localizable string extractor method. Please be aware that i add an entry for these elements in PO file manually. But no success.

Also, Unfortunately, I must say that it is disappointing that some element display name is hard-code in JavaScript files such as Toolbox.js and is not localizable.

sfmskywalker commented 8 years ago

Ah I think I see what you mean. Because the T delegate is injected into a base class, the context is that of the base class, not the derived one, so no translation will be found. In which case I totally agree and we should actually even force derived element types to implement this property and remove the virtual modifier on the base type.

If you can send a PR that would be awesome. Thanks!

sfmskywalker commented 8 years ago

lso, Unfortunately, I must say that it is disappointing that some element display name is hard-code in JavaScript files such as Toolbox.js and is not localizable.

Yes this is sad. You are probably referring to the hardcoded Row elements? This is work that has yet to be completed where the toolbox items are provided by the element themselves instead of being hardcoded. Could make for a cool PR as well. :)

armanforghani commented 8 years ago

Sure, I'll send PR as soon as possible. Thank you for your guidance.

armanforghani commented 8 years ago

@sfmskywalker I removed virtual modifier and replaced it with abstract in Element base class. Also i added implementation for DisplayText property in my element class but i cannot localize display text of element again. Can you help me please?

armanforghani commented 8 years ago

/cc @sfmskywalker , @Piedone It seems DisplayText of element is not localizable at all even if you override base class property.

armanforghani commented 8 years ago

/cc @sfmskywalker , @DanielStolt Can you please check this issue?

I also saw several places you used DisplayText to create HTML class name element.DisplayText.Text.HtmlClassify(). This is not good idea because DisplayText may contain non English character.

sfmskywalker commented 8 years ago

I will, thanks.

sfmskywalker commented 8 years ago

@sebastienros I'm wondering if at this point this should be done for 1.9.x or dev. If there will be a 1.9.4, then 1.9.x makes sense. Otherwise dev.

armanforghani commented 8 years ago

@sfmskywalker @sebastienros Please pay enough attention to non-English speakers. I understand that localization issues may not vital for English speakers, but they are very serious and important issues for non-English speakers. Please consider the next release for this type of issues. Even I think we need to have an hot patch for some of this type of issues.