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

Ensure cookie contains known locale #221

Merged
merged 1 commit into from
Oct 31, 2020

Conversation

jimohalloran
Copy link
Contributor

This is an alternate fix for @bmpf issue #213 ... The originally suggested fix of modifying the blade template directly referencing superglobals felt a bit messy to me. Givewn that the issue is the Vuex store being initialised with an invalid value, I thought it'd be better to fix the issue in the store itself, and have the store validate it's data on initialisation.

@bmpf
Copy link

bmpf commented Nov 2, 2019

Top :)

@bmpf
Copy link

bmpf commented Nov 15, 2019

Hi @jimohalloran @cretueusebiu

This 'fix' ll cause another bug. On the first load the i18n text wont load properly.

Tested on a private tab.

See Here
image
image

@jimohalloran
Copy link
Contributor Author

I can't reproduce what you're describing @bmpf ... On first page load the translated text always appears as expected, even when loaded in a private window (tried both Chrome and Safari). The translation strings in the screenshot you've provided aren't in an unmodified version of this project. Are you 100% sure this is a problem with the code from this project and not something you've added?

@jimohalloran
Copy link
Contributor Author

I wonder if this is related to #223 ? Maybe under some circumstances the .json file isn't loaded, or isn't loaded early enough?

@bmpf
Copy link

bmpf commented Dec 1, 2019

@jimohalloran , open a private tab, and enter the url . Has to be the very first load. After that all works fine.

@cretueusebiu cretueusebiu changed the base branch from master to next October 31, 2020 17:50
@cretueusebiu cretueusebiu merged commit 762680b into cretueusebiu:next Oct 31, 2020
@cretueusebiu cretueusebiu mentioned this pull request Oct 31, 2020
10 tasks
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.

3 participants