Feat/date value datepicker #1190

ddiasfront · 2023-12-14T19:16:48Z

I have followed the Your First Code Contribution section of the Contributing guide

It was added a new property called dateValue, which directly modifies the date inside of the datepicker whenever it's necessary, also the view is set to this selected date, automatically, independently from the type of view.

It closes #1187 #1055 #1039 #1167

The prop added is merely optional and will not cause any breaking change, test coverage is basically set and documentation is covered as well.

The PR introduces a breaking change for the Datepicker component.

Now the 'onSelectedDateChange' property changed name to 'onChange' as it is a more approachable/obvious pattern to follow.

We don't have any other occurrences for 'onSelectedDateChange' on the codebase ATM, so it is SAFE bro, typescript will do the rest of the work :)

Summary by CodeRabbit

New Features
- Added value, defaultValue, and label controls for enhanced Datepicker customization.
- Introduced a new section in documentation for creating a controlled Datepicker using the value prop.
- Added new story variants for different date scenarios, including controlled templates.
Improvements
- Enhanced Datepicker to manage dates dynamically with value and defaultValue props.
- Optimized internal state management using React hooks for better performance.
- Simplified logic for clearing selected dates and ensuring robust date comparisons.
Documentation
- Expanded documentation to include handling null values and practical examples for controlled usage.
Tests
- Introduced comprehensive unit tests for various Datepicker functionalities, ensuring accurate behavior and state management.

vercel · 2023-12-14T19:16:53Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
flowbite-react	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Sep 30, 2024 0:56am
flowbite-react-storybook	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Sep 30, 2024 0:56am

codecov · 2023-12-14T20:49:42Z

Codecov Report

Attention: Patch coverage is 80.55556% with 7 lines in your changes are missing coverage. Please review.

Project coverage is 97.36%. Comparing base (7461173) to head (baed714).
Report is 245 commits behind head on main.

❗ Current head baed714 differs from pull request most recent head 2070c8b

Please upload reports for the commit 2070c8b to get more accurate results.

Files	Patch %	Lines
src/components/Datepicker/Datepicker.tsx	90.00%	3 Missing ⚠️
src/components/Datepicker/Views/Decades.tsx	0.00%	2 Missing ⚠️
src/components/Datepicker/Views/Months.tsx	0.00%	1 Missing ⚠️
src/components/Datepicker/Views/Years.tsx	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1190      +/-   ##
==========================================
- Coverage   99.54%   97.36%   -2.19%     
==========================================
  Files         163      214      +51     
  Lines        6621     9091    +2470     
  Branches      401      542     +141     
==========================================
+ Hits         6591     8851    +2260     
- Misses         30      240     +210

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

ddiasfront · 2023-12-16T17:10:52Z

Hey @rluders , I guess now it is ready for review, could please czech it out ?

examples/datepicker/datepicker.dateValue.tsx

SutuSebastian · 2023-12-18T12:32:55Z

@ddiasfront I see some formatting issues

ddiasfront · 2023-12-18T12:46:31Z

@SutuSebastian Can you describe it, please? I didn't see any formatting issues regarding the date itself, is it on the date or another stuff?

SutuSebastian · 2023-12-18T15:22:43Z

@SutuSebastian Can you describe it, please? I didn't see any formatting issues regarding the date itself, is it on the date or another stuff?

Apparently u fixed it with the last commits :D

The build failed, and inside the build fail output there was the formatting issue AFAIR.

src/components/Datepicker/Datepicker.tsx

ddiasfront · 2023-12-20T12:15:52Z

@rluders , @SutuSebastian Okay guys, now I've fixed the issues bellow as well, including the clear functionality:

#1187 #1055 #1039 #1167

In the clear functionality scenario, I had to add a label property to be rendered whenever there's no date selected, and i set it to default "No date selected", please check it out.

coderabbitai

Actionable comments posted: 1

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 02d1f8f and 4548a0c.

Files selected for processing (9)

apps/web/content/docs/components/datepicker.mdx (1 hunks)
packages/ui/src/components/Datepicker/Datepicker.spec.tsx (7 hunks)
packages/ui/src/components/Datepicker/Datepicker.stories.tsx (4 hunks)
packages/ui/src/components/Datepicker/Datepicker.tsx (7 hunks)
packages/ui/src/components/Datepicker/DatepickerContext.tsx (1 hunks)
packages/ui/src/components/Datepicker/Views/Days.tsx (1 hunks)
packages/ui/src/components/Datepicker/Views/Decades.tsx (2 hunks)
packages/ui/src/components/Datepicker/Views/Months.tsx (2 hunks)
packages/ui/src/components/Datepicker/Views/Years.tsx (1 hunks)

Additional comments not posted (33)

packages/ui/src/components/Datepicker/DatepickerContext.tsx (1)

17-18: The past review comment on lines 17-17 is still valid and applicable to the current changes. It has already addressed the need to update setSelectedDate to handle null values for consistency with the selectedDate property.

packages/ui/src/components/Datepicker/Views/Years.tsx (1)

37-37: LGTM!

The conditional check for the truthiness of selectedDate before calling isDateEqual is a good defensive programming practice. It enhances the robustness of the date selection logic by preventing unnecessary function calls and potential runtime errors when selectedDate is not defined.

packages/ui/src/components/Datepicker/Views/Months.tsx (2)

5-5: LGTM!

The import statement has been correctly updated to include the isDateEqual function from the ../helpers module. This change is necessary to support the updated logic in the isSelected constant.

45-45: Approve the updated selection logic.

The changes to the isSelected constant are an improvement over the previous implementation:

Using isDateEqual instead of isMonthEqual ensures that the entire date object is compared for equality, not just the month. This provides a more accurate determination of whether a month is selected.

Checking if selectedDate is defined before performing the equality check enhances the robustness of the selection logic by avoiding potential errors when selectedDate is undefined.

These changes positively impact the user experience by ensuring that the selected month is accurately highlighted in the date picker.

packages/ui/src/components/Datepicker/Views/Decades.tsx (2)

36-36: LGTM!

The conditional check for selectedDate before calling isDateInDecade is a good improvement. It prevents potential errors when selectedDate is null or undefined.

52-52: LGTM!

The change to adjust the viewDate based on the difference between the year and the year of the selectedDate is a good improvement. It allows for a more dynamic adjustment of the view date, reflecting the selected decade more accurately. The conditional check for selectedDate also prevents potential errors.

packages/ui/src/components/Datepicker/Views/Days.tsx (1)

58-58: LGTM!

The updated logic for determining the selected date is more robust and handles the case when selectedDate is undefined. This change aligns with the PR objective of supporting controlled inputs and prevents potential errors.

apps/web/content/docs/components/datepicker.mdx (1)

72-76: LGTM!

The new section provides a clear explanation of how to create a controlled datepicker using the value prop. The example enhances the documentation by demonstrating the functionality in practice.

The changes align with the PR objective of introducing the dateValue prop for controlled usage and are consistent with the AI summary.

packages/ui/src/components/Datepicker/Datepicker.stories.tsx (5)

35-63: LGTM!

The ControlledTemplate story is a great addition to demonstrate the controlled behavior of the Datepicker component. The usage of React's useState and useEffect hooks to manage the selected date based on the value prop is implemented correctly. The story also handles the conversion of minDate and maxDate props to Date objects and modifies the defaultValue based on the provided date range, ensuring that the default date falls within the specified limits.

74-79: LGTM!

The Template story is correctly updated to align with the new logic. The handling of defaultDate is changed to defaultValue and it adheres to the same date range constraints.

84-96: LGTM!

The ControlledDefaultEmpty story is a great addition to demonstrate the controlled behavior of the Datepicker component with no selected date. Setting the value prop to null and the label prop to "No date selected" is a good way to showcase this scenario.

111-121: LGTM!

The NullDateValue story is a good addition to demonstrate the behavior of the Datepicker component with a null date value.

123-134: LGTM!

The DateValueSet story is a good addition to demonstrate the behavior of the Datepicker component with a default date value. Setting the defaultValue prop to the current date using new Date() is a nice way to showcase this scenario.

packages/ui/src/components/Datepicker/Datepicker.spec.tsx (8)

60-70: LGTM!

The test case has been correctly updated to use the new onChange prop, which replaces the previous onSelectedDateChanged prop. The test case logic remains the same and is correct.

83-83: LGTM!

The test case has been correctly updated to use the new defaultValue prop, which replaces the previous defaultDate prop. The test case logic remains the same and is correct.

99-99: LGTM!

The test case has been correctly updated to use the new defaultValue prop, which replaces the previous defaultDate prop. The test case logic remains the same and is correct.

116-116: LGTM!

The test case has been correctly updated to use the new defaultValue prop, which replaces the previous defaultDate prop. The test case logic remains the same and is correct.

133-133: LGTM!

The test case has been correctly updated to use the new defaultValue prop, which replaces the previous defaultDate prop. The test case logic remains the same and is correct.

150-150: LGTM!

The test case has been correctly updated to use the new defaultValue prop, which replaces the previous defaultDate prop. The test case logic remains the same and is correct.

170-170: LGTM!

The test case has been correctly updated to use the new defaultValue prop, which replaces the previous defaultDate prop. The test case logic remains the same and is correct.

Line range hint 1-220: LGTM!

The test cases in the Datepicker.spec.tsx file have been correctly updated to use the new prop names:

onSelectedDateChanged has been renamed to onChange.

defaultDate has been renamed to defaultValue.

The test case logic remains the same and is correct. The changes are consistent with the list of alterations provided.

packages/ui/src/components/Datepicker/Datepicker.tsx (12)

4-4: LGTM!

The additional imports are necessary for the changes made in the file.

80-80: LGTM!

The comment update accurately reflects the changes made to the clear method implementation.

85-101: LGTM!

The changes to the DatepickerProps interface are in line with the PR objectives and the AI-generated summary. The new props enhance the component's flexibility and usability by allowing for better integration with external state management.

114-123: LGTM!

The destructuring of the new props is necessary for using them in the component implementation.

138-140: LGTM!

The initialization logic for selectedDate and viewDate state variables is correct and handles the controlled and uncontrolled scenarios properly.

Line range hint 146-162: LGTM!

The changes to the changeSelectedDate and clearDate functions are in line with the PR objectives and the AI-generated summary. The modifications ensure that the selected date is updated correctly and the onChange callback is invoked with the updated date.

250-259: LGTM!

The new useEffect hook is necessary to keep the selectedDate state in sync with the controlled value prop. The logic correctly handles the controlled scenario and ensures that the selected date is within the allowed range.

261-262: LGTM!

The introduction of the displayValue variable is a good approach to handle the display logic for the input field. It ensures that the input reflects the current state accurately and falls back to the label prop when no date is selected.

289-292: LGTM!

Updating the viewDate state to selectedDate when the input field is focused ensures that the calendar view is synchronized with the selected date, providing a better user experience.

294-294: LGTM!

Setting the value prop of the TextInput component to displayValue ensures that the input field displays the formatted selected date or the label when no date is selected.

296-296: LGTM!

Setting the defaultValue prop of the TextInput component to the formatted initialDate or the label prop ensures that the input field displays the correct initial value when the component is uncontrolled.

359-359: LGTM!

Passing null as the date value to the changeSelectedDate function in the clear button's onClick handler ensures that the selected date is properly cleared when the clear button is clicked.

packages/ui/src/components/Datepicker/Datepicker.tsx

ddiasfront · 2024-09-13T11:08:44Z

Here.

#1484 (comment)

Alright, done guys, changeset added, shoutout anything else might need.

@rluders @SutuSebastian

SutuSebastian · 2024-09-25T08:02:07Z

Here.
#1484 (comment)

Alright, done guys, changeset added, shoutout anything else might need.

@rluders @SutuSebastian

all good 🔥

SutuSebastian · 2024-09-25T08:03:40Z

.changeset/six-tips-rush.md

we have to re-do the changeset, it is not a major, u can leave it as patch

@SutuSebastian, @rluders updated changeset

nice, can u also put some more info in regards of the breaking changes and what users have to modify when they update to this next version of the datepicker?

U can do this in the same changeset markdown file

coderabbitai

Actionable comments posted: 0

🧹 Outside diff range and nitpick comments (1)

.changeset/old-jobs-decide.md (1)

5-13: Consider adding information about clear functionality and label property.

The Datepicker Component Updates section provides a good overview of the improvements. However, it would be beneficial to explicitly mention:

The addition of clear functionality (related to Issue DatePicker component defaultDate/clear not working as expected #1039).

The new label property that displays "No date selected" when no date is chosen.

These additions would provide a more comprehensive summary of the changes made.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between dfef19d and f082ca7.

📒 Files selected for processing (1)

.changeset/old-jobs-decide.md (1 hunks)

🔇 Additional comments (2)

.changeset/old-jobs-decide.md (2)
15-22: Verify completeness of updated test files.

The Files Updated section provides a good overview of the main files changed. However, to ensure comprehensive test coverage for all the new features and changes:

Verify if any additional test files were updated or created, especially for the new controlled input functionality and clear feature.

Consider mentioning any updates to integration or end-to-end tests, if applicable.

This will help ensure that all aspects of the changes are properly tested and documented.
#!/bin/bash
# Description: Check for any new or modified test files related to Datepicker

# Test: Search for new or modified test files. Expect: List of test files if any were changed or added.
git diff --name-only origin/main | grep -E 'test|spec'
1-3: ⚠️ Potential issue

Verify the changeset type for breaking changes.

The changeset specifies a patch update, which is typically used for non-breaking changes and bug fixes. However, the PR description mentions a breaking change (renaming of onSelectedDateChange to onChange). Consider updating the changeset to a minor or major version bump if this breaking change is intended to be included.

ddiasfront · 2024-09-30T13:41:54Z

Thanks for doing this, I mostly just have recommendations on docs changes. It would be helpful if @rluders could review this because he understands this component best, and it's by far our most complex

@tulup-conner can we close this thread and head towards merging ?

rluders · 2024-09-30T13:58:32Z

Thanks for doing this, I mostly just have recommendations on docs changes. It would be helpful if @rluders could review this because he understands this component best, and it's by far our most complex

@tulup-conner can we close this thread and head towards merging ?

Yes, please. Let's get it merged. It is already too much to handle in here. I didn't notice anything that could cause problems.

SutuSebastian

awesome

corneliusroemer · 2024-10-23T13:03:14Z

Heads up that this ended up being a breaking change for typescript users due to changed types.

Might have been worth a minor (=major due to 0.) bump. I'll add details later.

Errors from our CI:

src/components/DataUseTerms/EditDataUseTermsButton.tsx:94:61 - error ts(7006): Parameter 'date' implicitly has an 'any' type.

94                                     onSelectedDateChanged={(date) => {
                                                               ~~~~
src/components/DataUseTerms/EditDataUseTermsButton.tsx:88:37 - error ts(2322): Type '{ className: string; defaultDate: Date; showClearButton: false; showTodayButton: false; minDate: Date; maxDate: Date; theme: FlowbiteDatepickerTheme; onSelectedDateChanged: (date: any) => void; inline: true; }' is not assignable to type 'IntrinsicAttributes & DatepickerProps & RefAttributes<DatepickerRef>'.
  Property 'defaultDate' does not exist on type 'IntrinsicAttributes & DatepickerProps & RefAttributes<DatepickerRef>'.

88                                     defaultDate={restrictedUntil.toJSDate()}
                                       ~~~~~~~~~~~

src/components/Submission/DateChangeModal.tsx:116:45 - error ts(7006): Parameter 'date' implicitly has an 'any' type.

116                     onSelectedDateChanged={(date) => {
                                                ~~~~
src/components/Submission/DateChangeModal.tsx:110:[21](https://github.com/loculus-project/loculus/actions/runs/11479666879/job/31946438119?pr=3050#step:7:22) - error ts(2322): Type '{ defaultDate: Date; showClearButton: false; showTodayButton: false; minDate: Date; maxDate: Date; theme: FlowbiteDatepickerTheme; onSelectedDateChanged: (date: any) => void; inline: true; }' is not assignable to type 'IntrinsicAttributes & DatepickerProps & RefAttributes<DatepickerRef>'.
  Property 'defaultDate' does not exist on type 'IntrinsicAttributes & DatepickerProps & RefAttributes<DatepickerRef>'.

110                     defaultDate={date}
                        ~~~~~~~~~~~

https://github.com/loculus-project/loculus/actions/runs/11479666879/job/31946438119?pr=3050#step:7:21

loculus-project/loculus#3054

vercel bot temporarily deployed to Preview December 14, 2023 20:48 Inactive

ddiasfront mentioned this pull request Dec 14, 2023

Datepicker only changes on manual click. #1187

Closed

2 tasks

vercel bot temporarily deployed to Preview December 14, 2023 20:54 Inactive

ddiasfront marked this pull request as draft December 15, 2023 00:35

vercel bot temporarily deployed to Preview December 15, 2023 01:41 Inactive

ddiasfront marked this pull request as ready for review December 15, 2023 01:45

vercel bot temporarily deployed to Preview December 15, 2023 01:46 Inactive

ddiasfront changed the title ~~Feat/date value datepicker~~ Feat/date value datepicker - CLOSES #1187 Dec 15, 2023

vercel bot temporarily deployed to Preview December 16, 2023 17:09 Inactive

ddiasfront changed the title ~~Feat/date value datepicker - CLOSES #1187~~ Feat/date value datepicker Dec 17, 2023

SutuSebastian reviewed Dec 18, 2023

View reviewed changes

examples/datepicker/datepicker.dateValue.tsx Outdated Show resolved Hide resolved

vercel bot temporarily deployed to Preview December 18, 2023 12:09 Inactive

vercel bot temporarily deployed to Preview December 18, 2023 12:11 Inactive

vercel bot temporarily deployed to Preview December 18, 2023 12:44 Inactive

ddiasfront requested a review from SutuSebastian December 18, 2023 14:42

SutuSebastian reviewed Dec 18, 2023

View reviewed changes

src/components/Datepicker/Datepicker.tsx Outdated Show resolved Hide resolved

vercel bot temporarily deployed to Preview December 19, 2023 20:16 Inactive

vercel bot temporarily deployed to Preview December 20, 2023 11:53 Inactive

vercel bot temporarily deployed to Preview December 20, 2023 12:05 Inactive

vercel bot temporarily deployed to Preview December 20, 2023 12:12 Inactive

ddiasfront requested review from rluders, tulup-conner and SutuSebastian December 20, 2023 12:32

feat: defaultDate empty and side-effects addressed

4548a0c

ddiasfront force-pushed the feat/dateValue-datepicker branch from 02d1f8f to 4548a0c Compare September 13, 2024 10:50

vercel bot deployed to Preview – flowbite-react September 13, 2024 10:51 View deployment

vercel bot deployed to Preview – flowbite-react-storybook September 13, 2024 10:52 View deployment

coderabbitai bot reviewed Sep 13, 2024

View reviewed changes

packages/ui/src/components/Datepicker/Datepicker.tsx Show resolved Hide resolved

feat: added changeset

dfef19d

vercel bot deployed to Preview – flowbite-react-storybook September 13, 2024 11:05 View deployment

vercel bot deployed to Preview – flowbite-react September 13, 2024 11:06 View deployment

rluders previously approved these changes Sep 13, 2024

View reviewed changes

SutuSebastian approved these changes Sep 25, 2024

View reviewed changes

SutuSebastian requested changes Sep 25, 2024

View reviewed changes

SutuSebastian mentioned this pull request Sep 25, 2024

fix(datepicker): datePicker now supports formatting of date #1314

Open

feat - changset to patch

f082ca7

ddiasfront dismissed rluders’s stale review via f082ca7 September 30, 2024 12:54

ddiasfront requested review from rluders and SutuSebastian September 30, 2024 12:54

vercel bot deployed to Preview – flowbite-react-storybook September 30, 2024 12:55 View deployment

vercel bot deployed to Preview – flowbite-react September 30, 2024 12:56 View deployment

coderabbitai bot reviewed Sep 30, 2024

View reviewed changes

SutuSebastian approved these changes Sep 30, 2024

View reviewed changes

SutuSebastian merged commit 25bb353 into themesberg:main Sep 30, 2024
8 checks passed

github-actions bot mentioned this pull request Sep 30, 2024

Version Packages #1486

Merged

gogicK mentioned this pull request Oct 25, 2024

[Snyk] Upgrade flowbite-react from 0.10.1 to 0.10.2 VidalM0103/R2S-Composites#28

Open

ajile-in mentioned this pull request Nov 4, 2024

[Snyk] Upgrade flowbite-react from 0.4.11 to 0.10.2 credebl/studio#813

Open

bhat-abhishek mentioned this pull request Nov 4, 2024

[Snyk] Upgrade flowbite-react from 0.3.5 to 0.10.2 bhat-abhishek/travel_website#16

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feat/date value datepicker #1190

Feat/date value datepicker #1190

ddiasfront commented Dec 14, 2023 •

edited by coderabbitai bot

Loading

vercel bot commented Dec 14, 2023 •

edited

Loading

codecov bot commented Dec 14, 2023 •

edited

Loading

ddiasfront commented Dec 16, 2023

SutuSebastian commented Dec 18, 2023

ddiasfront commented Dec 18, 2023 •

edited

Loading

SutuSebastian commented Dec 18, 2023 •

edited

Loading

ddiasfront commented Dec 20, 2023 •

edited

Loading

coderabbitai bot left a comment

ddiasfront commented Sep 13, 2024 •

edited

Loading

SutuSebastian commented Sep 25, 2024

SutuSebastian Sep 25, 2024

ddiasfront Sep 30, 2024

SutuSebastian Sep 30, 2024

coderabbitai bot left a comment

ddiasfront commented Sep 30, 2024

rluders commented Sep 30, 2024

SutuSebastian left a comment

corneliusroemer commented Oct 23, 2024 •

edited

Loading

Feat/date value datepicker #1190

Feat/date value datepicker #1190

Conversation

ddiasfront commented Dec 14, 2023 • edited by coderabbitai bot Loading

The PR introduces a breaking change for the Datepicker component.

Summary by CodeRabbit

Summary by CodeRabbit

vercel bot commented Dec 14, 2023 • edited Loading

codecov bot commented Dec 14, 2023 • edited Loading

Codecov Report

ddiasfront commented Dec 16, 2023

SutuSebastian commented Dec 18, 2023

ddiasfront commented Dec 18, 2023 • edited Loading

SutuSebastian commented Dec 18, 2023 • edited Loading

ddiasfront commented Dec 20, 2023 • edited Loading

coderabbitai bot left a comment

Choose a reason for hiding this comment

ddiasfront commented Sep 13, 2024 • edited Loading

SutuSebastian commented Sep 25, 2024

SutuSebastian Sep 25, 2024

Choose a reason for hiding this comment

ddiasfront Sep 30, 2024

Choose a reason for hiding this comment

SutuSebastian Sep 30, 2024

Choose a reason for hiding this comment

coderabbitai bot left a comment

Choose a reason for hiding this comment

ddiasfront commented Sep 30, 2024

rluders commented Sep 30, 2024

SutuSebastian left a comment

Choose a reason for hiding this comment

corneliusroemer commented Oct 23, 2024 • edited Loading

ddiasfront commented Dec 14, 2023 •

edited by coderabbitai bot

Loading

vercel bot commented Dec 14, 2023 •

edited

Loading

codecov bot commented Dec 14, 2023 •

edited

Loading

ddiasfront commented Dec 18, 2023 •

edited

Loading

SutuSebastian commented Dec 18, 2023 •

edited

Loading

ddiasfront commented Dec 20, 2023 •

edited

Loading

ddiasfront commented Sep 13, 2024 •

edited

Loading

corneliusroemer commented Oct 23, 2024 •

edited

Loading