feat(datepickers): add optional hasClearButton prop to datepickers#1088
feat(datepickers): add optional hasClearButton prop to datepickers#1088hoorayimhelping wants to merge 1 commit into
Conversation
🦋 Changeset detectedLatest commit: 383bd87 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 40fefbc. Configure here.
hoorayimhelping
force-pushed
the
bucky-datepicker-close
branch
from
40fefbc to
383bd87
Compare
Storybook Preview Deployed✅ Preview URL: https://click-ff7ye9oy2-clickhouse.vercel.app Built from commit: |
|
@crisalbu are you cool with the input changing size so much? @hoorayimhelping I assume a consumer of this input can avoid the changing width by just setting a reasonable min-width, yeah? |
|
If we have a date time range picker, I feel that we should have a min width for it by default that takes into consideration how much space we need to display those values. |
| }, | ||
| }, | ||
| component: DatePicker, | ||
| render: (args: Args) => { |
There was a problem hiding this comment.
side note: args should be spread onto the component in default story, right now you can't use the knobs to play around with it
| resetPartialState(); | ||
| setSelectedDate(undefined); | ||
| }, [resetPartialState]); | ||
|
|
There was a problem hiding this comment.
this clears internal state but does nothing for consumer
we need to call onSelectDate with undefined
same for other datepickers
There was a problem hiding this comment.
just something I noticed
we don't specify year in datetime range picker, so I can't tell when selected year is differnet from current
if we start showing it (which we should) this adds to the @crisalbu's concern about jumpy width, since the max possible width would be even bigger
There was a problem hiding this comment.
Good thing you mentioned that @ariser! Let's account for the year as well when we pick the width.
By default, we don't show the year if it's the current year, but we do display it when it's past years.
There was a problem hiding this comment.
Well we don't, in current implementation :) we should fix that, but perhaps in a separate PR
There was a problem hiding this comment.
That would be great if we could :)
There was a problem hiding this comment.
I think it should be a separate PR. I just implemented this behavior in a date formatter in CP.
There was a problem hiding this comment.
100%, not this PR's concern
| name="cross" | ||
| size="sm" | ||
| /> | ||
| </ClearButton> |
There was a problem hiding this comment.
why isn't this an IconButton?
There was a problem hiding this comment.
@ariser the hover styling on IconButton overflowed the input and couldn't be changed
@crisalbu @DreaminDani here's the issue with that: If we dynamically decide to show the year in a range picker that has two dates, we have to size it to the maximum possible size, because we won't know what year the selected date will be. The problem is, it's already huge without years:
|
How bout we set the min-with to be optimized for dates without years, and automatically resize when it has years? You know what would help with the width? using 24h times, instead of a.m. / p.m. 😉
Out of scope for this PR, but maybe worth rediscussing during our next UX-dev call? |
I like this idea :) |
4 participants
Screen.Recording.2026-06-18.at.4.24.23.PM.mov
Closes https://linear.app/clickhouse/issue/CUI-178/datepickers-add-a-prop-controlled-clear-button
Note
Low Risk
Opt-in UI with local state only; main integration caveat is that clear does not invoke
onSelectDate/onSelectDateRange, so parents must handle empty state themselves if needed.Overview
Adds an opt-in
hasClearButtonprop on DatePicker, DateRangePicker, and DateTimeRangePicker that shows a cross control on the right of the trigger input. Clearing resets internal selection (and partial year/month on the single date picker) without opening the popover/dropdown; range pickers also stoppointerdownon the clear control so the dropdown trigger does not toggle.Shared input UI in
Common.tsxintroduces a styledClearButton(spanwithrole="button", notIconButton) to avoid invalid button-in-button nesting inside popover/dropdown triggers. Layout tweaks include optional$minWidthon the highlighted wrapper andmax-contentwidth when the clear control is shown on range inputs.Stories, tests, and a minor changeset are included. Note: the changeset text refers to
hasCloseButton, while the public API ishasClearButton.Reviewed by Cursor Bugbot for commit 383bd87. Bugbot is set up for automated code reviews on this repo. Configure here.