fix: incorrect year in showMonthAndYearPickers with locale #3331
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
🦋 Changeset detectedLatest commit: 5c111b9 The changes in this PR will be included in the next version bump. This PR includes changesets to release 49 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughThe changes introduced localization enhancements across various components ( Changes
Sequence Diagram(s)N/A Assessment against linked issues
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range and nitpick comments (1)
packages/components/calendar/__tests__/calendar.test.tsx (1)
Line range hint
20-455: Comprehensive Tests for Calendar With LocaleThe tests for
CalendarWithLocaleare comprehensive and appear to cover the necessary scenarios to ensure the calendar behaves correctly across different locales, especially with the Buddhist calendar.It's important to continue expanding these tests to cover all supported locales and calendar systems to ensure comprehensive coverage.
Would you like assistance in expanding these tests to cover more scenarios or locales?
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (8)
- .changeset/purple-singers-knock.md (1 hunks)
- packages/components/calendar/tests/calendar.test.tsx (3 hunks)
- packages/components/calendar/src/use-calendar-base.ts (4 hunks)
- packages/components/date-input/src/use-date-input.ts (4 hunks)
- packages/components/date-picker/tests/date-picker.test.tsx (3 hunks)
- packages/core/system/src/provider.tsx (2 hunks)
- packages/utilities/shared-utils/src/dates.ts (1 hunks)
- packages/utilities/shared-utils/src/index.ts (1 hunks)
Files skipped from review due to trivial changes (2)
- packages/core/system/src/provider.tsx
- packages/utilities/shared-utils/src/index.ts
Additional context used
Biome
packages/utilities/shared-utils/src/dates.ts
[error] 20-20: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
[error] 21-21: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
[error] 22-22: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
Additional comments not posted (5)
.changeset/purple-singers-knock.md (1)
1-8: Changeset Description is Clear and RelevantThe changeset description accurately reflects the updates made in the PR, specifically addressing the locale-related issues in the
showMonthAndYearPickerscomponent. This ensures proper version tracking and clarity for future maintenance.packages/components/date-input/src/use-date-input.ts (1)
Line range hint
120-148: Enhanced Locale Handling in Date InputThe modifications to handle locale-specific year offsets are well implemented. It ensures that the date input respects the calendar system of the specified locale, particularly addressing issues with out-of-bound years.
However, ensure that these changes are thoroughly tested across different locales to verify their effectiveness.
packages/components/calendar/src/use-calendar-base.ts (1)
Line range hint
188-214: Locale-Specific Adjustments in Calendar BaseThe implementation of locale-specific year offsets in the calendar base utility is well executed. This should correctly handle the year discrepancies caused by different calendar systems.
As with the date input, testing across multiple locales is crucial to ensure the robustness of these changes.
Verification successful
Locale-Specific Adjustments in Calendar Base Verified
The initialization of
minValueandmaxValuecorrectly includes thegregorianYearOffset, ensuring that the years are within bounds for different locales. The implementation should handle year discrepancies caused by different calendar systems as intended.
use-calendar-base.ts:minValueandmaxValueincludegregorianYearOffset.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify calendar base adjustments across different locales. # Test: Check for proper functionality across a variety of locales. rg --type typescript $'useCalendarBase'Length of output: 73
Script:
#!/bin/bash # Description: Verify calendar base adjustments across different locales. # Test: Check for proper functionality across a variety of locales. # Correcting the file type to 'ts' for TypeScript files. rg --type ts 'useCalendarBase'Length of output: 769
Script:
#!/bin/bash # Description: Extract usage of useCalendarBase to verify locale-specific adjustments. # Extract relevant code from use-calendar.ts rg -A 10 -B 10 'useCalendarBase' packages/components/calendar/src/use-calendar.ts # Extract relevant code from use-range-calendar.ts rg -A 10 -B 10 'useCalendarBase' packages/components/calendar/src/use-range-calendar.tsLength of output: 2809
Script:
#!/bin/bash # Description: Verify if minValue and maxValue include locale-specific year offsets. # Search for minValue and maxValue initialization in use-calendar.ts rg 'minValue|maxValue' packages/components/calendar/src/use-calendar.ts # Search for minValue and maxValue initialization in use-range-calendar.ts rg 'minValue|maxValue' packages/components/calendar/src/use-range-calendar.tsLength of output: 265
Script:
#!/bin/bash # Description: Extract full initialization context of minValue and maxValue to verify locale-specific year offsets. # Extract full initialization context of minValue and maxValue in use-calendar.ts rg -A 5 -B 5 'minValue|minValue' packages/components/calendar/src/use-calendar.ts # Extract full initialization context of minValue and maxValue in use-range-calendar.ts rg -A 5 -B 5 'minValue|maxValue' packages/components/calendar/src/use-range-calendar.tsLength of output: 1164
Script:
#!/bin/bash # Description: Extract initialization of minValue and maxValue to verify locale-specific year offsets. # Extract initialization of minValue and maxValue in use-calendar-base.ts rg -A 5 -B 5 'minValue|maxValue' packages/components/calendar/src/use-calendar-base.tsLength of output: 863
packages/components/date-picker/__tests__/date-picker.test.tsx (2)
28-46: New ComponentDatePickerWithLocaleAddedThe addition of
DatePickerWithLocaleis a good approach to handle localization within the DatePicker component. WrappingDatePickerBaseinNextUIProviderwith a locale prop ensures that locale-specific settings are applied. This is crucial for maintaining consistency across different locales, especially when dealing with calendar systems that differ significantly from the Gregorian calendar.However, it's essential to ensure that this new component is fully covered by tests, especially since it's designed to handle locale-specific logic which can be prone to edge cases.
651-677: Test Case forDatePickerWithLocalewith Thai Buddhist CalendarThis test case ensures that the
DatePickerWithLocalecomponent correctly handles the Thai Buddhist calendar. The test renders the component with a specific locale (th-TH-u-ca-buddhist) and checks if the displayed month and year are correct according to the Buddhist calendar.It's good to see that the test verifies both the name and the
data-valueattributes of the month and year, which is crucial for ensuring that the UI elements are not only displaying correctly but are also functionally correct.Given the complexity of handling multiple calendar systems, this test is essential for verifying that the DatePicker behaves as expected across different locales. It might be beneficial to add similar tests for other supported calendar systems mentioned in the PR to ensure comprehensive coverage.
[APROVED]
| case "roc": | ||
| case "japanese": | ||
| case "gregory": | ||
| default: | ||
| return 0; |
There was a problem hiding this comment.
Remove Redundant Case Clauses
The cases for 'roc', 'japanese', and 'gregory' are redundant since they fall into the default case which returns 0. Removing these will simplify the switch statement without affecting functionality.
- case "roc":
- case "japanese":
- case "gregory":Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| case "roc": | |
| case "japanese": | |
| case "gregory": | |
| default: | |
| return 0; | |
| default: | |
| return 0; |
Tools
Biome
[error] 20-20: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
[error] 21-21: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
[error] 22-22: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
📝 Description
Currently we are using gregorian calendar with possible years in$[1900, 2099]$ . However, some locales such as
th-TH-u-ca-buddhistusing different calendar making the years out of bound. Hence, add the corresponding offset to make sure the year is within the bound.⛳️ Current behavior (updates)
locale:
th-TH-u-ca-buddhist🚀 New behavior
locale:
th-TH-u-ca-buddhist💣 Is this a breaking change (Yes/No):
📝 Additional Information
Summary by CodeRabbit
New Features
CalendarWithLocaleandDatePickerWithLocalecomponents for enhanced localization.Enhancements
createCalendar,DateFormatter, andgetGregorianYearOffsetfor better calendar management.minValueandmaxValuecalculations based on calendar identifiers.Refactor
NextUIProviderwith new initialization for default dates.Chores