Consultation update page form UI updated #9074
Conversation
|
Warning Rate limit exceeded@modamaan has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 3 minutes and 20 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughThe pull request introduces significant updates to several components in the application, particularly the Changes
Assessment against linked issues
Suggested labels
Suggested reviewers
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 (
|
❌ Deploy Preview for care-ohc failed.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (10)
src/components/ui/separator.tsx (1)
14-26: Consider using CSS variables for better maintainability.The implementation is solid, but the hardcoded color values could be moved to CSS variables for better maintainability and theming support.
Consider this improvement:
- "shrink-0 bg-gray-200 dark:bg-gray-800", + "shrink-0 bg-[var(--separator-color)]",Then define in your CSS:
:root { --separator-color: #e5e7eb; } .dark { --separator-color: #1f2937; }src/components/ui/badge.tsx (3)
1-4: Minor: Fix import formatting.The imports should follow consistent formatting with semicolons and proper ordering (third-party modules before local imports).
-import * as React from "react" -import { cva, type VariantProps } from "class-variance-authority" +import { type VariantProps, cva } from "class-variance-authority"; +import * as React from "react"; -import { cn } from "@/lib/utils" +import { cn } from "@/lib/utils";🧰 Tools
🪛 GitHub Check: lint
[failure] 1-1:
Replaceimport·*·as·React·from·"react"⏎import·{·cva,·type·VariantProps·}·from·"class-variance-authority"withimport·{·type·VariantProps,·cva·}·from·"class-variance-authority";⏎import·*·as·React·from·"react";
[failure] 4-4:
Insert;
6-24: Consider adding size variants and improving accessibility.The variant implementation is solid with good dark mode support and focus states. Consider these enhancements:
- Add size variants for more flexibility:
const badgeVariants = cva( "inline-flex items-center rounded-md border border-gray-200 px-2.5 py-0.5 text-xs font-semibold transition-colors focus:outline-none focus:ring-2 focus:ring-gray-950 focus:ring-offset-2 dark:border-gray-800 dark:focus:ring-gray-300", { variants: { + size: { + sm: "text-xs px-2 py-0.5", + md: "text-sm px-2.5 py-1", + lg: "text-base px-3 py-1.5", + }, variant: { // ... existing variants }, }, defaultVariants: { variant: "default", + size: "sm", }, } );🧰 Tools
🪛 GitHub Check: lint
[failure] 23-23:
Insert,
[failure] 24-24:
Insert;
26-28: Add JSDoc documentation and explicit aria props.Consider adding documentation and explicit aria props for better type safety and developer experience.
+/** + * Badge component props + * @property {string} variant - Visual style variant of the badge + * @property {string} className - Additional CSS classes + */ export interface BadgeProps extends React.HTMLAttributes<HTMLDivElement>, - VariantProps<typeof badgeVariants> {} + VariantProps<typeof badgeVariants> { + 'aria-label'?: string; +}src/components/ui/card.tsx (3)
5-18: Consider using semantic HTML for better accessibility.While the implementation is solid, consider using
<article>or<section>instead of<div>to improve semantic meaning and accessibility.- <div + <article ref={ref} className={cn( "rounded-xl border border-gray-200 bg-white text-gray-950 shadow dark:border-gray-800 dark:bg-gray-950 dark:text-gray-50", className )} {...props} - /> + />🧰 Tools
🪛 GitHub Check: lint
[failure] 13-13:
Insert,
[failure] 17-17:
Insert;
20-30: Consider using semantic HTML header element.For better semantic structure and accessibility, consider using a
<header>element instead of a<div>.- <div + <header ref={ref} className={cn("flex flex-col space-y-1.5 p-6", className)} {...props} - /> + />
56-74: Consider semantic elements for content and footer.Enhance the semantic structure:
For CardContent:
- <div ref={ref} className={cn("p-6 pt-0", className)} {...props} /> + <main ref={ref} className={cn("p-6 pt-0", className)} {...props} />For CardFooter:
- <div + <footer ref={ref} className={cn("flex items-center p-6 pt-0", className)} {...props} - /> + />src/components/Patient/DailyRoundListDetails.tsx (3)
Line range hint
49-82: Consider improving mobile layout for the update button.While the overall header layout is well-structured, the update button placement on mobile could be improved. Currently, it has a
mt-2which might create inconsistent spacing on smaller screens.Consider this modification:
- <div> - <div className="mt-2"> + <div className="sm:ml-4"> + <div className="sm:mt-0"> <ButtonV2 href={`/facility/${facilityId}/patient/${patientId}/consultation/${consultationId}/daily-rounds/${id}/update`} >
83-117: Clean up unnecessary whitespace in JSX.The code contains some unnecessary whitespace characters that should be removed for consistency.
Apply these changes:
- Temperature:{" "} + Temperature: - {" "} - {dailyRoundListDetailsData.pulse ?? "-"} + {dailyRoundListDetailsData.pulse ?? "-"}
118-142: Consider adding visual indicators for BP values.The blood pressure section could benefit from visual indicators to quickly assess if the values are within normal range.
Consider adding a color-coded badge or icon based on the BP values:
+const getBPStatus = (systolic?: number, diastolic?: number) => { + if (!systolic || !diastolic) return null; + if (systolic >= 140 || diastolic >= 90) return "high"; + if (systolic <= 90 || diastolic <= 60) return "low"; + return "normal"; +}; <div className="space-y-4"> - <h3 className="text-lg font-medium">Blood Pressure</h3> + <div className="flex items-center gap-2"> + <h3 className="text-lg font-medium">Blood Pressure</h3> + {getBPStatus( + dailyRoundListDetailsData.bp?.systolic, + dailyRoundListDetailsData.bp?.diastolic + ) && ( + <Badge variant={getBPStatus( + dailyRoundListDetailsData.bp?.systolic, + dailyRoundListDetailsData.bp?.diastolic + )}> + {getBPStatus( + dailyRoundListDetailsData.bp?.systolic, + dailyRoundListDetailsData.bp?.diastolic + )} + </Badge> + )} + </div>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
src/components/Patient/DailyRoundListDetails.tsx(3 hunks)src/components/ui/badge.tsx(1 hunks)src/components/ui/card.tsx(1 hunks)src/components/ui/separator.tsx(1 hunks)
🧰 Additional context used
🪛 GitHub Check: lint
src/components/ui/badge.tsx
[failure] 1-1:
Replace import·*·as·React·from·"react"⏎import·{·cva,·type·VariantProps·}·from·"class-variance-authority" with import·{·type·VariantProps,·cva·}·from·"class-variance-authority";⏎import·*·as·React·from·"react";
[failure] 4-4:
Insert ;
[failure] 23-23:
Insert ,
[failure] 24-24:
Insert ;
[failure] 33-33:
Insert ;
[failure] 36-36:
Insert ;
src/components/ui/card.tsx
[failure] 1-1:
Insert ;
[failure] 3-3:
Insert ;
[failure] 13-13:
Insert ,
[failure] 17-17:
Insert ;
🔇 Additional comments (7)
src/components/ui/separator.tsx (4)
1-4: LGTM! Clean imports following best practices.
The imports are well-organized, using RadixUI primitives for accessibility and the utility function for consistent styling.
6-13: LGTM! Excellent TypeScript implementation.
The component is well-typed using forwardRef with proper type inheritance from RadixUI primitives. Default props are sensibly chosen for common use cases.
27-29: LGTM! Proper component naming and export.
Correctly sets the display name for debugging and uses appropriate export style for a UI component.
1-29: Verify integration with consultation update page.
Let's ensure this separator component is properly integrated into the consultation update page UI mentioned in the PR objectives.
src/components/ui/badge.tsx (1)
36-36: LGTM!
The exports are well-structured, exporting both the component and variants for flexibility.
🧰 Tools
🪛 GitHub Check: lint
[failure] 36-36:
Insert ;
src/components/ui/card.tsx (1)
1-76: Well-structured component library implementation!
The card components are well-implemented with:
- Consistent use of forwardRef
- Proper TypeScript types
- Flexible className composition
- Dark mode support
- Modular component structure
The implementation aligns well with the PR objectives to improve the consultation update page form UI.
🧰 Tools
🪛 GitHub Check: lint
[failure] 1-1:
Insert ;
[failure] 3-3:
Insert ;
[failure] 13-13:
Insert ,
[failure] 17-17:
Insert ;
src/components/Patient/DailyRoundListDetails.tsx (1)
Line range hint 1-48: LGTM! Well-organized imports and component setup.
The new imports are properly structured and the component setup maintains good separation of concerns with proper data fetching.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (4)
src/components/ui/card.tsx (4)
5-19: Enhance Card component accessibility.Consider adding ARIA role and attributes to improve accessibility:
<div ref={ref} + role="article" + aria-labelledby={props['aria-labelledby']} className={cn( "rounded-xl border border-gray-200 bg-white text-gray-950 shadow dark:border-gray-800 dark:bg-gray-950 dark:text-gray-50", className, )} {...props} />
20-31: Use semantic HTML for CardHeader.Consider using
headerelement instead ofdivfor better semantic structure:-<div +<header ref={ref} className={cn("flex flex-col space-y-1.5 p-6", className)} {...props} -/> +/>
56-63: Consider making padding configurable.The current implementation has hardcoded padding values. Consider making them configurable through props for better flexibility:
+interface CardContentProps extends React.HTMLAttributes<HTMLDivElement> { + noPadding?: boolean; +} const CardContent = React.forwardRef< HTMLDivElement, - React.HTMLAttributes<HTMLDivElement> + CardContentProps ->(({ className, ...props }, ref) => ( +>(({ className, noPadding, ...props }, ref) => ( <div ref={ref} - className={cn("p-6 pt-0", className)} + className={cn(!noPadding && "p-6 pt-0", className)} {...props} /> ));
64-74: Use semantic HTML for CardFooter.Consider using
footerelement instead ofdivfor better semantic structure:-<div +<footer ref={ref} className={cn("flex items-center p-6 pt-0", className)} {...props} -/> +/>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
src/components/ui/badge.tsx(1 hunks)src/components/ui/card.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/ui/badge.tsx
🔇 Additional comments (3)
src/components/ui/card.tsx (3)
1-4: LGTM! Clean and well-organized imports.
32-55: Skipping review for CardTitle and CardDescription.
A previous review comment already covers the semantic HTML improvements needed for these components.
76-83: LGTM! Clean and well-organized exports.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (5)
src/components/ui/card.tsx (3)
5-18: Add ARIA role for better accessibility.The Card component would benefit from an explicit ARIA role to improve screen reader support.
<div ref={ref} + role="article" className={cn( "rounded-xl border border-gray-200 bg-white text-gray-950 shadow dark:border-gray-800 dark:bg-gray-950 dark:text-gray-50", className, )} {...props} />
63-73: Add semantic role to footer component.Consider adding a semantic role to the footer component for better accessibility.
<div ref={ref} + role="contentinfo" className={cn("flex items-center p-6 pt-0", className)} {...props} />
1-82: Consider implementing component composition patterns.This card component system provides a good foundation for the consultation update form UI. To maximize reusability and maintainability:
- Consider creating preset card variants for different use cases (e.g., consultation cards, patient info cards)
- Document common composition patterns in Storybook or similar documentation
- Create higher-level components for specific consultation form sections using these card components
This will help maintain consistency across the consultation update page and other similar features.
src/components/Patient/DailyRoundListDetails.tsx (2)
Line range hint
49-82: Enhance accessibility for patient details section.While the layout is well-structured, consider adding ARIA attributes to improve screen reader experience.
- <Card className="w-full max-w-8xl mx-auto my-5"> + <Card className="w-full max-w-8xl mx-auto my-5" role="region" aria-label="Patient Details"> - <div className="flex flex-col sm:flex-row sm:items-center gap-2"> + <div className="flex flex-col sm:flex-row sm:items-center gap-2" role="group" aria-label="Patient Category and Timestamp">
143-217: Add validation for respiratory rate.Consider adding validation for the respiratory rate to ensure it falls within normal ranges.
const isValidRespiratoryRate = (rate: number) => rate >= 12 && rate <= 25; // Usage: <p className="text-sm text-muted-foreground"> {dailyRoundListDetailsData.resp && isValidRespiratoryRate(Number(dailyRoundListDetailsData.resp)) ? dailyRoundListDetailsData.resp : "-"} </p>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
src/components/Patient/DailyRoundListDetails.tsx(3 hunks)src/components/ui/badge.tsx(1 hunks)src/components/ui/card.tsx(1 hunks)src/components/ui/separator.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/components/ui/badge.tsx
- src/components/ui/separator.tsx
🔇 Additional comments (7)
src/components/ui/card.tsx (4)
1-4: LGTM! Clean and focused imports.
20-30: LGTM! Well-structured header component.
32-53: LGTM! Semantic HTML elements properly implemented.
Previous review suggestions for semantic HTML have been successfully addressed.
75-82: LGTM! Clean and organized exports.
src/components/Patient/DailyRoundListDetails.tsx (3)
1-15: LGTM! Well-organized imports with appropriate icon choices.
The new imports enhance the UI with meaningful icons and modern components.
118-142: LGTM! Clean layout for blood pressure readings.
The separation of systolic and diastolic values improves readability.
219-235: Update badge styling as suggested in previous review.
The Bootstrap badge classes are still present and should be updated to match the new UI components.
There was a problem hiding this comment.
I'd recommend to change to using this component (https://github.com/ohcnetwork/care_fe/blob/develop/src/components/LogUpdate/CriticalCarePreview.tsx) instead as that's the ones used for newer daily rounds and making improvements there. Do rename the component too when doing that to something generic (LogUpdatePreviewPage / LogUpdateDetails).
|
|
|
Yup |
|
Do delete the old component too once done |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
src/components/LogUpdate/CriticalCarePreview.tsx (3)
1-1: Improve import statement formattingThe import statement should be formatted with each icon on a new line for better readability.
-import { ClipboardIcon, StethoscopeIcon, UserIcon,Bandage, Bed } from "lucide-react"; +import { + Bandage, + Bed, + ClipboardIcon, + StethoscopeIcon, + UserIcon, +} from "lucide-react";🧰 Tools
🪛 GitHub Check: lint
[failure] 1-1:
Replace·ClipboardIcon,·StethoscopeIcon,·UserIcon,Bandage,·Bed·with⏎··Bandage,⏎··Bed,⏎··ClipboardIcon,⏎··StethoscopeIcon,⏎··UserIcon,⏎
120-143: Fix inconsistent spacing in className attributesThere are multiple instances of double spaces in className attributes.
- <div className="flex items-center rounded-lg border p-4"> + <div className="flex items-center rounded-lg border p-4"> - <div className="flex items-center rounded-lg border p-4"> + <div className="flex items-center rounded-lg border p-4"> - <div className="flex items-center rounded-lg border p-4"> + <div className="flex items-center rounded-lg border p-4">
290-305: Consider centralizing unit suffixesThe "ml/h" suffix is repeated in multiple places. Consider centralizing it as a constant to ensure consistency and easier maintenance.
+const FLOW_RATE_UNIT = "ml/h"; + <Section title="Dialysis"> <div className="grid grid-cols-1 gap-4 sm:grid-cols-2 lg:grid-cols-2"> <div> <Detail label="Fluid Balance" value={data.dialysis_fluid_balance} - suffix="ml/h" + suffix={FLOW_RATE_UNIT} /> </div> <div> <Detail label="Net Balance" value={data.dialysis_net_balance} - suffix="ml/h" + suffix={FLOW_RATE_UNIT} /> </div> </div> </Section>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
src/components/LogUpdate/CriticalCarePreview.tsx(6 hunks)
🧰 Additional context used
🪛 GitHub Check: lint
src/components/LogUpdate/CriticalCarePreview.tsx
[failure] 1-1:
Replace ·ClipboardIcon,·StethoscopeIcon,·UserIcon,Bandage,·Bed· with ⏎··Bandage,⏎··Bed,⏎··ClipboardIcon,⏎··StethoscopeIcon,⏎··UserIcon,⏎
[failure] 5-5:
Delete ⏎
🔇 Additional comments (3)
src/components/LogUpdate/CriticalCarePreview.tsx (3)
73-100: LGTM! Well-structured grid layout with clear visual hierarchy
The implementation uses a clean grid layout with consistent spacing and meaningful icons, improving the visual organization of patient information.
Line range hint 241-274: LGTM! Well-implemented responsive grid layout
The grid layout implementation properly handles different screen sizes and maintains consistent spacing.
336-377: LGTM! Well-structured vital signs display
The implementation properly handles:
- Medical terminology with subscript text
- Range-based value descriptions
- Responsive grid layout
Proposed Changes
Updated UI
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit
Release Notes
New Features
Badgecomponent for displaying patient categories with various styles.Separatorcomponent for improved visual separation of content.Improvements
These updates significantly enhance the user experience and improve the overall presentation of patient information.