-
Notifications
You must be signed in to change notification settings - Fork 549
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
Treatment Summary UI Enhancement #10563
base: develop
Are you sure you want to change the base?
Treatment Summary UI Enhancement #10563
Conversation
WalkthroughThe changes update the visual styling in several patient components. The modifications remove the Changes
Possibly related PRs
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: 1
🔭 Outside diff range comments (1)
src/components/Patient/MedicationStatementList.tsx (1)
179-179
: 🛠️ Refactor suggestionRemove inconsistent border styling.
The Card component still has
border-none
which conflicts with the parent component's border styling in TreatmentSummary.- <Card className={cn("border-none rounded-sm", className)}> + <Card className={cn("rounded-sm", className)}>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/components/Patient/MedicationStatementList.tsx
(2 hunks)src/components/Patient/TreatmentSummary.tsx
(3 hunks)src/components/Patient/allergy/list.tsx
(3 hunks)src/components/Patient/diagnosis/list.tsx
(3 hunks)src/components/Patient/symptoms/list.tsx
(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: cypress-run (1)
🔇 Additional comments (5)
src/components/Patient/TreatmentSummary.tsx (1)
194-230
: LGTM! Consistent border styling across components.The border styling changes are consistent across all components (AllergyList, SymptomsList, DiagnosisList, MedicationStatementList), improving the visual consistency of the treatment summary.
src/components/Patient/diagnosis/list.tsx (1)
142-142
: LGTM! Consistent Card styling.The Card styling changes align with other components, maintaining visual consistency.
src/components/Patient/symptoms/list.tsx (1)
140-140
: LGTM! Consistent Card styling.The Card styling changes align with other components, maintaining visual consistency.
src/components/Patient/allergy/list.tsx (2)
98-98
: LGTM! Consistent className prop forwarding.The
className
prop is now consistently forwarded toAllergyListLayout
across all rendering paths (loading, empty, and main states), improving the component's styling flexibility.Also applies to: 122-122
310-310
: LGTM! Enhanced Card styling with proper className composition.The styling changes:
- Add consistent padding with
p-2
- Apply subtle border radius with
rounded-sm
- Maintain className extensibility through proper composition with the
cn()
utility
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Don't need colors for the badges here: ![]() Modify the filteredMedications length check to display no medications recorded, and the table row styling as well when there's data: ![]() cc: @amjithtitus09 |
Is this done @AdityaJ2305 ? |
@amjithtitus09 ![]() LMK your thoughts
|
@amjithtitus09 Done with the changes . Ready for review 🙂 |
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/Patient/diagnosis/DiagnosisTable.tsx (1)
74-104
: Consider extracting the badge styling logic.The badge styling logic is duplicated across multiple badges. Consider extracting it into a reusable helper function or component.
+const getBadgeClassName = (isPrintPreview: boolean, styles: string) => + isPrintPreview ? "" : `whitespace-nowrap ${styles}`; <Badge variant="outline" - className={ - isPrintPreview - ? "" - : `whitespace-nowrap ${ - DIAGNOSIS_CLINICAL_STATUS_STYLES[ - diagnosis.clinical_status - ] - }` - } + className={getBadgeClassName( + isPrintPreview, + DIAGNOSIS_CLINICAL_STATUS_STYLES[diagnosis.clinical_status] + )} >src/components/Patient/symptoms/SymptomTable.tsx (1)
75-87
: Consider sharing badge styling logic between components.The badge styling implementation is identical to DiagnosisTable.tsx. Consider:
- Creating a shared helper function for badge styling
- Creating a shared styled badge component
This would improve maintainability and ensure consistent behavior across components.
// src/components/Common/PrintAwareBadge.tsx interface PrintAwareBadgeProps { isPrintPreview?: boolean; styles: string; children: React.ReactNode; } export function PrintAwareBadge({ isPrintPreview = false, styles, children, ...props }: PrintAwareBadgeProps) { return ( <Badge variant="outline" className={isPrintPreview ? "" : `whitespace-nowrap ${styles}`} {...props} > {children} </Badge> ); }Usage:
-<Badge - variant="outline" - className={ - isPrintPreview - ? "" - : `whitespace-nowrap ${ - SYMPTOM_SEVERITY_STYLES[symptom.severity] - }` - } -> +<PrintAwareBadge + isPrintPreview={isPrintPreview} + styles={SYMPTOM_SEVERITY_STYLES[symptom.severity]} +> {t(symptom.severity)} -</Badge> +</PrintAwareBadge>Also applies to: 89-101, 103-117
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/components/Patient/MedicationStatementList.tsx
(2 hunks)src/components/Patient/allergy/list.tsx
(4 hunks)src/components/Patient/diagnosis/DiagnosisTable.tsx
(1 hunks)src/components/Patient/diagnosis/list.tsx
(3 hunks)src/components/Patient/symptoms/SymptomTable.tsx
(1 hunks)src/components/Patient/symptoms/list.tsx
(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- src/components/Patient/diagnosis/list.tsx
- src/components/Patient/MedicationStatementList.tsx
- src/components/Patient/symptoms/list.tsx
- src/components/Patient/allergy/list.tsx
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: cypress-run (1)
- GitHub Check: OSSAR-Scan
🔇 Additional comments (3)
src/components/Patient/diagnosis/DiagnosisTable.tsx (2)
74-88
: LGTM! Clinical status badge styling implementation.The conditional styling based on
isPrintPreview
aligns with the PR objectives and reviewer feedback about badge colors.
90-104
: LGTM! Verification status badge styling implementation.The conditional styling based on
isPrintPreview
aligns with the PR objectives and reviewer feedback about badge colors.src/components/Patient/symptoms/SymptomTable.tsx (1)
75-117
: Verify badge styling behavior in print preview.The changes align with the PR objectives and reviewer feedback. However, let's verify the print preview appearance matches the expected design.
✅ Verification successful
🌐 Web query:
What is the recommended way to test print preview in React applications?
💡 Result:
To test print preview functionality in React applications effectively, developers should combine specific testing strategies and leverage the react-to-print library's features. Here's a structured approach:
1. Component Inspection with React Testing Library
- Use screen.debug() to inspect rendered components:
test('verify printable content', () => { render(<PrintableComponent />); screen.debug(undefined, Infinity); // Shows full component structure[10] });- This exposes hidden elements wrapped in
<div style={{ display: "none" }}>
that are only visible during printing[3][6].2. State Synchronization Testing
- Test asynchronous state handling required for printing:
test('state updates before printing', async () => { const { getByText } = render(<PrintButton />); fireEvent.click(getByText('Print')); await waitFor(() => expect(screen.getByTestId('print-content')).toBeVisible()); });- Ensures components properly use Promises to wait for DOM updates[2][3][7].
3. Print Iframe Preservation
- Enable preserveAfterPrint flag during testing:
<ReactToPrint preserveAfterPrint={true} content={() => componentRef.current} />- Allows post-print inspection of generated iframe content[7].
4. CSS Media Query Verification
Test print-specific styles using:
@media print { .no-print { display: none; } }Validate styles with tools like:
expect(getComputedStyle(element).display).toBe('none');5. Browser Print Dialog Mocking
Mock window.print() in tests:
const printMock = jest.spyOn(window, 'print').mockImplementation(); // ... test execution ... expect(printMock).toHaveBeenCalled();Key Considerations
Aspect Implementation Source Hidden Components Use display: none
wrapper[3][7] Async State Promise resolution in onBeforePrint
[2][3][7] Style Isolation Dedicated @media print CSS [1][4] Debugging preserveAfterPrint + screen.debug [7][10] For comprehensive testing, combine visual inspection of preserved print iframes with state/behavior assertions to ensure all printable elements render correctly while non-essential UI remains hidden[1][3][7].
Citations:
- 1: https://blog.bytescrum.com/mastering-react-print-a-comprehensive-guide
- 2: https://buaiscia.github.io/blog/tips/printing-developer-journey-react-to-print
- 3: https://github.com/MatthewHerbst/react-to-print
- 4: https://www.dhiwise.com/post/boosting-efficiency-optimizing-your-react-to-print-workflow
- 5: https://dev.to/ebereplenty/printing-in-react-made-easy-with-react-to-print-4b3k
- 6: https://www.browserstack.com/guide/react-testing-library-debug-method
- 7: https://www.npmjs.com/package/react-to-print
- 8: https://blog.logrocket.com/using-react-to-print-generate-printable-document/
- 9: https://supportcenter.devexpress.com/ticket/details/t1052262/react-print-report-without-preview-doesn-t-work
- 10: https://javascript.plainenglish.io/how-to-print-the-entire-component-using-screen-debug-in-react-testing-library-330db38d3e78?gi=cf5c55e36588
Print Preview Badge Styling: Behavior Verified
- The conditional logic using
isPrintPreview
correctly bypasses the addition of extra style classes in print preview mode.- The code omits badge-specific styling when printing, which is in line with the expected design.
- Recommended testing approaches (e.g., with React Testing Library and react-to-print) support verifying that print-specific rendering omits non-print styles.
In the right direction but few more changes needed here. There is still bg colour, and coloured labels. Also border needs to be consistent for all tables. Check if there's shadow for the questionnaire responses at the bottom. |
@amjithtitus09n, Let me know ur input I just want to confirm if I’ve understood the requirements correctly: |
|
…/AdityaJ2305/care_fe into treatment_summary_ui_enhancement
Proposed Changes
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit