-
-
Notifications
You must be signed in to change notification settings - Fork 476
fix(datepicker): datePicker now supports formatting of date #1314
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
base: main
Are you sure you want to change the base?
Conversation
|
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe recent changes enhance the Datepicker component by adding localization and customizable date formats, leveraging Changes
Poem
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? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
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.
Review Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files ignored due to path filters (1)
packages/ui/package.jsonis excluded by:!**/*.json
Files selected for processing (7)
- apps/web/content/docs/components/datepicker.mdx (1 hunks)
- apps/web/examples/datepicker/datepicker.format.tsx (1 hunks)
- apps/web/examples/datepicker/index.ts (1 hunks)
- packages/ui/src/components/Datepicker/Datepicker.spec.tsx (5 hunks)
- packages/ui/src/components/Datepicker/Datepicker.stories.tsx (1 hunks)
- packages/ui/src/components/Datepicker/Datepicker.tsx (4 hunks)
- packages/ui/src/components/Datepicker/helpers.ts (3 hunks)
Additional comments: 5
apps/web/examples/datepicker/index.ts (1)
- 2-2: The addition of the
formatexport fromdatepicker.formatis correctly implemented.apps/web/examples/datepicker/datepicker.format.tsx (1)
- 1-42: The new example file
datepicker.format.tsxcorrectly demonstrates the usage of theinputFormatprop with theDatepickercomponent. The structure and export of the example are consistent with the project's standards.packages/ui/src/components/Datepicker/Datepicker.stories.tsx (1)
- 64-77: The addition of the
FormattedDatestory inDatepicker.stories.tsxcorrectly showcases the newinputFormatprop with a sample format. This enhances the documentation and provides a practical example of the new feature.apps/web/content/docs/components/datepicker.mdx (1)
- 22-29: The addition of the documentation section for setting the date format using the
inputFormatprop is clear and informative. The note on format conventions is particularly helpful for guiding users in correctly specifying date formats.packages/ui/src/components/Datepicker/Datepicker.tsx (1)
- 100-100: The addition of the
inputFormatproperty to theDatepickerPropsinterface and its usage inDatepickerRenderare correctly implemented. The default value and its application in date formatting calls enhance the component's flexibility and align with the PR objectives.Also applies to: 121-121, 206-206, 209-209, 277-277
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.
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.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files ignored due to path filters (1)
packages/ui/package.jsonis excluded by:!**/*.json
Files selected for processing (5)
- apps/web/examples/datepicker/datepicker.format.tsx (1 hunks)
- packages/ui/src/components/Datepicker/Datepicker.spec.tsx (5 hunks)
- packages/ui/src/components/Datepicker/Datepicker.tsx (3 hunks)
- packages/ui/src/components/Datepicker/helpers.spec.tsx (1 hunks)
- packages/ui/src/components/Datepicker/helpers.ts (3 hunks)
Files skipped from review as they are similar to previous changes (4)
- apps/web/examples/datepicker/datepicker.format.tsx
- packages/ui/src/components/Datepicker/Datepicker.spec.tsx
- packages/ui/src/components/Datepicker/Datepicker.tsx
- packages/ui/src/components/Datepicker/helpers.spec.tsx
Additional comments: 1
packages/ui/src/components/Datepicker/helpers.ts (1)
- 1-1: The import of
DateFNSFormatfrom 'date-fns' is correctly added to support date formatting functionality.
|
btw, what code editor are u using @dhavalveera ? It seems to me that u are not formatting the code on-save, or at all. |
I'm using VSCode, and when I do save ( |
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (9)
- apps/web/content/docs/components/datepicker.mdx (1 hunks)
- apps/web/examples/datepicker/datepicker.format.tsx (1 hunks)
- apps/web/examples/datepicker/datepicker.localization.tsx (1 hunks)
- apps/web/examples/datepicker/index.ts (1 hunks)
- packages/ui/src/components/Datepicker/Datepicker.spec.tsx (5 hunks)
- packages/ui/src/components/Datepicker/Datepicker.stories.tsx (1 hunks)
- packages/ui/src/components/Datepicker/Datepicker.tsx (4 hunks)
- packages/ui/src/components/Datepicker/helpers.spec.tsx (1 hunks)
- packages/ui/src/components/Datepicker/helpers.ts (3 hunks)
Files skipped from review as they are similar to previous changes (5)
- apps/web/examples/datepicker/datepicker.format.tsx
- apps/web/examples/datepicker/index.ts
- packages/ui/src/components/Datepicker/Datepicker.spec.tsx
- packages/ui/src/components/Datepicker/Datepicker.stories.tsx
- packages/ui/src/components/Datepicker/Datepicker.tsx
Additional comments: 11
apps/web/examples/datepicker/datepicker.localization.tsx (3)
- 10-10: The update to the
languageattribute and label attributes for the Datepicker component enhances consistency and ensures compatibility with thedate-fnspackage.- 18-18: The consistency in updating the
languageattribute and label attributes across different parts of the file ensures uniformity in the component's usage.- 23-23: The consistency in updating the
languageattribute and label attributes across different parts of the file ensures uniformity in the component's usage.apps/web/content/docs/components/datepicker.mdx (2)
- 22-30: The addition of a section for setting the date format using the
inputFormatprop, along with notes on date format conventions and the reliance ondate-fns, is crucial for informing users about the new formatting capabilities of the Datepicker component.- 38-38: The note on the reliance of the
Datepickercomponent ondate-fnsfor localization, and guiding users to refer to available localizations fromdate-fns, is important for correctly utilizing the localization feature.packages/ui/src/components/Datepicker/helpers.ts (3)
- 1-2: The addition of imports for
DateFNSFormatandDateFNSLocaleis necessary for enabling the new date formatting feature, aligning with the objective of providing customizable date formatting capabilities.- 4-5: Defining a type for available locales enhances type safety and clarity in the code, supporting the new date formatting feature by specifying the types of locales that can be used.
- 105-116: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [108-127]
The modification of the
getFormattedDatefunction to include adateFormatparameter and utilizeDateFNSFormatfor date formatting aligns with best practices for localization support. The approach to determining the locale ensures that the correct locale object is used for formatting.packages/ui/src/components/Datepicker/helpers.spec.tsx (3)
- 157-157: The update to include an additional argument for the date format string in the
getFormattedDatefunction call is necessary for testing the new formatting capabilities. The specified format matches the expected output, ensuring the test correctly assesses the functionality.- 164-164: The update to include an additional argument for the date format string in the
getFormattedDatefunction call, along with specified options, is necessary for testing the new formatting capabilities. The approach is consistent with the objective of testing the new feature.- 170-170: The update to test the localization feature by specifying a different language code is important for ensuring the feature works as expected across different locales. This is crucial for testing the localization aspect of the new feature.
|
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.
Actionable comments posted: 0
Review Status
Configuration used: CodeRabbit UI
Files selected for processing (1)
- packages/ui/package.json (1 hunks)
Additional Context Used
Additional comments not posted (1)
packages/ui/package.json (1)
80-80: The addition ofdate-fnsversion3.6.0to the dependencies is correctly implemented and aligns with the PR's objectives. Using a fixed version ensures stability but consider the trade-offs between fixed versions and version ranges for future maintenance.
|
@dhavalveera could you please rebase it with main? I want to get this one approved. :) |
@rluders --- this conflicts, can't be done from my end, as I don't have the write access to this repository. |
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.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- packages/ui/package.json (1 hunks)
Additional comments not posted (2)
packages/ui/package.json (2)
64-64: The addition ofdate-fnswith a fixed version number3.6.0aligns with the PR's objectives to enhance date formatting capabilities in the DatePicker component.
64-64: The addition ofclean-packageis a good practice for keeping the package lightweight for production. Ensure that thepostpublishscript correctly restores removed sections.Verification successful
The
postpublishscript is correctly configured to runclean-package restore, which should restore the removed sections. However, we should verify theclean-packagescript's configuration to ensure it is set up properly.
The
clean-packagescript is correctly configured in thescriptssection and is set to run duringprepack. Thepostpublishscript is also correctly set to restore the removed sections.
- packages/ui/package.json
"prepack": "clean-package""postpublish": "clean-package restore"Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correct configuration of `clean-package` in the `postpublish` script. # Test: Check the postpublish script for restoring the removed sections. rg --type json 'postpublish' packages/ui/package.jsonLength of output: 100
Script:
#!/bin/bash # Description: Verify the configuration of the `clean-package` script in the `scripts` section. # Test: Check the scripts section for the `clean-package` script configuration. rg --type json 'clean-package' packages/ui/package.jsonLength of output: 177
|
Hello @SutuSebastian , currently I'm facing an issue is like e.g. for |
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.
Actionable comments posted: 0
Outside diff range and nitpick comments (1)
packages/ui/src/components/Datepicker/helpers.ts (1)
Line range hint
108-128: **** Incorrect default parameter position and potential locale handling issue.
- Parameter Order: The default parameter
languageshould not precede required parameters. This can lead to confusion and errors in function calls.- Locale Handling: The way locales are handled might lead to unexpected behaviors if the locale code does not match exactly with
DateFNSLocaleentries.- export const getFormattedDate = ( - language: AvailableLocales = "enUS", - date: Date, - options?: Intl.DateTimeFormatOptions, - dateFormat: string = "dd-MMM-yyyy", - ): string => { + export const getFormattedDate = ( + date: Date, + dateFormat: string = "dd-MMM-yyyy", + language: AvailableLocales = "enUS", + options?: Intl.DateTimeFormatOptions + ): string => { - const getLocale = - language === "enUS" ? DateFNSLocale["enUS"] : Object.values(DateFNSLocale).find((l) => l.code === language); + const getLocale = DateFNSLocale[language] || DateFNSLocale.enUS;
- Parameter Order: Rearrange parameters to place the default parameter last.
- Locale Handling: Simplify the locale retrieval logic to directly access locales from
DateFNSLocaleand provide a fallback toenUSif the specified locale is not found.Tools
Biome
[error] 109-109: This default parameter should follow the last required parameter or should be a required parameter. (lint/style/useDefaultParameterLast)
The last required parameter is here:
A default parameter that precedes a required parameter cannot be omitted at call site.
Unsafe fix: Turn the parameter into a required parameter.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
bun.lockbis excluded by!**/bun.lockb
Files selected for processing (2)
- package.json (1 hunks)
- packages/ui/src/components/Datepicker/helpers.ts (3 hunks)
Files skipped from review due to trivial changes (1)
- package.json
Additional context used
Learnings (1)
packages/ui/src/components/Datepicker/helpers.ts (1)
User: rluders PR: themesberg/flowbite-react#1314 File: packages/ui/src/components/Datepicker/helpers.ts:112-119 Timestamp: 2024-03-25T13:03:57.322Z Learning: The `getFormattedDate` function in `packages/ui/src/components/Datepicker/helpers.ts` should use `DateFNSFormat` directly with the date and format string for proper localization support, mapping language codes to locale objects from `date-fns/locale`.
Biome
packages/ui/src/components/Datepicker/helpers.ts
[error] 109-109: This default parameter should follow the last required parameter or should be a required parameter. (lint/style/useDefaultParameterLast)
The last required parameter is here:
A default parameter that precedes a required parameter cannot be omitted at call site.
Unsafe fix: Turn the parameter into a required parameter.
Additional comments not posted (1)
packages/ui/src/components/Datepicker/helpers.ts (1)
5-5: **** Good use of TypeScript for defining type aliases.Defining
AvailableLocalesas a type alias for locale keys indate-fns/localeensures type safety and better integration with thedate-fnslibrary.
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.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- packages/ui/package.json (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- packages/ui/package.json
|
Any news on this ? |
|
any update ? |
|
needs a re-work after #1190 is merged |
|
I think #1190 is already merged. If not i would like to work on it. |
|
Any update on this? |
🛠️ Enhancement: Added Date Formatting Support to DatePicker
This PR introduces date formatting support to the DatePicker component by leveraging the
date-fnspackage. Users can now customize date formats such asdd-MMM-yyyyordd MMM yyyyaccording to their preferences.outputs/examples:
dd-MMM-yyyy // output => 11-Mar-2024
dd MMM yyyy // output => 11 Mar 2024
dd-MM-yyyyy // output => 11-03-2024
Summary by CodeRabbit
New Features
Improvements
date-fns.Dependencies
date-fnsversion3.6.0to support date formatting and localization.