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

Move CultureValue to OC.Localization module #16971

Closed
wants to merge 1 commit into from

Conversation

hishamco
Copy link
Member

@hishamco hishamco commented Nov 8, 2024

To keep things clear and consistent it would be nice to move the CultureValue into the OC.Localization module, this way the fluid value will show up once the Liquid module is enabled

@sebastienros
Copy link
Member

I don't think we should do it. The ILocalizationService is available outside of this module, we have a default implementation. And one could use this helper in liquid to get the current culture, whatever it is, even without the localization module. And another module could implement ILocalizationService too.

@hishamco
Copy link
Member Author

hishamco commented Nov 8, 2024

The ILocalizationService is available outside of this module, we have a default implementation

The default implementation inside the module

And one could use this helper in liquid to get the current culture, whatever it is, even without the localization module. And another module could implement ILocalizationService too.

I might agree if DefaultCulture and SupportedCultures was in Locale object to Culture

@sebastienros
Copy link
Member

DefaultLocalizationService is in OrchardCore. ILocalizationService is in OrchardCore.Abstractions.

DefaultCulture and SupportedCulture are part of this service. So this object will always work even without our localization module. I confirm my recommendtaion.

@hishamco
Copy link
Member Author

hishamco commented Nov 8, 2024

DefaultLocalizationService is in OrchardCore. ILocalizationService is in OrchardCore.Abstractions.

Right

No problem closing this then

@hishamco hishamco closed this Nov 8, 2024
@hishamco hishamco deleted the hishamco/localization-liquid branch November 8, 2024 22:53
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 this pull request may close these issues.

2 participants