fix(frontend): format sidebar date group labels with the user's locale - #14437
fix(frontend): format sidebar date group labels with the user's locale#14437gaoanze888 wants to merge 1 commit into
Conversation
|
This PR targets the Automatically setting the base branch to |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review. 📜 Recent review details⏰ Context from checks skipped due to timeout. (10)
|
| Layer / File(s) | Summary |
|---|---|
Localized date labels and grouping autogpt_platform/frontend/src/components/layout/AppSidebar/components/RecentChats/helpers.ts, autogpt_platform/frontend/src/components/layout/AppSidebar/components/RecentChats/__tests__/helpers.test.ts |
getDateGroupLabel uses Intl.DateTimeFormat and omits the year for current-year dates. getDateGroupLabel and groupSessionsByDate accept an optional locale. Tests validate localized labels and grouping behavior. |
Priority: ➖ Normal — Impact reflects medium issue severity.
Estimated code review effort: 2 (Simple) | ~10 minutes
Severity of issue fixed: Medium
Merge Risk: ⚪ Minimal · up to 750a1
Recent Chat date labels now use locale-aware formatting with localized test coverage; no merge-blocking risk is identified.
🚥 Pre-merge checks | ✅ 4 | ❌ 1
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (4 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title check | ✅ Passed | The title clearly identifies the main change: formatting sidebar date-group labels with the user's locale. |
| Description check | ✅ Passed | The description explains the localization problem, the Intl.DateTimeFormat fix, affected APIs, tests, and verification results. |
| Linked Issues check | ✅ Passed | The changes satisfy issue #13519 by replacing hand-assembled date labels with consistent locale-aware formatting and adding locale coverage in tests. |
| Out of Scope Changes check | ✅ Passed | The changes are limited to Recent Chats date-label formatting, related API parameters, and corresponding tests. No unrelated changes are present. |
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
- Create stacked PR
- Commit on current branch
🧪 Generate unit tests (beta)
- Create PR with unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
3f3406d to
c8b8c9d
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
c8b8c9d to
58067a0
Compare
The RecentChats day-group labels hand-assembled English-style date parts
around a localized month name (`26th June`, `${day}${suffix} ${month}`),
which is not a valid format for any locale — e.g. zh users saw "20th 六月".
Replace the hand-assembly with a single Intl.DateTimeFormat options set
(day/month/year), so each locale gets its own ordering and punctuation
("June 20", "20 June", "20. Juni", "6月20日"). The year is still only
included for dates outside the current year. Tests now pass an explicit
locale for determinism and cover en-GB/de-DE/zh-CN output.
Fixes Significant-Gravitas#13519
58067a0 to
750a1a1
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev #14437 +/- ##
=======================================
Coverage 81.34% 81.34%
=======================================
Files 3515 3515
Lines 263403 263394 -9
Branches 24413 24411 -2
=======================================
- Hits 214278 214271 -7
+ Misses 43780 43764 -16
- Partials 5345 5359 +14
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Problem
The new sidebar's "Recent chats" day-group labels mix a localized month name with hand-assembled English date parts, producing a format that is incorrect for every locale (#13519):
The month name follows the user's locale, but the
day + English ordinal + "day month year"assembly does not. A zh-CN user sees20th 六月(correct would be6月20日); a de-DE user sees20th Juni(correct:20. Juni).This is reproducible on
master— the existing unit tests actually fail on machines with a non-English locale (expected '20th 六月' to be '20th June'), because the expectations hardcode English output.Fix
Format the label with a single
Intl.DateTimeFormat(locale, …)options set, per the suggestion in #13519:{ day: "numeric", month: "long" }for dates in the current yearyear: "numeric"for older datesResult per locale:
June 20/December 1, 2024(en-US),20 June(en-GB),20. Juni(de-DE),6月20日/2024年12月1日(zh-CN). The customordinalSuffixhelper is gone — English ordinals can't be expressed viaIntl, and keeping them is what made the format locale-invalid.getDateGroupLabelandgroupSessionsByDatenow accept an optionallocale(defaulting to the user's locale), mirroringformatHeaderDateinhome/helpers.ts, which keeps tests deterministic.Test plan
RecentChats/helpers.test.ts: expectations pass an explicit"en-US"locale (no longer runner-locale-dependent), plus new assertions foren-GB,de-DE, andzh-CNcovering the previously broken mixed formatpnpm vitest run src/components/layout/AppSidebar/...— 11/11 passpnpm format,pnpm lint,pnpm types— cleanhome/tests are unrelated (they fail on unmodifiedmasterunder a non-en-USlocale —Intl.NumberFormatcurrency prefix; same class of issue as this one, out of scope here)Fixes #13519