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

Initial support for i18n #54

Merged
merged 45 commits into from
Feb 29, 2024
Merged

Initial support for i18n #54

merged 45 commits into from
Feb 29, 2024

Conversation

nickskyline
Copy link
Member

@nickskyline nickskyline commented Jan 30, 2024

:shipit:

Copy link
Member

@BGMP BGMP left a comment

Choose a reason for hiding this comment

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

Nice job! Take a look at the comments I left. We need those resolved before I can review further.

app/views/application/_nav.haml Outdated Show resolved Hide resolved
app/views/application/index.haml Outdated Show resolved Hide resolved
app/views/sessions/extra.haml Outdated Show resolved Hide resolved
@BGMP
Copy link
Member

BGMP commented Jan 30, 2024

You may request my review when you finish off the changes. Same goes for #53

Thanks, Nick!

@BGMP
Copy link
Member

BGMP commented Feb 28, 2024

I think this is good enough to merge. The language selector is available at the user settings, it has its own tab. Although, I've been thinking that maybe bringing back the on-screen flag icon to change locales would be a good idea.

Could you try the changes out on your end @nickskyline ?

@BGMP BGMP self-requested a review February 28, 2024 05:00
@nickskyline
Copy link
Member Author

nickskyline commented Feb 28, 2024

Looks and works good for me although it's a little weird to me that you have to introduce your password to only change the language.

And yes, bringing back that selector is great, specially for unregistered users. @BGMP

@BGMP
Copy link
Member

BGMP commented Feb 28, 2024

Looks and works good for me although it's a little weird to me that you have to introduce your password to only change the language.

And yes, bringing back that selector is great, specially for unregistered users. @BGMP

The issue with unregistered users is keeping track of their locale. I'm not sure if rails-i18n supports local storage or a solution similar to that...

@BGMP
Copy link
Member

BGMP commented Feb 29, 2024

Alright, I've brought back the language selector with the country flags and everything! We should be good to go now! 😄

@BGMP BGMP merged commit a15d35e into master Feb 29, 2024
3 checks passed
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.

None yet

2 participants