Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
import { useTranslation } from "react-i18next";

import { MonetaryDisplay } from "@/components/ui/monetary-display";
import {
Popover,
PopoverContent,
PopoverTrigger,
} from "@/components/ui/popover";
import { Separator } from "@/components/ui/separator";

import ChargeItemPriceDisplay from "@/components/Billing/ChargeItem/ChargeItemPriceDisplay";

import { cn } from "@/lib/utils";
import { calculateTotalPrice } from "@/types/base/monetaryComponent/monetaryComponent";
import { ChargeItemDefinitionRead } from "@/types/billing/chargeItemDefinition/chargeItemDefinition";

interface ChargeItemDefinitionPopoverProps {
chargeItemDefinition: ChargeItemDefinitionRead;
className?: string;
}

export default function ChargeItemDefinitionPopover({
chargeItemDefinition,
className,
}: ChargeItemDefinitionPopoverProps) {
Comment on lines +17 to +25
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ 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.

const { t } = useTranslation();
const priceComponents = chargeItemDefinition.price_components;

const hasPriceComponents = priceComponents && priceComponents.length > 0;

if (!hasPriceComponents) {
return (
<span className={cn("text-sm text-gray-500", className)}>{t("na")}</span>
);
}

const totalPrice = calculateTotalPrice(priceComponents);

return (
<Popover>
<PopoverTrigger asChild>
<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")}
>
Comment on lines +42 to +48
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

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.

<MonetaryDisplay amount={totalPrice.toString()} />
</button>
</PopoverTrigger>
<PopoverContent
side="right"
align="start"
className="p-0 w-auto max-w-[calc(100vw-2rem)]"
>
<div className="p-3 space-y-2">
<div>
<p className="font-medium text-sm">{chargeItemDefinition.title}</p>
{chargeItemDefinition.description && (
<p className="text-xs text-gray-600 mt-1">
{chargeItemDefinition.description}
</p>
)}
</div>
{chargeItemDefinition.purpose && (
<div>
<p className="text-xs text-gray-500">{t("purpose")}</p>
<p className="text-xs text-gray-700">
{chargeItemDefinition.purpose}
</p>
</div>
)}
</div>
<Separator />
<ChargeItemPriceDisplay priceComponents={priceComponents} />
</PopoverContent>
</Popover>
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { useTranslation } from "react-i18next";
import { toast } from "sonner";

import CareIcon from "@/CAREUI/icons/CareIcon";
import duoToneIcons from "@/CAREUI/icons/DuoTonePaths.json";

import { Alert, AlertDescription, AlertTitle } from "@/components/ui/alert";
import { Badge } from "@/components/ui/badge";
Expand All @@ -18,6 +19,8 @@ import {
} from "@/components/ui/card";
import { Separator } from "@/components/ui/separator";

import ChargeItemDefinitionPopover from "@/components/Billing/ChargeItem/ChargeItemDefinitionPopover";

import BackButton from "@/components/Common/BackButton";
import ConfirmActionDialog from "@/components/Common/ConfirmActionDialog";
import Page from "@/components/Common/Page";
Expand All @@ -33,6 +36,11 @@ import {
import activityDefinitionApi from "@/types/emr/activityDefinition/activityDefinitionApi";
import { ArrowLeft } from "lucide-react";

type DuoToneIconName = keyof typeof duoToneIcons;

const getIconName = (name: string): DuoToneIconName =>
`d-${name}` as DuoToneIconName;
Comment on lines +39 to +42
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

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.


Comment on lines +41 to +43
Copy link

Copilot AI Jan 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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;
};

Copilot uses AI. Check for mistakes.
interface Props {
facilityId: string;
activityDefinitionSlug: string;
Expand Down Expand Up @@ -329,15 +337,17 @@ export default function ActivityDefinitionView({
className="rounded-lg border bg-gray-50/50 p-4 transition-colors hover:bg-gray-50"
>
<div className="space-y-2">
<p className="font-medium">{chargeItem.title}</p>
<p className="text-sm text-gray-600">
{chargeItem.description}
</p>
<Separator />
<div className="pt-2">
<p className="text-sm text-gray-500">{t("purpose")}</p>
<p className="text-gray-700">{chargeItem.purpose}</p>
<div className="flex items-center justify-between gap-4">
<p className="font-medium">{chargeItem.title}</p>
<ChargeItemDefinitionPopover
chargeItemDefinition={chargeItem}
/>
</div>
{chargeItem.description && (
<p className="text-sm text-gray-600">
{chargeItem.description}
</p>
)}
</div>
</div>
))}
Expand All @@ -356,15 +366,30 @@ export default function ActivityDefinitionView({
</CardHeader>
<CardContent>
<div className="rounded-lg border bg-gray-50/50 p-4 transition-colors hover:bg-gray-50">
<div className="space-y-2">
<p className="font-medium">
{definition.healthcare_service.name}
</p>
{definition.healthcare_service.extra_details && (
<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">
Copy link

Copilot AI Jan 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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">

Copilot uses AI. Check for mistakes.
<CareIcon
icon={
definition.healthcare_service.styling_metadata?.careIcon
? getIconName(
definition.healthcare_service.styling_metadata
.careIcon,
)
: "d-health-worker"
}
className="size-6"
/>
</div>
<div className="space-y-1">
<p className="font-medium">
{definition.healthcare_service.name}
</p>
)}
{definition.healthcare_service.extra_details && (
<p className="text-sm text-gray-600">
{definition.healthcare_service.extra_details}
</p>
)}
</div>
</div>
</div>
</CardContent>
Expand Down
Loading