gavin/maybe
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 28 Wiki Activity

Start I18n Internationalization setup #276

Merged
ricsdeol merged 5 commits from feat/internationalization_setup into main 2024-02-07 01:58:17 +08:00
Conversation 12 Commits 5 Files Changed 33 +177 -102
ricsdeol commented 2024-02-03 23:22:33 +08:00 (Migrated from github.com)
Copy Link

Start Internationalization Setup

Close #256

TODOS:

  • Models
  • Layouts
  • Registration
  • Sessions
  • Passwords
  • Pages
  • Account
### Start Internationalization Setup Close #256 #### TODOS: - [x] Models - [x] Layouts - [x] Registration - [x] Sessions - [x] Passwords - [x] Pages - [x] Account
adrienpoly commented 2024-02-04 23:11:36 +08:00 (Migrated from github.com)
Copy Link

tiny detail but some files are .yml some are .yaml

tiny detail but some files are `.yml` some are `.yaml`
👍 1
ricsdeol (Migrated from github.com) reviewed 2024-02-05 05:15:46 +08:00
app/models/account/credit.rb
@@ -4,4 +3,1 @@
def type_name
"Credit Card"
end
end
ricsdeol (Migrated from github.com) commented 2024-02-05 05:15:46 +08:00
Copy Link

Can use: credit.model_name.human

Can use: `credit.model_name.human`
ricsdeol (Migrated from github.com) reviewed 2024-02-05 05:16:54 +08:00
app/views/accounts/new_credit.html.erb
ricsdeol (Migrated from github.com) commented 2024-02-05 05:16:54 +08:00
Copy Link

isn't used in any action

isn't used in any action
ricsdeol commented 2024-02-05 05:38:14 +08:00 (Migrated from github.com)
Copy Link

@Shpigford i thinks the initial setup is done.

@Shpigford i thinks the initial setup is done.
Shpigford commented 2024-02-05 07:39:18 +08:00 (Migrated from github.com)
Copy Link

@ricsdeol Please resolve conflicts + failing tests.

@ricsdeol Please resolve conflicts + failing tests.
ricsdeol commented 2024-02-05 21:20:23 +08:00 (Migrated from github.com)
Copy Link

@Shpigford I fixed the missing keys, however the unused test is we can't use, it doesn't recognize when using some translations dynamically like here:

  <%= link_to new_account_path(type: type.class.name.demodulize), class: "flex flex-col items-center justify-center w-full text-center focus:outline-none" do %>
    <span class="absolute inset-0" aria-hidden="true"></span>
    <span class="flex w-10 h-10 shrink-0 grow-0 items-center justify-center rounded-xl <%= bg_color %> mb-2">
      <%= inline_svg_tag(icon, class: "#{text_color} stroke-current") %>
    </span>
    <%= type.model_name.human %>
  <% end %>

or

<% Current.family.accounts.each do |account| %>
    <div class="flex items-center justify-between px-3 py-3 mb-2 bg-white shadow-sm rounded-xl">
      <div class="flex items-center text-sm">
        <%= account.name %>
      </div>
      <div class="flex items-center text-sm">
        <%= account.accountable.model_name.human %>
      </div>
      <p class="text-sm text-right">
        <span class="block mb-1"><%= number_to_currency account.balance %></span>
      </p>
    </div>
<% end %>

or others 'rails default' like here:

en:
  date:
    abbr_day_names:
    - Sun
    - Mon
    - Tue
    - Wed
    - Thu
    - Fri
    - Sat
    abbr_month_names:
    -
    - Jan
    - Feb
    - Mar
    - Apr
    - May
    - Jun
    - Jul
    - Aug
    - Sep
    - Oct
    - Nov
    - Dec
    day_names:
    - Sunday
    - Monday
    - Tuesday
    - Wednesday
    - Thursday
    - Friday
    - Saturday
    formats:
      default: "%Y-%m-%d"
      long: "%B %d, %Y"
      short: "%b %d"

which is a good practice to facilitate translation into other languages or add date custom formats.

I suggest disabling this test for now, then we can enable and check how to configure these cases.

@Shpigford I fixed the missing keys, however the unused test is we can't use, it doesn't recognize when using some translations dynamically like here: ```erb <%= link_to new_account_path(type: type.class.name.demodulize), class: "flex flex-col items-center justify-center w-full text-center focus:outline-none" do %> <span class="absolute inset-0" aria-hidden="true"></span> <span class="flex w-10 h-10 shrink-0 grow-0 items-center justify-center rounded-xl <%= bg_color %> mb-2"> <%= inline_svg_tag(icon, class: "#{text_color} stroke-current") %> </span> <%= type.model_name.human %> <% end %> ``` or ```erb <% Current.family.accounts.each do |account| %> <div class="flex items-center justify-between px-3 py-3 mb-2 bg-white shadow-sm rounded-xl"> <div class="flex items-center text-sm"> <%= account.name %> </div> <div class="flex items-center text-sm"> <%= account.accountable.model_name.human %> </div> <p class="text-sm text-right"> <span class="block mb-1"><%= number_to_currency account.balance %></span> </p> </div> <% end %> ``` or others 'rails default' like here: ```yml en: date: abbr_day_names: - Sun - Mon - Tue - Wed - Thu - Fri - Sat abbr_month_names: - - Jan - Feb - Mar - Apr - May - Jun - Jul - Aug - Sep - Oct - Nov - Dec day_names: - Sunday - Monday - Tuesday - Wednesday - Thursday - Friday - Saturday formats: default: "%Y-%m-%d" long: "%B %d, %Y" short: "%b %d" ``` which is a good practice to facilitate translation into other languages or add date custom formats. I suggest disabling this test for now, then we can enable and check how to configure these cases.
josefarias commented 2024-02-05 21:31:05 +08:00 (Migrated from github.com)
Copy Link

I suggest disabling this test for now, then we can enable and check how to configure these cases.

Do the suggestions in https://github.com/glebm/i18n-tasks#dynamic-keys help with this?

> I suggest disabling this test for now, then we can enable and check how to configure these cases. Do the suggestions in https://github.com/glebm/i18n-tasks#dynamic-keys help with this?
ricsdeol commented 2024-02-05 21:39:41 +08:00 (Migrated from github.com)
Copy Link

I suggest disabling this test for now, then we can enable and check how to configure these cases.

Do the suggestions in https://github.com/glebm/i18n-tasks#dynamic-keys help with this?

Doesn't work with active record attributes and name like this:

---
en:
  activerecord:
    attributes:
      account:
        balance: Balance
        currency: Currency
        family: Family
        family_id: Family
        name: Name
        subtype: Subtype
    models:
      account: Account
      account/credit: Credit Card
      account/depository: Bank Accounts
      account/investiment: Investments
      account/loan: Loan
      account/other_asset: Other Asset
      account/other_liability: Other Liability
      account/property: Real Estate
      account/vehicle: Vehicle
> > I suggest disabling this test for now, then we can enable and check how to configure these cases. > > Do the suggestions in https://github.com/glebm/i18n-tasks#dynamic-keys help with this? Doesn't work with active record attributes and name like this: ```yml --- en: activerecord: attributes: account: balance: Balance currency: Currency family: Family family_id: Family name: Name subtype: Subtype models: account: Account account/credit: Credit Card account/depository: Bank Accounts account/investiment: Investments account/loan: Loan account/other_asset: Other Asset account/other_liability: Other Liability account/property: Real Estate account/vehicle: Vehicle ```
josefarias commented 2024-02-05 23:13:15 +08:00 (Migrated from github.com)
Copy Link

Doesn't work with active record attributes and name like this:

I think we should ignore those in the config instead of skipping the test. If we skip, enabling down the road will be a pain as we'll be missing context for which keys can safely be ignored.

For that same reason, I'd suggest adding a note to the config about why we're ignoring these in particular and how we are, in fact, using them (i.e. via model_name.human).

I see we use the model names in template. So fine to ignore those. Do we use the attribute names as well? (E.g in validations) If not, I'd avoid adding them now. I18n files can get unruly, so would rather only add what we absolutely need.

> Doesn't work with active record attributes and name like this: I think we should ignore those in the config instead of skipping the test. If we skip, enabling down the road will be a pain as we'll be missing context for which keys can safely be ignored. For that same reason, I'd suggest adding a note to the config about why we're ignoring these in particular and how we are, in fact, using them (i.e. via model_name.human). I see we use the model names in template. So fine to ignore those. Do we use the attribute names as well? (E.g in validations) If not, I'd avoid adding them now. I18n files can get unruly, so would rather only add what we absolutely need.
ricsdeol commented 2024-02-07 00:02:20 +08:00 (Migrated from github.com)
Copy Link

Doesn't work with active record attributes and name like this:

I think we should ignore those in the config instead of skipping the test. If we skip, enabling down the road will be a pain as we'll be missing context for which keys can safely be ignored.

For that same reason, I'd suggest adding a note to the config about why we're ignoring these in particular and how we are, in fact, using them (i.e. via model_name.human).

I see we use the model names in template. So fine to ignore those. Do we use the attribute names as well? (E.g in validations) If not, I'd avoid adding them now. I18n files can get unruly, so would rather only add what we absolutely need.

Thanks @josefarias I did your suggestion.

@Shpigford Please could re-evaluate these changes?

> > Doesn't work with active record attributes and name like this: > > I think we should ignore those in the config instead of skipping the test. If we skip, enabling down the road will be a pain as we'll be missing context for which keys can safely be ignored. > > For that same reason, I'd suggest adding a note to the config about why we're ignoring these in particular and how we are, in fact, using them (i.e. via model_name.human). > > I see we use the model names in template. So fine to ignore those. Do we use the attribute names as well? (E.g in validations) If not, I'd avoid adding them now. I18n files can get unruly, so would rather only add what we absolutely need. Thanks @josefarias I did your suggestion. @Shpigford Please could re-evaluate these changes?
Shpigford commented 2024-02-07 01:58:14 +08:00 (Migrated from github.com)
Copy Link

There's an error but I'm ignoring. Going to merge and then just kill of the system tests as they're getting in the way of our quickly-changing UI.

There's an error but I'm ignoring. Going to merge and then just kill of the system tests as they're getting in the way of our quickly-changing UI.
Shpigford commented 2024-02-07 01:58:26 +08:00 (Migrated from github.com)
Copy Link

Thanks for tackling this, @ricsdeol!

Thanks for tackling this, @ricsdeol!
🎉 1 🚀 1
Sign in to join this conversation.
Reviewers
No Reviewers
ricsdeol
Labels
Clear labels
📆 Planned
Not ready to be worked on yet, but planned for future
 💻 Self Hosted only
Issues pertaining to self-hosted versions of Maybe
 ⚙️ Config
ℹ️ Needs info
Further information is requested
 📱 Mobile
Bugs exclusively happening on mobile devices
 1️⃣ High Priority
High priority issues will be worked on next
 🚨 Urgent
Issues that are urgent for app functionality
 3️⃣ Low priority
Contributions accepted, but Maybe team will not be working on this in the near term
 2️⃣ Medium Priority
Community contributions accepted, Maybe team only works on if there are no high priority items open
 ⚡ Performance
Performance issues to address
 🎨 UI / UX
Issues related to UI / UX interactions and design.
 💎 Bounty
💰 Rewarded
📊 Data issue
Issues where the app functionally works, but the data is missing or inaccurate.
 🙋 Bounty claim
$1.5K
Community
This is a great issue for community members to work on
 dependencies
Pull requests that update a dependency file
 Design
Hosted only
Hosted app only
 Required self hoster data migration
Self hosters must run data migrations to resolve this issue
 ruby
Pull requests that update Ruby code
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
gavin
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: gavin/maybe#276
Delete Branch "feat/internationalization_setup"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?