-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
fixes 6106 month/year datepicker-ui not working #6199
fixes 6106 month/year datepicker-ui not working #6199
Conversation
Is this correct following twentyhq#6141?
closes twentyhq#6041 - enabled removing all dropdown items including the primary one - primary one can be removed even when it is not the last remaining one from the list, this will set the next item on the list as the new primary one (_idk if it was expected to implement this_) https://github.com/twentyhq/twenty/assets/19856731/405a055d-13de-43f4-b3e8-d6a199bfdf24 --------- Co-authored-by: Félix Malfait <[email protected]>
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.
PR Summary
packages/twenty-front/src/modules/ui/input/components/internal/date/components/InternalDatePicker.tsx
: AddeduseState
hook to manage selected date state.packages/twenty-front/src/modules/object-record/object-filter-dropdown/components/ObjectFilterDropdownDateInput.tsx
: Removed unnecessary blank line inhandleChange
function.
2 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings
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.
PR Summary
(updates since last review)
packages/twenty-front/src/modules/object-record/object-filter-dropdown/components/ObjectFilterDropdownDateInput.tsx
: IntroducedinternalDate
state variable for managing selected date.packages/twenty-front/src/modules/ui/input/components/internal/date/components/InternalDatePicker.tsx
: Modified date handling and dropdown closures; changeddate
prop type toDate | null
.packages/twenty-front/src/modules/ui/input/components/internal/date/components/__stories__/InternalDatePicker.stories.tsx
: Renamed file and added new story 'WithOpenYearSelect'; updated 'WithOpenMonthSelect' story.
3 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings
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.
@Faisal-imtiyaz123, thanks for the PR.
I've made a few modifications:
- we should not introduce an internal state to represent the date within InternalDatePicker. InternalDatePicker is a controlled input meaning that it's value is controlled by the outside. If you introduce an internal state, it won't be updated when the outside changes the value. The state should actually be introduced in ObjectFilterDropdownDateInput component
- Added stories to make sure the issue is fixed
Log
|
@charlesBochet Thankyou for the feedback and sorry you had to do a lot of modifications to the PR . |
fixes #6106