Refactor PrintTable and PrescriptionPreview components for improved c… #15250
Conversation
…ell rendering and styling
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds per-cell rendering and styling APIs to PrintTable (new props: className, classNameCell, cellConfig, renderCell; HeaderRow.className). PrescriptionPreview updated to use these props and a compact Changes
🚥 Pre-merge checks | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull request overview
The PR refactors the PrintTable and PrescriptionPreview components to enhance cell rendering flexibility and improve print layout styling. The changes focus on making tables more customizable through cell-specific configurations and adjusting font sizes and spacing for better print output.
Changes:
- Enhanced
PrintTablecomponent with flexible cell rendering and styling options viacellConfigandrenderCellprops - Adjusted font sizes and spacing throughout
PrescriptionPreviewfor improved print aesthetics - Re-enabled keyboard shortcuts in
MedicationBillFormand removed redundant shortcut context
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/components/Common/PrintTable.tsx | Added cell-specific configuration support with custom render functions and className overrides |
| src/components/Prescription/PrescriptionPreview.tsx | Applied new PrintTable configurations and adjusted typography for print optimization |
| src/pages/Facility/services/pharmacy/MedicationBillForm.tsx | Re-enabled keyboard shortcuts and removed duplicate shortcut context |
| ? format(new Date(encounter.period.start), "dd/mm/yy") | ||
| : format(new Date(), "dd/mm/yy") |
There was a problem hiding this comment.
The date format string uses lowercase 'mm' which represents minutes, not months. This will produce incorrect dates. Use 'dd/MM/yy' instead where 'MM' represents months.
| ? format(new Date(encounter.period.start), "dd/mm/yy") | |
| : format(new Date(), "dd/mm/yy") | |
| ? format(new Date(encounter.period.start), "dd/MM/yy") | |
| : format(new Date(), "dd/MM/yy") |
Deploying care-preview with
|
| Latest commit: |
53baa85
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://b402caf5.care-preview-a7w.pages.dev |
| Branch Preview URL: | https://manyu-print-phar.care-preview-a7w.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/components/Common/PrintTable.tsx (1)
71-105: Fix Tailwind utility typo (warp-break-words).
warp-break-wordsisn't a valid Tailwind utility; usewrap-break-wordinstead to enable proper overflow wrapping for long words.🛠️ Proposed fix
- "warp-break-words whitespace-normal text-center", + "wrap-break-word whitespace-normal text-center",src/components/Prescription/PrescriptionPreview.tsx (1)
253-259: Date format uses minutes instead of month.In date-fns,
mmrepresents minutes (00–59); this produces incorrect timestamp output. UseMMfor month (01–12).🛠️ Proposed fix
- ? format(new Date(encounter.period.start), "dd/mm/yy") - : format(new Date(), "dd/mm/yy") + ? format(new Date(encounter.period.start), "dd/MM/yy") + : format(new Date(), "dd/MM/yy")
🤖 Fix all issues with AI agents
In `@src/components/Common/PrintTable.tsx`:
- Around line 22-26: The CellConfig.value field is defined but never used in
getCellContent, so callers cannot override displayed cell text; update the
getCellContent function in PrintTable.tsx to prefer CellConfig.value (if
defined) over deriving the value from row data, and pass that chosen value into
any cellConfig.render callback (and into any fallback formatting logic) so both
plain-value and rendered paths respect the override; reference the CellConfig
type, the getCellContent function, and the render property when making this
change.
- Around line 14-18: The HeaderRow.className property is never applied when
rendering table headers in PrintTable; update the header-rendering code paths
(where PrintTable maps over headers in TableHead / headers.map) to add the
header.className to the header cell element (e.g., the <th> or TableCell used
for header rows), merging it with any existing classes (safely handling
undefined) and keeping the existing key and width handling; make the same change
for the second header-rendering block (the other headers.map usage) so
per-header styling via HeaderRow.className takes effect everywhere.
| type HeaderRow = { | ||
| key: string; | ||
| width?: number; | ||
| className?: string; | ||
| }; |
There was a problem hiding this comment.
HeaderRow.className is never applied to header cells.
HeaderRow.className is added but not used in <TableHead>, so per-header styling won’t take effect.
🛠️ Proposed fix
- {headers.map(({ key, width }, index) => (
+ {headers.map(({ key, width, className: headerClassName }, index) => (
<TableHead
className={cn(
index == 0 && "first:rounded-l-md",
"h-auto py-1 pl-2 pr-2 text-black text-center ",
width && `w-${width}`,
+ headerClassName,
)}
key={key}
>Also applies to: 75-81
🤖 Prompt for AI Agents
In `@src/components/Common/PrintTable.tsx` around lines 14 - 18, The
HeaderRow.className property is never applied when rendering table headers in
PrintTable; update the header-rendering code paths (where PrintTable maps over
headers in TableHead / headers.map) to add the header.className to the header
cell element (e.g., the <th> or TableCell used for header rows), merging it with
any existing classes (safely handling undefined) and keeping the existing key
and width handling; make the same change for the second header-rendering block
(the other headers.map usage) so per-header styling via HeaderRow.className
takes effect everywhere.
🎭 Playwright Test ResultsStatus: ❌ Failed
📊 Detailed results are available in the playwright-final-report artifact. Run: #5004 |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/components/Common/PrintTable.tsx`:
- Around line 51-68: getCellContent currently calls
cellConfig[key].render(value) which prevents per-column renderers from accessing
the key or rowIndex; update getCellContent to call cellConfig[key].render with
the same signature as renderCell (e.g., render(value, key, rowIndex)) and adjust
any CellConfig type/definition for render to accept (value, key, rowIndex) so
column-specific renderers can use the extra context while keeping the existing
renderCell(key, value, rowIndex) priority logic.
♻️ Duplicate comments (3)
src/components/Common/PrintTable.tsx (3)
14-18: HeaderRow.className is declared but never applied to header cells.The
classNameproperty added toHeaderRowat line 17 is never destructured or used when rendering<TableHead>elements (lines 75-86). Per-header styling will not take effect.🛠️ Proposed fix
- {headers.map(({ key, width }, index) => ( + {headers.map(({ key, width, className: headerClassName }, index) => ( <TableHead className={cn( index == 0 && "first:rounded-l-md", "h-auto py-1 pl-2 pr-2 text-black text-center ", width && `w-${width}`, + headerClassName, )} key={key} >
22-26: CellConfig.value is defined but never used.The
valueproperty inCellConfigis declared butgetCellContentnever reads it. Callers cannot override a cell's displayed value via config.🛠️ Proposed fix in getCellContent
+ // Priority 3: Cell-specific value override + const configuredValue = cellConfig?.[key]?.value; + if (configuredValue !== undefined) { + return configuredValue; + } + - // Priority 3: Default value or "-" + // Priority 4: Default value or "-" return value || "-";
100-109: Invalid Tailwind class:wrap-break-wordsshould bebreak-words.
wrap-break-wordsis not a valid Tailwind CSS utility class. The standard class for word breaking isbreak-words(which appliesoverflow-wrap: break-word).🛠️ Proposed fix
<TableCell className={cn( - "wrap-break-words whitespace-normal text-center", + "break-words whitespace-normal text-center", classNameCell, cellConfig?.[key]?.className, )}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/components/Prescription/PrescriptionPreview.tsx`:
- Around line 200-205: In PrescriptionPreview.tsx update the wrapping utility on
the container div (the element rendering facility.address inside the component)
by replacing the non-standard class "wrap-break-word" with Tailwind's standard
"break-words"; locate the div with className "text-gray-500 whitespace-pre-wrap
wrap-break-word text-xs" and change the class list to include "break-words" so
text wrapping behaves correctly (no other logic changes required).
- Around line 248-253: Replace the hard-coded date format usage in
PrescriptionPreview (the value prop that currently uses format(new
Date(encounter.period.start), "dd/MM/yy") or format(new Date(), "dd/MM/yy"))
with the shared utility formatDateTime by using
formatDateTime(encounter?.period?.start || new Date()); also ensure
formatDateTime is imported where PrescriptionPreview is defined so the component
uses the app-wide medical timestamp formatting.
…ell rendering and styling
Proposed Changes
Fixes #issue_number
Tagging: @ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit
New Features
Style
✏️ Tip: You can customize this high-level summary in your review settings.