Conversation
notvasub
force-pushed
the
vasu/clearable-data-input-comp
branch
from
3618df0 to
f363f3e
Compare
| @@ -0,0 +1,74 @@ | |||
| "use client"; | |||
There was a problem hiding this comment.
To be transparent, nick was a tad bit unclear on the original ticket requirements.
He meant to say "make new component if extending the original date input component is clunky or tricky in some sense". Would it be okay for you to provide some justification as to whether this new component is necessary? If there isn't much justification, go ahead and try to refactor the PR into extensions of the old date input component. I can review the rest in more detail after the refactor if that's the path we take.
There was a problem hiding this comment.
Maybe I am missing something but before this PR, both StudentTableForm and LocationTableForm had the full Popover + Button + Calendar date picker pattern copy-pasted inline. There wasn't really any shared date input component to extend. Thats why I made ClearableDatePicker to extract that logic. The only real alternative would be adding the clear button logic inline to both forms separately, which would make the duplication worse. Just tryna prevent duplicated logic 😅
There was a problem hiding this comment.
Hey Vasu sorry about that. I was sure we had an existing date picker but I see now we only have a date range picker. Thanks for calling that out. Since we're making a reusable component in this ticket, could you go ahead and replace all the places where it is duplicated with an option to make it clearable? That would help a lot to resolve some tech debt
Made-with: Cursor
manyuagashe
left a comment
manyuagashe
left a comment
There was a problem hiding this comment.
FilterInput Line 137 and SplitDateRangeFilter Lines 29 and 46 still use inline date pickers. Please resolve this to your brand new component instead. no moar inline date pickers
|
LGTM otherwise |
naasanov
left a comment
naasanov
left a comment
There was a problem hiding this comment.
I'm going through all outstanding PRs. Waiting on fixes and merge conflicts on this one
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…into vasu/clearable-data-input-comp
3 participants
Add clearable date picker component for nullable date fields
Users need a way to clear nullable date fields (Hold
Expiration and Last Registered) back to null after selecting
a date. Currently, once a date is picked, there's no clear
button.
Changes:
ClearableDatePickercomponent that wraps Popover +Calendar and displays an X button when a date is selected
LocationTableForm.tsxwithClearableDatePickerfor Hold Expiration fieldStudentTableForm.tsxwithClearableDatePickerfor Last Registered fieldCloses #106