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 localized time formatting and update customer dashboard example #2780

Merged
merged 1 commit into from
Feb 18, 2025

Conversation

goosys
Copy link
Contributor

@goosys goosys commented Feb 18, 2025

  • refs Refactor Time-related Fields ahead of upcoming improvements #2702
  • Ensured consistency across public methods for DateTime, Date, and Time, which each had slight differences.
  • Updated Field::Time#time to always go through I18n.localize:
  • Added Customer.example_time to the Dashboard in the example app to check the behavior of the Field::Time .
  • Added tests
    • Added a test to confirm that the existing behavior remains intact.
    • Also added a test for cases where the am/pm translations are undefined.
      • Since our application operates in non-English locales and always includes the rails-i18n gem, we don’t have to worry about am/pm translations being undefined.

@goosys goosys force-pushed the 2702-refactor-time-format branch from 805cde3 to 2000027 Compare February 18, 2025 06:27
This ensures consistency across public methods for `DateTime`, `Date`,
and `Time` which all had slight differences.

`Field::Time#time` now always goes through `I18n.localize`, previously
if the `format` option wasn't specified, it bypassed
`translate_localization_format` and so AM/PM was not localized.

There's now an example on `Customer` so that we can see the behaviour.
@nickcharlton nickcharlton force-pushed the 2702-refactor-time-format branch from 2000027 to 876a250 Compare February 18, 2025 16:41
@nickcharlton nickcharlton merged commit f8d5d33 into thoughtbot:main Feb 18, 2025
25 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.

2 participants