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

Add i18n-js gem #97

Merged
merged 8 commits into from
Apr 23, 2019
Merged

Add i18n-js gem #97

merged 8 commits into from
Apr 23, 2019

Conversation

rosle
Copy link
Contributor

@rosle rosle commented Apr 12, 2019

What happened

✅ Add I18n-js gem

✅ Add devise-i18nand rails-i18n gems that is optional for including many languages translations for Devise and Rails (Suggested by Yutna 👍)

Insight

Due to issue #6 , we would like to add the i18n-js by default because we use it on most of the project.

I18n Configurations

The files locate at:

  • The library - vendor/assets/javascripts/i18n.js
  • The translation file - app/assets/javascripts/translations/translations.js

On config/development.rb, add config.middleware.use I18n::JS::Middleware, so it will regenerate translations file.

For production we should run rake i18n:js:export before deployment.

Proof Of Work

Configuration added:

  • config/development.rb
    Screen Shot 2562-04-12 at 18 40 56

  • app/assets/javascripts/application.js
    Screen Shot 2562-04-12 at 18 43 25

The translation file gets generated when running the site on development
Screen Shot 2562-04-12 at 18 45 36

I create a simple page for test and alert a translation key.
Screen Shot 2562-04-12 at 18 39 12
Screen Shot 2562-04-12 at 18 39 06
Screen Shot 2562-04-12 at 18 39 24

@rosle rosle added this to the 3.2.0 milestone Apr 12, 2019
@rosle rosle changed the title Add i18n-js gem Add i18n-js gem - WIP Apr 12, 2019
@rosle rosle force-pushed the chore/add-i18n-js branch from a279325 to 415b895 Compare April 17, 2019 04:14
@rosle rosle changed the base branch from chore/cleanup-gems to develop April 17, 2019 04:14
@rosle rosle force-pushed the chore/add-i18n-js branch from 415b895 to 59d8af0 Compare April 17, 2019 04:16
@rosle rosle changed the title Add i18n-js gem - WIP Add i18n-js gem Apr 17, 2019
lib/i18n_js.rb Outdated

# Ignore i18n.js generated files
/vendor/assets/javascripts/i18n.js
/app/assets/javascripts/translations/translations.js
Copy link
Member

Choose a reason for hiding this comment

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

@rosle For this config, when we want to deploy the project to Heroku we can't push those files to Heroku.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added more comment regarding deployment on Heroku 🚀

@rosle
Copy link
Contributor Author

rosle commented Apr 18, 2019

@olivierobert I checked with @anduonghien about the i18n-js. The language switching does not work out of the box. We will need to set the current locale like we do in Rails part (that we read locale from param and set it in the controller).

For javascript part, What we did for other project is we set lang attribute on html tag and use js to read and set that value to i18n-js.

So currently, we're not sure if we should include all that setup here or we let the developer implement it for each project. 💭 I try to check for other template but I don't find anyone including that.

@rosle rosle force-pushed the chore/add-i18n-js branch from d218f54 to db85977 Compare April 18, 2019 07:55
@andyduong1920
Copy link
Member

@rosle How about we generate the i18n.js file that includes

'use strict';

I18n.locale = document.getElementsByTagName('html')[0].getAttribute('lang');

and put it in the appplication.js file

@olivierobert
Copy link
Contributor

olivierobert commented Apr 20, 2019

@rosle Since it's used in all projects without any exceptions, I would recommend to add the whole setup in this PR. We should move the controller code into a concern (as done on Braive) so that it's easy to isolate the localization code.

@rosle
Copy link
Contributor Author

rosle commented Apr 22, 2019

@olivierobert @anduonghien I added the script to generate initial 18n setup on controller and js. After the project creation, it will look like this

application_controller.rb

  • include the new concerns - Localization
class ApplicationController < ActionController::Base
  include Localization
end

application.js

  • Automatically require initializers and screens
  • The i18n setup script is in the initializers directory
//= require rails-ujs
//= require activestorage
//= require i18n
//= require translations/translations
//= require initializers/index
//= require screens/index

And also add the lang attribute on the page for js to setup the locale
Screen Shot 2562-04-22 at 12 25 36

@olivierobert
Copy link
Contributor

As discussed the only remaining change is not to include the translations/translations inside the manifest file. Instead, it needs to be added in the default layout. The rationale behind it is that translation do not change that much so can be cached.

@rosle
Copy link
Contributor Author

rosle commented Apr 22, 2019

@olivierobert @anduonghien I updated the i18n to be included in layout instead.

layouts/application.html.erb

  • Add i18n and translations ⬇️
<%= stylesheet_link_tag    'application', media: 'all', 'data-turbolinks-track': 'reload' %>
<%= javascript_include_tag 'i18n' %>
<%= javascript_include_tag 'translations/translations' %>
<%= javascript_include_tag 'application', 'data-turbolinks-track': 'reload' %>

Need to precompile new assets in config/initializers/assets.rb

Rails.application.config.assets.precompile += %w( i18n.js translations/translations.js )

@rosle rosle merged commit 71bc0f9 into develop Apr 23, 2019
@rosle rosle mentioned this pull request Apr 23, 2019
@svnlto svnlto deleted the chore/add-i18n-js branch May 2, 2019 01:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants