Add healthcare service icon and charge item price popover in Activity Definition View #15299
+121
−16
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
WalkthroughTwo files modified: new ChargeItemDefinitionPopover component created for displaying charge item price details in a popover, and ActivityDefinitionView updated to integrate this popover component while adding duo-tone icon support for healthcare services. Changes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR enhances the Activity Definition View with two visual improvements: displaying healthcare service icons and adding an interactive price breakdown popover for charge item definitions.
Changes:
- Adds icon display for healthcare services using
styling_metadata.careIconwith fallback - Introduces interactive price popover showing detailed charge item pricing breakdown
- Creates reusable
ChargeItemDefinitionPopovercomponent for consistent price display
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/components/Billing/ChargeItem/ChargeItemDefinitionPopover.tsx |
New component that displays charge item price with expandable popover showing title, description, purpose, and detailed price breakdown |
src/pages/Facility/settings/activityDefinition/ActivityDefinitionView.tsx |
Integrates price popover for charge items and adds healthcare service icon display with dynamic icon selection |
Comment on lines
+41
to
+43
| const getIconName = (name: string): DuoToneIconName => | ||
| `d-${name}` as DuoToneIconName; | ||
|
|
There was a problem hiding this comment.
The getIconName function uses a type assertion without validation, which could cause runtime errors if an invalid icon name is passed. The function should validate that the resulting icon name exists in duoToneIcons before asserting the type, or handle the case where the icon doesn't exist by returning the fallback icon name.
Suggested change
| const getIconName = (name: string): DuoToneIconName => | |
| `d-${name}` as DuoToneIconName; | |
| const FALLBACK_ICON_NAME: DuoToneIconName = Object.keys( | |
| duoToneIcons, | |
| )[0] as DuoToneIconName; | |
| const getIconName = (name: string): DuoToneIconName => { | |
| const iconName = `d-${name}`; | |
| if (Object.prototype.hasOwnProperty.call(duoToneIcons, iconName)) { | |
| return iconName as DuoToneIconName; | |
| } | |
| return FALLBACK_ICON_NAME; | |
| }; |
| <p className="text-sm text-gray-600"> | ||
| {definition.healthcare_service.extra_details} | ||
| <div className="flex items-start gap-3"> | ||
| <div className="shrink-0 flex items-center justify-center size-10 rounded-lg bg-primary-100 text-primary-700"> |
There was a problem hiding this comment.
The Tailwind utility classes shrink-0 flex should be ordered as flex shrink-0 according to Tailwind's recommended class ordering (layout modifiers before sizing utilities). This improves consistency and readability.
Suggested change
| <div className="shrink-0 flex items-center justify-center size-10 rounded-lg bg-primary-100 text-primary-700"> | |
| <div className="flex shrink-0 items-center justify-center size-10 rounded-lg bg-primary-100 text-primary-700"> |
Deploying care-preview with
|
| Latest commit: |
cb3119a
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://e362c0d4.care-preview-a7w.pages.dev |
| Branch Preview URL: | https://issues-15298-activity-defini.care-preview-a7w.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@src/components/Billing/ChargeItem/ChargeItemDefinitionPopover.tsx`:
- Around line 42-48: The button in the ChargeItemDefinitionPopover component is
missing an explicit type and a visible keyboard focus indicator; update the
trigger button (in ChargeItemDefinitionPopover) to include type="button" to
avoid submitting parent forms, and add accessible focus styles (e.g.,
focus-visible or focus classes such as focus:outline-none removed and replaced
with a clear ring/outline like focus-visible:ring or focus:ring and
focus-visible:outline) within the same className string so keyboard users get a
visible focus state while preserving existing styles and the className prop
handling.
- Around line 17-25: Add a JSDoc block above the ChargeItemDefinitionPopover
component that documents the clinical/medical use-case and accessibility
expectations: state that ChargeItemDefinitionPopover displays billing/charge
item metadata for clinical billing workflows (e.g., clinician or billing staff
review of ChargeItemDefinitionRead), describe keyboard interactions (focusable
trigger, keyboard open/close with Enter/Escape, focus trap/return focus), and
screen-reader behavior (aria-label/role, announces title and description). Also
document the props in ChargeItemDefinitionPopoverProps (chargeItemDefinition:
ChargeItemDefinitionRead — required clinical billing data; className?: string —
optional styling hook) and reference any ARIA attributes or focus-management
behaviors implemented by the component.
In `@src/pages/Facility/settings/activityDefinition/ActivityDefinitionView.tsx`:
- Around line 39-42: getIconName currently just prefixes and force-casts, which
can produce invalid keys for already-prefixed or unknown values; update
getIconName to normalize the input (trim, toLowerCase, remove leading "d-"),
build the candidate key `d-${normalized}` and check it exists in duoToneIcons
(or DuoTonePaths) before casting, returning that key if valid or a safe fallback
key (e.g., a default duo-tone icon) otherwise; adjust any callers (e.g., where
careIcon is passed) to use the new getIconName for safe resolution.
Comment on lines
+17
to
+25
| interface ChargeItemDefinitionPopoverProps { | ||
| chargeItemDefinition: ChargeItemDefinitionRead; | ||
| className?: string; | ||
| } | ||
|
|
||
| export default function ChargeItemDefinitionPopover({ | ||
| chargeItemDefinition, | ||
| className, | ||
| }: ChargeItemDefinitionPopoverProps) { |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add medical use-case and WCAG/accessibility notes for this reusable component.
Please document the clinical use case and keyboard/screen-reader expectations in a short JSDoc block on the component. As per coding guidelines.
📝 Example documentation
+/**
+ * Charge item pricing popover used while reviewing Activity Definitions.
+ *
+ * Medical use case: exposes charge item totals and component breakdowns during
+ * clinical configuration review.
+ * Accessibility/WCAG: ensure keyboard operability and visible focus state
+ * (WCAG 2.1.1, 2.4.7).
+ */
export default function ChargeItemDefinitionPopover({
chargeItemDefinition,
className,
}: ChargeItemDefinitionPopoverProps) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| interface ChargeItemDefinitionPopoverProps { | |
| chargeItemDefinition: ChargeItemDefinitionRead; | |
| className?: string; | |
| } | |
| export default function ChargeItemDefinitionPopover({ | |
| chargeItemDefinition, | |
| className, | |
| }: ChargeItemDefinitionPopoverProps) { | |
| interface ChargeItemDefinitionPopoverProps { | |
| chargeItemDefinition: ChargeItemDefinitionRead; | |
| className?: string; | |
| } | |
| /** | |
| * Charge item pricing popover used while reviewing Activity Definitions. | |
| * | |
| * Medical use case: exposes charge item totals and component breakdowns during | |
| * clinical configuration review. | |
| * Accessibility/WCAG: ensure keyboard operability and visible focus state | |
| * (WCAG 2.1.1, 2.4.7). | |
| */ | |
| export default function ChargeItemDefinitionPopover({ | |
| chargeItemDefinition, | |
| className, | |
| }: ChargeItemDefinitionPopoverProps) { |
🤖 Prompt for AI Agents
In `@src/components/Billing/ChargeItem/ChargeItemDefinitionPopover.tsx` around
lines 17 - 25, Add a JSDoc block above the ChargeItemDefinitionPopover component
that documents the clinical/medical use-case and accessibility expectations:
state that ChargeItemDefinitionPopover displays billing/charge item metadata for
clinical billing workflows (e.g., clinician or billing staff review of
ChargeItemDefinitionRead), describe keyboard interactions (focusable trigger,
keyboard open/close with Enter/Escape, focus trap/return focus), and
screen-reader behavior (aria-label/role, announces title and description). Also
document the props in ChargeItemDefinitionPopoverProps (chargeItemDefinition:
ChargeItemDefinitionRead — required clinical billing data; className?: string —
optional styling hook) and reference any ARIA attributes or focus-management
behaviors implemented by the component.
Comment on lines
+42
to
+48
| <button | ||
| className={cn( | ||
| "text-sm font-medium underline decoration-dotted underline-offset-4 cursor-pointer hover:text-primary-700 transition-colors", | ||
| className, | ||
| )} | ||
| aria-label={t("view_details")} | ||
| > |
Contributor
There was a problem hiding this comment.
Add explicit button type and visible focus state.
Without type="button", this trigger can submit surrounding forms, and the current classes omit a keyboard focus indicator (WCAG 2.4.7).
🐛 Proposed fix
- <button
- className={cn(
- "text-sm font-medium underline decoration-dotted underline-offset-4 cursor-pointer hover:text-primary-700 transition-colors",
- className,
- )}
- aria-label={t("view_details")}
- >
+ <button
+ type="button"
+ className={cn(
+ "text-sm font-medium underline decoration-dotted underline-offset-4 cursor-pointer hover:text-primary-700 transition-colors focus-visible:outline-hidden focus-visible:ring-1 focus-visible:ring-primary-700",
+ className,
+ )}
+ aria-label={t("view_details")}
+ >🧰 Tools
🪛 Biome (2.1.2)
[error] 42-48: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
🤖 Prompt for AI Agents
In `@src/components/Billing/ChargeItem/ChargeItemDefinitionPopover.tsx` around
lines 42 - 48, The button in the ChargeItemDefinitionPopover component is
missing an explicit type and a visible keyboard focus indicator; update the
trigger button (in ChargeItemDefinitionPopover) to include type="button" to
avoid submitting parent forms, and add accessible focus styles (e.g.,
focus-visible or focus classes such as focus:outline-none removed and replaced
with a clear ring/outline like focus-visible:ring or focus:ring and
focus-visible:outline) within the same className string so keyboard users get a
visible focus state while preserving existing styles and the className prop
handling.
Comment on lines
+39
to
+42
| type DuoToneIconName = keyof typeof duoToneIcons; | ||
|
|
||
| const getIconName = (name: string): DuoToneIconName => | ||
| `d-${name}` as DuoToneIconName; |
Contributor
There was a problem hiding this comment.
Normalize and validate careIcon before casting to a duo‑tone key.
getIconName blindly prefixes and casts, so already-prefixed or unknown values can yield invalid icon keys. Normalize and verify against DuoTonePaths with a safe fallback.
✅ Safer icon resolution
-type DuoToneIconName = keyof typeof duoToneIcons;
-
-const getIconName = (name: string): DuoToneIconName =>
- `d-${name}` as DuoToneIconName;
+type DuoToneIconName = keyof typeof duoToneIcons;
+
+const getIconName = (name: string): DuoToneIconName | null => {
+ const normalized = name.startsWith("d-") ? name : `d-${name}`;
+ return normalized in duoToneIcons ? (normalized as DuoToneIconName) : null;
+};
@@
- icon={
- definition.healthcare_service.styling_metadata?.careIcon
- ? getIconName(
- definition.healthcare_service.styling_metadata
- .careIcon,
- )
- : "d-health-worker"
- }
+ icon={
+ definition.healthcare_service.styling_metadata?.careIcon
+ ? getIconName(
+ definition.healthcare_service.styling_metadata
+ .careIcon,
+ ) ?? "d-health-worker"
+ : "d-health-worker"
+ }Also applies to: 372-379
🤖 Prompt for AI Agents
In `@src/pages/Facility/settings/activityDefinition/ActivityDefinitionView.tsx`
around lines 39 - 42, getIconName currently just prefixes and force-casts, which
can produce invalid keys for already-prefixed or unknown values; update
getIconName to normalize the input (trim, toLowerCase, remove leading "d-"),
build the candidate key `d-${normalized}` and check it exists in duoToneIcons
(or DuoTonePaths) before casting, returning that key if valid or a safe fallback
key (e.g., a default duo-tone icon) otherwise; adjust any callers (e.g., where
careIcon is passed) to use the new getIconName for safe resolution.
🎭 Playwright Test ResultsStatus: ❌ Failed
📊 Detailed results are available in the playwright-final-report artifact. Run: #5116 |
bodhish
deleted the
issues/15298/activity-definition-icons-price-popover
branch
|
@rithviknishad 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! 🙌 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Enhances the Activity Definition View page with visual improvements:
Healthcare Service Icon Display: Shows the configured icon for linked healthcare services using the `styling_metadata.careIcon` field, with a fallback to the default `d-health-worker` icon.
Charge Item Definition Price Popover: Displays total price for linked charge item definitions with a clickable underlined trigger that opens a detailed breakdown popover showing:
Changes
Fixes #15298
Summary by CodeRabbit
New Features
✏️ Tip: You can customize this high-level summary in your review settings.