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

User Localization settings #13181

Merged
merged 48 commits into from
Feb 13, 2024
Merged

User Localization settings #13181

merged 48 commits into from
Feb 13, 2024

Conversation

Skrypt
Copy link
Contributor

@Skrypt Skrypt commented Jan 31, 2023

Adds User Localization settings.
Also adds a RequestCultureProvider based on claims. This provider will take precedence over any existing ones allowing to override browser settings and always displaying with unique culture for that user. This is not a User Culture Picker for the frontend but a way to set the culture to a prefered one for each users. A User Culture Picker would be another RequestCultureProvider with a higher priority than this one. This is why I think we need to have this in the Core.

image

@jtkech
Copy link
Member

jtkech commented Jan 31, 2023

Let me know if you agree with the suggested change and then if it still works, so that I can approve.

@Skrypt
Copy link
Contributor Author

Skrypt commented Jan 31, 2023

I think I will wait for tomorrow to continue on this. It doesn't work anyways. The app.UseAuthentication(); is called after app.UseRequestLocalization(requestLocalizationOptions); which cause the HttpContext.User to always be null.

@hishamco
Copy link
Member

@Skrypt if you want me to continue on this today let me know> I know the time zone difference sometime has pros & cons ;)

@Skrypt
Copy link
Contributor Author

Skrypt commented Jan 31, 2023

4:15 am I'm going back to sleep guys. Have fun! 😉

@hishamco
Copy link
Member

I might add some commits if you don't mind

@Skrypt
Copy link
Contributor Author

Skrypt commented Jan 31, 2023

Try it first. Then make the changes.

@hishamco
Copy link
Member

hishamco commented Feb 6, 2023

@Skrypt did the authorization issue still happening?

@Skrypt
Copy link
Contributor Author

Skrypt commented Feb 6, 2023

Yes, it needs to be figured out properly. Don't merge anything yet. I have really low time to spend on this but you get the idea for now.

@hishamco
Copy link
Member

hishamco commented Feb 6, 2023

Don't merge anything yet

No worry, I will not, but I'm asking about the progress of this one in case if you need a help

@hishamco
Copy link
Member

@Skrypt do you have a time to finalize this before 1.6?

@Skrypt
Copy link
Contributor Author

Skrypt commented Feb 28, 2023

Depends when we release 1.6

@hishamco
Copy link
Member

Seems there's only one lucene issue left, which we are waiting for our lucene guru to fix it :)

@Skrypt
Copy link
Contributor Author

Skrypt commented Jan 11, 2024

@Piedone This is a PR that could be easily completed by @hishamco if he wants as he requested a while ago.

@Piedone
Copy link
Member

Piedone commented Jan 11, 2024

Hisham, would you like to take over?

@hishamco hishamco self-assigned this Jan 11, 2024
@hishamco
Copy link
Member

@Skrypt believe it or not I'm thinking two days ago about this PR :)

Hisham, would you like to take over?

Sure I will do that, @Skrypt I might demo this to get any feedback from Seb, unless you demoed before

@Piedone
Copy link
Member

Piedone commented Jan 11, 2024

Awesome!

@Skrypt
Copy link
Contributor Author

Skrypt commented Jan 11, 2024

@hishamco all yours.

@hishamco
Copy link
Member

@Skrypt is there anything else to add or shall I do a final review?

@Skrypt
Copy link
Contributor Author

Skrypt commented Jan 31, 2024

I was thinking about unit tests. Never take time to do them.

@hishamco
Copy link
Member

I was thinking about unit tests. Never take time to do them.

I can do them if the PR is ready now

src/docs/releases/1.9.0.md Outdated Show resolved Hide resolved
src/docs/releases/1.9.0.md Show resolved Hide resolved
Copy link
Contributor

This pull request has merge conflicts. Please resolve those before requesting a review.

# Conflicts:
#	src/docs/releases/1.9.0.md
@Skrypt
Copy link
Contributor Author

Skrypt commented Feb 12, 2024

Should we merge this or wait for unit tests? @Piedone anything else to add here on this PR?

@Piedone
Copy link
Member

Piedone commented Feb 12, 2024

Please re-request review next time (top-right corner) so I can know you're done.

@Skrypt Skrypt requested a review from Piedone February 12, 2024 17:10
…el/UserLocalizationViewModel.cs

Co-authored-by: Sébastien Ros <[email protected]>
…/UserLocalizationDisplayDriver.cs

Co-authored-by: Sébastien Ros <[email protected]>
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.

5 participants