-
-
Notifications
You must be signed in to change notification settings - Fork 437
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
Fix/datepicker decade picker unclickable #1452
Fix/datepicker decade picker unclickable #1452
Conversation
- Range of years presented in title now match the calendar for the year view - Removed unnecessary subtraction and multiplication in year view
- Found the date range presented was offset incorrectly. e.g. the range presented would be 1990-2100 but range presented was 2000-2090
- Found the use of `isDateInRange` was using the wrong dates for the date, minDate and maxDate. - Updated `isDisabled` check to ensure both the first and last date are in range. - Minor optimisation as `first` doesn't need to be regenerated for each decade.
- Testing that decade titles are being rendered correctly (and technically days, years and months)
- Tested if the button for the earlier decade is enabled when max date is set Tested if the button for the later decade is disabled when max date is set
- Tested if the button for the earlier decade is disabled when min date is set - Tested if the button for the later decade is enabled when min date is set
- When minDate and maxDate were defined, defaultDate was not updated to a Date.
- Tested if decades within a maxDate minDate range are correctly selectable.
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe updates to the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Datepicker
participant DecadesView
participant YearsView
User->>Datepicker: Select a decade
Datepicker->>DecadesView: Update displayed decade range
DecadesView->>Datepicker: Check minDate and maxDate
alt Within range
DecadesView->>User: Show available decades
else Out of range
DecadesView->>User: Disable unavailable decades
end
User->>Datepicker: Select a year
Datepicker->>YearsView: Update displayed year range
YearsView->>Datepicker: Validate selected year
YearsView->>User: Show selected year
Poem
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 Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- packages/ui/src/components/Datepicker/Datepicker.spec.tsx (1 hunks)
- packages/ui/src/components/Datepicker/Datepicker.stories.tsx (1 hunks)
- packages/ui/src/components/Datepicker/Datepicker.tsx (1 hunks)
- packages/ui/src/components/Datepicker/Views/Decades.tsx (2 hunks)
- packages/ui/src/components/Datepicker/Views/Years.tsx (1 hunks)
Additional comments not posted (16)
packages/ui/src/components/Datepicker/Datepicker.stories.tsx (1)
43-44
: LGTM! Ensure proper date handling.The conversion of
args.defaultDate
into aDate
object before passing it togetFirstDateInRange
ensures proper date handling.packages/ui/src/components/Datepicker/Views/Years.tsx (1)
33-33
: LGTM! Correct year calculation.The updated logic correctly calculates the
year
variable by directly assigning the value offirst
plusindex
.packages/ui/src/components/Datepicker/Views/Decades.tsx (5)
23-23
: LGTM! Enhanced date handling.The inclusion of
minDate
andmaxDate
in theuseDatePickerContext
hook enhances the date handling logic.
26-26
: LGTM! Streamlined year calculation.The calculation of the
first
year in the decade view has been streamlined for better readability and maintainability.
31-32
: LGTM! Simplified date manipulation.The introduction of the
newDate
variable simplifies date manipulation and enhances code clarity.
36-37
: LGTM! Improved date range validation.The updated
isDisabled
condition ensures that users cannot select dates outside the specified range.
51-51
: LGTM! Correct view update logic.The logic for updating the view when a decade button is clicked has been correctly implemented.
packages/ui/src/components/Datepicker/Datepicker.spec.tsx (8)
81-95
: LGTM! The test case is well-structured.The test case effectively verifies that the year range is correctly displayed when selecting a decade.
97-112
: LGTM! The test case is well-structured.The test case effectively verifies that earlier decades can be selected when the
maxDate
allows it.
114-129
: LGTM! The test case is well-structured.The test case effectively verifies that later decades are disabled if they exceed the
maxDate
.
131-146
: LGTM! The test case is well-structured.The test case effectively verifies that earlier decades are disabled if they fall below the
minDate
.
148-163
: LGTM! The test case is well-structured.The test case effectively verifies that later decades can be selected when within the
minDate
limit.
165-187
: LGTM! The test case is well-structured.The test case effectively verifies that only decades within the defined range of
minDate
andmaxDate
are selectable.
188-191
: LGTM! The test case is well-structured.The test case effectively verifies that the input is focused when
ref.current.focus
is called.
Line range hint
192-198
:
LGTM! The test case is well-structured.The test case effectively verifies that the value is cleared when
ref.current.clear
is called.packages/ui/src/components/Datepicker/Datepicker.tsx (1)
200-202
: LGTM! The changes improve date range representation.The modifications to the
getViewTitle
function enhance the date range representation for both "Decades" and "Years" views.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @Maraket.
Summary:
Fixed several bugs in the DatePicker component, including:
Added unit tests to cover several basic use cases:
Summary by CodeRabbit
New Features
minDate
andmaxDate
to prevent invalid date selections.Bug Fixes
Datepicker
component's story.Years
view.Tests
minDate
andmaxDate
.