-
Notifications
You must be signed in to change notification settings - Fork 649
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
Enhance Comment Section UI with Redesigned User Icon and Styling #10458
Enhance Comment Section UI with Redesigned User Icon and Styling #10458
Conversation
WalkthroughThe changes enhance the comment section by introducing a tooltip on the avatar to display the creator’s name, replacing plain text with Markdown for comment formatting, and improving overall layout using flexbox and semantic HTML with a Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant C as Comment Component
participant T as TooltipComponent
U->>C: Hover over Avatar
C->>T: Trigger tooltip with creator name
T-->>U: Display tooltip
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
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 (
|
✅ Deploy Preview for care-ohc ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
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)
src/components/Resource/ResourceCommentSection.tsx (2)
146-160
: Consider extracting the tooltip implementation into a reusable component.The tooltip implementation could be extracted into a reusable component to promote DRY principles and improve maintainability.
Here's a suggested implementation:
// components/Common/TooltipAvatar.tsx interface TooltipAvatarProps { name: string; className?: string; } export const TooltipAvatar = ({ name, className }: TooltipAvatarProps) => ( <TooltipProvider> <Tooltip> <TooltipTrigger asChild> <div className="flex"> <Avatar name={name} className={className} /> </div> </TooltipTrigger> <TooltipContent> <p>{formatName(name)}</p> </TooltipContent> </Tooltip> </TooltipProvider> );Then simplify the usage in the Comment component:
- <TooltipProvider> - <Tooltip> - <TooltipTrigger asChild> - <div className="flex"> - <Avatar - name={`${created_by.first_name} ${created_by.last_name}`} - className="w-8 h-8 rounded-full object-cover" - /> - </div> - </TooltipTrigger> - <TooltipContent> - <p>{formatName(created_by)}</p> - </TooltipContent> - </Tooltip> - </TooltipProvider> + <TooltipAvatar + name={`${created_by.first_name} ${created_by.last_name}`} + className="w-8 h-8 rounded-full object-cover" + />
140-143
: Consider extracting animation classes to a constant.The animation classes could be moved to a constant to ensure consistency across the application and make updates easier to manage.
// constants/animations.ts export const FADE_IN_ANIMATION = 'animate-in fade-in-0 slide-in-from-bottom-4'; // Then in the component: className={cn( "flex w-full mb-4", FADE_IN_ANIMATION, "justify-start" )}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/Resource/ResourceCommentSection.tsx
(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: cypress-run (1)
🔇 Additional comments (2)
src/components/Resource/ResourceCommentSection.tsx (2)
11-16
: LGTM! Well-organized imports.The new imports for tooltips and formatting utilities align well with the UI enhancement objectives.
Also applies to: 27-27
167-174
: Great use of semantic HTML and accessibility!The
<time>
element with properdateTime
andtitle
attributes improves accessibility and provides better user experience.
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.
self reviewed
comment order updated, latest comment should be first
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)
src/components/Resource/ResourceCommentSection.tsx (1)
148-162
: Enhance tooltip accessibility.While the tooltip implementation looks good, consider adding an aria-label to improve accessibility.
- <TooltipTrigger asChild> + <TooltipTrigger asChild aria-label={`View ${formatName(created_by)}'s full name`}>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/Resource/ResourceCommentSection.tsx
(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: cypress-run (1)
🔇 Additional comments (3)
src/components/Resource/ResourceCommentSection.tsx (3)
11-16
: LGTM! Well-structured imports for UI enhancements.The new imports for Tooltip components and formatting utilities align well with the UI enhancement objectives.
Also applies to: 27-27
100-107
: LGTM! Improved comment ordering.The reverse ordering ensures newest comments appear first, improving user experience. The implementation maintains proper React list rendering practices with unique keys.
172-178
: LGTM! Excellent timestamp implementation.Great use of the semantic
time
element with both relative and absolute timestamps. The implementation follows accessibility best practices by providing machine-readable datetime attribute and human-friendly display.
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: 1
🧹 Nitpick comments (1)
src/components/Resource/ResourceCommentSection.tsx (1)
101-108
: Optimize comment ordering performance.Creating a new reversed array on every render using
slice().reverse()
is inefficient. Consider reversing the order at the data source or storing the reversed array.- {resourceComments?.results - ?.slice() - .reverse() - .map((comment) => ( + {[...(resourceComments?.results || [])] + .reverse() + .map((comment) => (Alternatively, consider adding a query parameter to fetch comments in reverse order from the API.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/Resource/ResourceCommentSection.tsx
(4 hunks)
🧰 Additional context used
🪛 GitHub Actions: Lint Code Base
src/components/Resource/ResourceCommentSection.tsx
[error] 1-1: Unresolved import: react-markdown.
🪛 GitHub Actions: Cypress Tests
src/components/Resource/ResourceCommentSection.tsx
[error] 1-1: Rollup failed to resolve import 'react-markdown'. This is most likely unintended because it can break your application at runtime. If you do want to externalize this module, explicitly add it to build.rollupOptions.external
.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: OSSAR-Scan
🔇 Additional comments (3)
src/components/Resource/ResourceCommentSection.tsx (3)
171-173
: LGTM! ReactMarkdown implementation.Great job implementing ReactMarkdown for comment rendering, which addresses the previous review feedback.
149-163
: LGTM! Enhanced user experience with tooltip.The tooltip implementation on the avatar provides better user experience by showing the full name on hover.
175-181
: LGTM! Improved date display with semantic HTML.Good use of the semantic
time
element with both machine-readabledateTime
and human-readable relative time.
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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
package.json
(2 hunks)
🧰 Additional context used
🪛 GitHub Actions: Lint Code Base
package.json
[error] 1-1: npm ci
can only install packages when your package.json and package-lock.json or npm-shrinkwrap.json are in sync. Please update your lock file with npm install
before continuing.
🪛 GitHub Actions: Cypress Tests
package.json
[error] 1-1: npm ci
can only install packages when your package.json and package-lock.json or npm-shrinkwrap.json are in sync. Please update your lock file with npm install
before continuing.
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Test
- GitHub Check: OSSAR-Scan
- GitHub Check: CodeQL-Build
🔇 Additional comments (1)
package.json (1)
214-215
: Formatting: Trailing Newline Added.
The addition of a newline at the end of the file conforms to best practices for file formatting.
@rithviknishad should I push package.json and lock file |
we don't need any new packages. we already have a markdown component |
…amaan/care_feAmaan into issues/10428/comment-section
…amaan/care_feAmaan into issues/10428/comment-section
@rithviknishad i removed the package.json and lock.json file, now it shows cypress issue. can you help me resolve it? |
LGTM |
@modamaan Your efforts have helped advance digital healthcare and TeleICU systems. 🚀 Thank you for taking the time out to make CARE better. We hope you continue to innovate and contribute; your impact is immense! 🙌 |
Proposed Changes
Fixes Redesign Comment Section User Icon and Style for Improved UI #10428
Change 1
Old

updated

@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit
New Features
Style