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

[Feature]: Adding Callback to Set Locale #52330

Closed
wants to merge 5 commits into from
Closed

[Feature]: Adding Callback to Set Locale #52330

wants to merge 5 commits into from

Conversation

joaopalopes24
Copy link
Contributor

I want to add the locale dynamically within the Number class.

Example:

use Illuminate\Support\Number;
 
/**
 * Bootstrap any application services.
 */
public function boot(): void
{
    Number::setLocale(function (): string {
        return Auth::user()?->locale ?? config('app.locale');
    });
}

Copy link

Thanks for submitting a PR!

Note that draft PR's are not reviewed. If you would like a review, please mark your pull request as ready for review in the GitHub user interface.

Pull requests that are abandoned in draft may be closed due to inactivity.

@joaopalopes24 joaopalopes24 marked this pull request as ready for review July 30, 2024 16:31
@devajmeireles
Copy link
Contributor

devajmeireles commented Jul 30, 2024

I want to add the locale dynamically within the Number class.

Example:

use Illuminate\Support\Number;
 
/**
 * Bootstrap any application services.
 */
public function boot(): void
{
    Number::setLocale(function (): string {
        return Auth::user()?->locale ?? config('app.locale');
    });
}

You can use withLocale for a single-use only or useLocale:

Number::useLocale(Auth::user()?->locale ?? config('app.locale'));

@joaopalopes24
Copy link
Contributor Author

Number::useLocale(Auth::user()?->locale ?? config('app.locale'));

This doesn't work because Auth::user() in this case will always be null within the AppServiceProvider.

@devajmeireles
Copy link
Contributor

devajmeireles commented Jul 30, 2024

Number::useLocale(Auth::user()?->locale ?? config('app.locale'));

This doesn't work because Auth::user() in this case will always be null within the AppServiceProvider.

Have you tried using middleware? Setting this at the service provider is more logical for situations where the locale comes from a config rather than the user.

Furthermore, your PR breaks principles by using the ensureIntlExtensionIsInstalled function to define the language. This function should only be used to handle intl checking I guess.

@joaopalopes24
Copy link
Contributor Author

Why would I make middleware, when I can define a callback in my service provider, just like Laravel uses in other resources?

https://laravel.com/docs/11.x/passwords#reset-link-customization

It's a feature that I find interesting.

@joaopalopes24 joaopalopes24 deleted the adding-callback-to-set-locale branch July 30, 2024 18:48
@devajmeireles
Copy link
Contributor

Why would I make middleware, when I can define a callback in my service provider, just like Laravel uses in other resources?

https://laravel.com/docs/11.x/passwords#reset-link-customization

It's a feature that I find interesting.

I mentioned the middleware thinking about getting the current authenticated user, that doesn't exist in the service provider layer.

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