feat: add LocaleDateTime for markdown time #12426
Conversation
wackerow
requested review from
corwintines,
pettinarip,
minimalsm and
nhsz
as code owners
WalkthroughThis change introduces a new Changes
Assessment against linked issues
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 (
|
✅ Deploy Preview for ethereumorg ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files selected for processing (2)
- src/components/LocaleDateTime.tsx (1 hunks)
- src/components/MdComponents.tsx (2 hunks)
Additional comments: 7
src/components/LocaleDateTime.tsx (6)
- 3-8: The
LocaleDateTimePropstype definition is clear and well-documented through the prop names. However, it might be beneficial to include comments for each prop, especially foroptionsto clarify what kind of options are expected here, as they directly relate toIntl.DateTimeFormatOptions.- 29-29: Using
useRouterfrom Next.js to determine the locale is a smart approach for localization. This ensures that the date and time are formatted according to the user's locale, enhancing the user experience for international users.- 31-40: The default options for date and time formatting (
defaultDateOptionsanddefaultTimeOptions) are well-chosen, covering the most common elements of a date and time. This ensures that, by default, the component will render a comprehensive and understandable date-time string for users.- 41-44: The merging of
defaultDateOptions,defaultTimeOptions, and user-providedoptionsis done elegantly, allowing for a high degree of customization while maintaining sensible defaults. This pattern is a good practice for components that need to balance flexibility with ease of use.- 47-49: The use of the
<time>HTML element with bothdateTimeand the formatted string is semantically correct and enhances the accessibility of the website. This is a good practice for date and time representation in HTML.- 53-53: Exporting the
LocaleDateTimecomponent as a default export is consistent with the module system in React. This makes it easy to import and use the component elsewhere in the project.src/components/MdComponents.tsx (1)
- 43-43: Importing the
LocaleDateTimecomponent is done correctly. This ensures that the component is available for use within theMdComponentsfile, which is crucial for its integration into the markdown rendering process.
| if (hideDate && hideTime) | ||
| throw new Error( | ||
| "LocaleDateTime hideDate and hideTime props cannot both be true" | ||
| ) |
There was a problem hiding this comment.
Throwing an error when both hideDate and hideTime are true is a good safeguard against misuse of the component. However, consider the user experience on the website if this error occurs. It might be more user-friendly to log a warning and default to showing both date and time, or just the date, instead of throwing an error that could potentially break the page rendering.
There was a problem hiding this comment.
Agree with the rabbit @wackerow, this should log but not throw
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
| if (hideDate && hideTime) | ||
| throw new Error( | ||
| "LocaleDateTime hideDate and hideTime props cannot both be true" | ||
| ) |
There was a problem hiding this comment.
Agree with the rabbit @wackerow, this should log but not throw
Description
timehtml element inMdComponentsfor use within markdown pagesutcDateTimeprop which will be parsed as aDateobjecthideDateORhideTimeprop to optionally hide one half of the resulting stringoptionsoverride to customize styling if neededExample usage
This PR only adds the component but does not implement it anywhere.
Example of how to use would be
<time utcDateTime="2024-04-20T01:00:00Z" />inside a markdown file, which would render asApril 19, 2024 at 9:00:00 PMfor Eastern US timezone. AddinghideTimewould result inApril 19, 2024, and addinghideDatewould render as6:00:00 PM. A differentlocalewould adjust accordingly.Related Issue
Summary by CodeRabbit
LocaleDateTimecomponent for displaying date and time in a localized format with customization options.