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

Hide locale field when only 1 locale is available (#968) #969

Merged
merged 1 commit into from
Apr 18, 2019

Conversation

avsdev-cw
Copy link
Contributor

No description provided.

@lcharette lcharette added the internationalization Related to the localization feature label Apr 18, 2019
@avsdev-cw
Copy link
Contributor Author

FYI - no conflicts caused by merging with hotfix branch either (currently on develop branch)

'hidden' => [],
'disabled' => []
];
if (count($config->getDefined('site.locales.available')) <= 1) {
Copy link
Member

Choose a reason for hiding this comment

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

Could reuse $locales here. Could be mirrored on line 519 for consistency.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Would it be more preferable to use:

        $localePathBuilder = $this->ci->localePathBuilder;

        // Get locale information
        $currentLocales = $localePathBuilder->getLocales();

vs

        // Get a list of all locales
        $locales = $config->getDefined('site.locales.available');

in all cases?

Copy link
Member

Choose a reason for hiding this comment

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

I'm not 100% sure, but I think localePathBuilder will only return the locale associated with the one currently in use? It will at least return them in the inheritance order.

Copy link
Member

Choose a reason for hiding this comment

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

In any case, this behavior will be changed in #850, so I guess it doesn't matters for now.

@lcharette
Copy link
Member

As this doesn't introduce any breaking changes, I think it's safe to merge into hotfix.

@lcharette lcharette modified the milestones: 4.2.x, 4.2.1 Apr 18, 2019
@lcharette lcharette self-assigned this Apr 18, 2019
@lcharette lcharette merged commit d8be6a1 into userfrosting:develop Apr 18, 2019
lcharette added a commit that referenced this pull request Apr 18, 2019
@lcharette
Copy link
Member

So apparently I forgot to change the branch before merging... I'll see what I can do 🤔

@lcharette
Copy link
Member

Ok, fixed. Merged in hotfix for 4.2.1 release. Thanks !

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

Successfully merging this pull request may close these issues.

None yet

2 participants