Open bvedad opened 2 months ago
changeset-bot[bot]
commented
2 months ago Latest commit: 62b2525c2b7af41f35c97e00a7d5b8f31ecfcf71
The changes in this PR will be included in the next version bump.
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
vercel[bot]
commented
2 months ago 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 | Jul 4, 2024 3:29pm |
| flowbite-react-storybook | ✅ Ready (Inspect) | Visit Preview | 💬 Add feedback | Jul 4, 2024 3:29pm |
coderabbitai[bot]
commented
2 months ago The update involves enhancing the Datepicker component by introducing a new parameter isClearAction to the onSelectedDateChanged callback. This addition enables explicit handling of date clearing actions, enhancing functionality and user experience in scenarios where users need to reset their date inputs.
| File Path | Change Summary |
|---|---|
packages/ui/src/components/Datepicker/Datepicker.tsx |
Added isClearAction parameter to onSelectedDateChanged in DatepickerProps; updated relevant functions to support this change. |
isClearAction parameter added in this PR. This change allows developers to explicitly set the date to null when clearing.🐰✨ A hop, a skip, a jump so high, A Datepicker change, oh my! Clear your dates, without dismay, For "isClearAction" leads the way! Celebrate, for bugs we fray! 🎉 🐰✨
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?
SutuSebastian
commented
2 months ago onChange prop should be returning Date | null in the first place, avoiding this workaround
rluders
commented
2 months ago
onChangeprop should be returningDate | nullin the first place, avoiding this workaround
I'm not sure... 🤔 Let me think.
Update: Yeah, I think it makes sense.
SutuSebastian
commented
2 months ago Also, this has packages/ui changes, which should introduce a changeset, please run bun changeset in the root folder and follow instructions to bump the flowbite-react package to a patch, in the end u have to give it a changelog/summary of the PR, then commit the generated changeset.
bvedad
commented
2 months ago
onChangeprop should be returningDate | nullin the first place, avoiding this workaround
@SutuSebastian I agree with you but the reason why I proposed this implementation is because at the moment clear button does not clear the date to be null, but it resets it to the default value. To me this is counterintuitive but if developers decide for some reason to implement it that way I am not planning to be a bottleneck. This approach is harmless and it allows us to set dates to null if we want. If you want to I can implement it so onSelectedDateChanged accept Date | null but then I will need to change implementation of clear from:
const clearDate = () => {
changeSelectedDate(defaultDate, true, true);
if (defaultDate) {
setViewDate(defaultDate);
}
};to something like:
const clearDate = () => {
changeSelectedDate(defaultDate, true, true);
setViewDate(defaultDate);
};and as well change code from:
{showClearButton && (
<button
type="button"
className={twMerge(theme.popup.footer.button.base, theme.popup.footer.button.clear)}
onClick={() => {
changeSelectedDate(defaultDate, true, true);
if (defaultDate) {
setViewDate(defaultDate);
}
}}
>
{labelClearButton}
</button>
)}to something like below:
{showClearButton && (
<button
type="button"
className={twMerge(theme.popup.footer.button.base, theme.popup.footer.button.clear)}
onClick={() => {
changeSelectedDate(null, true, true);
setViewDate(null);
}}
>
{labelClearButton}
</button>
)}Also, this has
packages/uichanges, which should introduce a changeset, please runbun changesetin the root folder and follow instructions to bump theflowbite-reactpackage to apatch, in the end u have to give it a changelog/summary of the PR, then commit the generated changeset.
Thanks for giving me this information. I have just generated changelog content and pushed it to this MR.
bvedad
commented
2 months ago Check comments ☝️
I have just checked them and I have performed the necessary changes.
Update Datepicker onSelectedDateChanged signature to include isClearAction parameter
1366
Summary by CodeRabbit