-
-
Notifications
You must be signed in to change notification settings - Fork 430
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
Feat/render timeline point inner content #1520
base: main
Are you sure you want to change the base?
Feat/render timeline point inner content #1520
Conversation
🦋 Changeset detectedLatest commit: de75a47 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 |
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
packages/ui/src/components/Timeline/TimelinePoint.tsxOops! Something went wrong! :( ESLint: 8.57.0 ESLint couldn't find the plugin "eslint-plugin-react". (The package "eslint-plugin-react" was not found when loaded as a Node module from the directory "/packages/ui".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following:
The plugin "eslint-plugin-react" was referenced from the config file in "packages/ui/.eslintrc.cjs". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. WalkthroughThe pull request introduces a new "Render props" feature for the Timeline component, enhancing its documentation and implementation across several files. This feature allows developers to customize the inner content of timeline points by rendering components such as avatars. Changes include updates to documentation, the addition of a new React component, export statements, test cases, and Storybook stories, collectively improving the flexibility and usability of the Timeline component. Changes
Sequence DiagramsequenceDiagram
participant Developer
participant TimelineComponent
participant RenderProp
Developer->>TimelineComponent: Configure Timeline
Developer->>TimelineComponent: Provide render prop
TimelineComponent->>RenderProp: Invoke render function
RenderProp-->>TimelineComponent: Return custom React element
TimelineComponent->>Developer: Render custom content
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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration 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.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/ui/src/components/Timeline/Timeline.stories.tsx (1)
54-144
: NewActivityLog
storyDefining a realistic “activity log” scenario with custom rendering of
Avatar
insideTimeline.Point
effectively showcases the utility of the newrender
prop. Great for user clarity.
Consider referencing the newrender
prop in story comments for discoverability.apps/web/examples/timeline/timeline.render.tsx (1)
109-197
: Encapsulate repeated structures for maintainability.
Three similarTimelineItem
blocks are repeated (lines 113-138, 139-170, 171-194). Consider abstracting common patterns (e.g., the<Avatar>
rendering and markup inside<TimelineItem>
) into a reusable helper component or a higher-order function to minimize duplication and improve maintainability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/web/content/docs/components/timeline.mdx
(1 hunks)apps/web/examples/timeline/index.ts
(1 hunks)apps/web/examples/timeline/timeline.render.tsx
(1 hunks)packages/ui/src/components/Timeline/Timeline.spec.tsx
(3 hunks)packages/ui/src/components/Timeline/Timeline.stories.tsx
(2 hunks)packages/ui/src/components/Timeline/TimelinePoint.tsx
(2 hunks)
🔇 Additional comments (12)
apps/web/examples/timeline/index.ts (1)
4-4
: Export statement looks good.
Exporting the render
function here ensures it's accessible elsewhere. Make sure the named export render
matches the default or named export in the target file for consistency.
packages/ui/src/components/Timeline/TimelinePoint.tsx (3)
29-29
: New optional prop: render
Adding render?: () => React.ReactElement;
extends component flexibility nicely. This approach provides a clean way to inject custom content into the timeline point.
37-37
: Default value initialization
Defining render = undefined
helps clarify usage and ensures consistent handling internally. Looks fine.
56-57
: Conditional rendering is well-structured.
Using Icon
if present, otherwise falling back to render
, then a fallback marker, is logically consistent. Nicely implemented for clarity and maintainability.
packages/ui/src/components/Timeline/Timeline.stories.tsx (1)
2-3
: New imports for Avatar and Badge
These imports enhance the story’s visual demonstration of custom timeline points and item content. No issues found.
packages/ui/src/components/Timeline/Timeline.spec.tsx (3)
24-29
: Test coverage for the render prop (horizontal mode)
This test ensures that the custom inner content is rendered. The assertion .toContainHTML("img")
is straightforward and sufficient to confirm correctness.
66-71
: Test coverage for the render prop (vertical mode)
Likewise, verifying the custom content in vertical mode is consistent and provides full coverage of both orientations.
152-168
: Helper component TestTimelineWithRenderProp
The example usage of render={() => <img ... />}
is concise and clearly demonstrates the custom render flow. Good addition to the test suite for completeness.
apps/web/examples/timeline/timeline.render.tsx (3)
1-2
: Use of "use client"
is correct and aligns with Next.js app directory best practices.
No issues found related to rendering on the client side.
15-46
: Ensure synchronization of example code with the actual component usage.
The multiline string in code
(lines 15-46) displays the same component structure as defined later in the file. Any future modifications to the Component
should be mirrored here, to avoid inconsistency in the example code snippet.
199-211
: Export object provides a clean story/demo reference.
No issues found with the explanation or code snippet references. The definition of render: CodeData
is straightforward and aids in providing a user-facing example.
apps/web/content/docs/components/timeline.mdx (1)
34-39
: Documentation properly illustrates the new render
prop usage.
Adding a dedicated “Render props” section clarifies how to pass a component, like Avatar
, within the TimelinePoint
. The example reference is concise and effectively guides users toward 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.
make sure to run bun changeset
and select flowbite-react
for a patch
change, give it a title and add the file to this PR
} | ||
|
||
export const TimelinePoint: FC<TimelnePointProps> = ({ | ||
children, | ||
className, | ||
icon: Icon, | ||
theme: customTheme = {}, | ||
render = undefined, |
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.
the default to undefined
is redundant in this case
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.changeset/lucky-ladybugs-film.md (1)
1-5
: Enhance the changeset description with more details.While the description is clear, it would be more helpful to include:
- An example of how to use the new
render
prop- Any migration notes if applicable
- The motivation behind this change (reference to issue Allow Avatar as a Timeline.Point #1276)
Consider expanding the description like this:
--- "flowbite-react": patch --- -Allow `Timeline.Point` to render components as inner content. +Allow `Timeline.Point` to render components as inner content through an optional `render` prop. + +Example usage: +```jsx +<Timeline.Point render={() => <Avatar img="path/to/image" size="md" rounded />} /> +``` + +Resolves #1276 by providing more flexibility in customizing timeline points.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.changeset/lucky-ladybugs-film.md
(1 hunks)packages/ui/src/components/Timeline/TimelinePoint.tsx
(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/ui/src/components/Timeline/TimelinePoint.tsx
Issue #1276
Summary
This PR adds an optional
render
prop on theTimelinePoint
to render any component as inner content for theTimelinePoint
component.Example usage
Preview
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes
Documentation