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

Update documentation "Using Subsites in combination with Fluent" part #514

Closed
wants to merge 5 commits into from

Conversation

MLindenhofer
Copy link

References: #509 - the yml-config solution for setting the Fluent locale instead of the Subsite-locale is not existing anymore. I replaced the old solution with a workaround which works for me.

it's my first PR :)

Copy link
Member

@GuySartorelli GuySartorelli left a comment

Choose a reason for hiding this comment

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

Hi,

Thank you for your contribution, and sorry it's taken so long to get back to you. There are some requested changes and clarification questions below.

README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated
*PageController.php*
```
:::php
protected function init(){
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
protected function init(){
protected function init()
{

Copy link
Member

Choose a reason for hiding this comment

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

Please make this change, so that the documentation is showing best practice.

README.md Outdated
Comment on lines 223 to 233
When using Subsites in combination with Fluent module, the Subsites module sets the i18n locale to the language defined in the current Subsite. When this behaviour is not desired and you need to use the locale in FluentState use the following setting in your yml config file:
When using Subsites in combination with Fluent module, the Subsites module sets the i18n locale to the language defined in the current Subsite. When this behaviour is not desired and you need to use the locale in FluentState you can set the current Fluent locale in your PageController like this:

```yaml
SilverStripe\Subsites\Extensions\SiteTreeSubsites:
ignore_subsite_locale: true
*PageController.php*
```
:::php
protected function init(){
if ($this->dataRecord->SubsiteID !== 0){
i18n::set_locale(FluentState::singleton()->getLocale());
}
}
```
Copy link
Member

Choose a reason for hiding this comment

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

Can you please explain why you're making this change? What was wrong with the yaml configuration documented here?

Copy link
Author

Choose a reason for hiding this comment

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

I make this change, because the yaml configuration documented is not in use in the Subsite-Module. So even with the yml configuration set, the i18n-Locale is still the one defined for the subsite and not the one from fluent. My solution is just one possible workaround.

Copy link
Member

Choose a reason for hiding this comment

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

I make this change, because the yaml configuration documented is not in use in the Subsite-Module.

That yaml configuration is used here:

public static function contentcontrollerInit($controller)
{
/** @var Subsite $subsite */
$subsite = Subsite::currentSubsite();
if ($subsite && $subsite->Theme) {
SSViewer::set_themes(ThemeResolver::singleton()->getThemeList($subsite));
}
$ignore_subsite_locale = Config::inst()->get(self::class, 'ignore_subsite_locale');
if (!$ignore_subsite_locale
&& $subsite
&& $subsite->Language
&& i18n::getData()->validate($subsite->Language)
) {
i18n::set_locale($subsite->Language);
}
}

Copy link
Author

Choose a reason for hiding this comment

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

ah I see - okay. In module version 2.6.0 it wasn't there :). Seems it changed in the meanwhile.
Than my change is unnecessary of course .

Copy link
Member

Choose a reason for hiding this comment

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

Okay cool. I'll close this PR then. Thank you for your efforts and patience though, it's definitely appreciated.

revert changes as suggested
Revert part 2
last revert
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