[ENG-570] adds support for extending encounter/patient files tab via plug / remove drawings support.#16469
Open
rithviknishad wants to merge 27 commits into
Conversation
✅ Deploy Preview for care-ohc ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughRemoves the hardcoded ChangesPlugin-based drawing system refactor
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Deploying care-preview with
|
| Latest commit: |
6bf5c28
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://227a1552.care-preview-a7w.pages.dev |
| Branch Preview URL: | https://eng-570-move-out-excalidraw.care-preview-a7w.pages.dev |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR replaces the built-in Excalidraw-based drawings implementation with a plug-driven “drawing applications” system backed by meta_artifacts, refactors the Files → Drawings UI to use the new mechanism, and removes the Excalidraw dependency.
Changes:
- Introduces plugin manifest support for
drawingApplicationsand new UI components (DrawingsSubTab,DrawingPreview,DrawingEditor) that render app-provided preview/editor implementations. - Refactors meta-artifact types/API response typing (
MetaArtifactRead, enums for associating/object types) to support generalized drawing payloads. - Removes Excalidraw editor/components and dependency, updates loading UI to a new SVG animation, and adjusts locale keys.
Reviewed changes
Copilot reviewed 21 out of 22 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| vite.config.mts | Removes a no-longer-needed process.env.IS_PREACT define. |
| tests/facility/patient/encounter/files/drawings/encounterDrawing.spec.ts | Deletes Excalidraw-specific E2E coverage. |
| src/types/metaArtifact/metaArtifactApi.ts | Updates API typing to use MetaArtifactRead consistently. |
| src/types/metaArtifact/metaArtifact.ts | Generalizes drawing value schema; adds enums and MetaArtifactRead. |
| src/Routers/routes/PatientRoutes.tsx | Removes patient drawing routes (needs replacement for new editor route). |
| src/Routers/routes/ConsultationRoutes.tsx | Routes encounter drawings to the new DrawingEditor. |
| src/pluginTypes.ts | Adds drawingApplications to plugin manifest; imports drawing prop types (should be type-only). |
| src/hooks/useCareApps.tsx | Makes withSuspense generic for broader plugin component wrapping. |
| src/components/Files/FilesTab.tsx | Switches drawings subtab to DrawingsSubTab (patient props currently incorrect). |
| src/components/Files/DrawingSubTab.tsx | Removes old Excalidraw-based drawings tab implementation. |
| src/components/Files/Drawings/* | Adds plugin-driven drawings list/preview/icon/editor + application lookup hooks. |
| src/components/Common/Loading.tsx | Replaces static logo with LoadingAnimationSvg (needs a11y status). |
| src/components/Common/Drawings/ExcalidrawEditor.tsx | Removes Excalidraw editor implementation. |
| src/components/careui/loading-animation-svg.tsx | Adds new SVG-based loading animation (hardcoded label). |
| public/locale/en.json | Reorders/removes some old drawing strings; adds new keys (saved, unsupported_drawing_application, etc.). |
| public/locale/ml.json | Removes some drawing-related strings (now missing drawings). |
| package.json | Removes @excalidraw/excalidraw dependency. |
Greptile SummaryThis PR removes the built-in Excalidraw drawing implementation from care_fe and replaces it with a generic plugin extension point (
Confidence Score: 5/5Safe to merge — the change is a clean extraction of Excalidraw into a plugin, with no remaining runtime dependencies on the deleted code. The deletions are complete and consistent: routes, types, API definitions, and tests all point to the old implementation and are all removed. The new plugin extension point is minimal and both current call sites use it correctly. No broken imports, no dangling references to removed types. src/hooks/useCareApps.tsx and src/pluginTypes.ts — the type-safety gap in Important Files Changed
Reviews (13): Last reviewed commit: "optimizations" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
rithviknishad
changed the title
![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: 11
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@public/locale/ml.json`:
- Line 2859: The Malayalam locale file is missing eight critical drawing-related
translation keys that exist in the English locale. Add the following missing
keys to the ml.json file with appropriate Malayalam translations: drawings,
saved, discard_changes, save_and_go_back, enter_drawing_name,
no_drawings_so_far, unsupported_drawing_application, and
you_have_unsaved_changes_what_would_you_like_to_do. Verify each key has a proper
Malayalam translation value to ensure the drawing UI displays correctly for
Malayalam users instead of falling back to English.
In `@src/components/careui/loading-animation-svg.tsx`:
- Around line 385-390: The hardcoded "loading" string in the
loading-animation-svg.tsx component bypasses i18next localization required for
multilingual healthcare interfaces. Replace the literal "loading" text with an
appropriate i18next translation call using the useTranslation hook. Import
useTranslation at the top of the component if not already present, call it
within the component function, and wrap the "loading" text with the translation
function (typically t("key_name")) to ensure the label respects the user's
language settings.
In `@src/components/Files/Drawings/DrawingEditor.tsx`:
- Around line 85-95: The DrawingEditorApplication component uses
usePluginDrawingApplication hook which can throw DrawingApplicationNotFoundError
when an application is not found, but currently has no error boundary to handle
this error. Wrap the DrawingEditorApplication component with an ErrorBoundary
component in the JSX return statement, following the same pattern and i18n
fallback that is already established in DrawingsSubTab.tsx and DrawingIcon.tsx
for handling the same DrawingApplicationNotFoundError exception.
In `@src/components/Files/Drawings/DrawingIcon.tsx`:
- Around line 15-26: The usePluginDrawingApplication hook is called at the top
level of the DrawingIcon component before the ErrorBoundary wrapper, so any
error thrown by the hook (such as DrawingApplicationNotFoundError) cannot be
caught by the ErrorBoundary. Extract the hook call and the ApplicationIcon
rendering into a separate child component, then render this new child component
inside the ErrorBoundary in DrawingIcon. This ensures the hook execution happens
within the ErrorBoundary's scope so it can properly catch and handle any errors
thrown by usePluginDrawingApplication.
In `@src/components/Files/Drawings/DrawingsSubTab.tsx`:
- Around line 253-257: The associatingId calculation in the DrawingsSubTab
component can result in an empty string when both the primary source
(encounter.id or patient.id) and the fallback are undefined, which causes
incorrect backend queries. Replace the empty string fallback ("") with patientId
to ensure there is always a valid identifier. Additionally, add validation
checks before any query or creation operations that use associatingId (including
the areas noted at lines 264-277 and 292-295) to ensure associatingId is not
empty before proceeding, preventing unpredictable backend behavior.
- Line 287: The onChange handler for the search input in DrawingsSubTab only
updates the search state with setSearch but does not reset the pagination page
state. When search text changes, the page offset becomes stale relative to the
new filtered results, which can cause empty states or false data displays.
Modify the onChange handler to also call the pagination reset function (such as
setPage with value 1 or 0) alongside setSearch to ensure the user returns to the
first page whenever the search query is modified.
- Around line 314-320: The Card component in DrawingsSubTab.tsx currently only
responds to click events via the onClick handler that calls navigate, making it
inaccessible via keyboard navigation. Add keyboard accessibility by including a
tabIndex prop to make the Card focusable, adding an appropriate ARIA role
attribute (such as button), and implementing an onKeyDown handler that detects
Enter and Space key presses to trigger the same navigate call as the onClick
handler. This ensures users can open drawings using keyboard navigation, meeting
WCAG 2.1 AA compliance requirements.
In `@src/components/Files/Drawings/usePluginDrawingApplications.ts`:
- Around line 13-18: The getDrawingApplicationsFromCareApps function does not
validate that drawing application keys are unique across plugins, allowing
duplicate keys to silently route to the first matched application. Add
validation logic after the flatMap operation to detect duplicate application
keys and throw an error when duplicates are found, ensuring each application key
is unique before returning the resolved applications. Apply the same validation
check to the other location mentioned around lines 31-33 where drawing
applications are similarly resolved.
In `@src/components/Files/FilesTab.tsx`:
- Around line 128-137: In the DrawingsSubTab component props, when the type is
FileType.PATIENT, you are currently passing patientId with the value
patient?.id. However, DrawingsSubTab expects the full patient object (not just
the id) to properly compute the associatingId from props.patient?.id. Change the
conditional props spread to pass the entire patient object instead of just the
patientId property when type === FileType.PATIENT, ensuring DrawingsSubTab
receives the complete patient data needed for its functionality.
In `@src/pluginTypes.ts`:
- Around line 1-2: The imports for DrawingEditorApplicationProps and
DrawingPreviewProps should use `import type` syntax since these interfaces are
only used in type positions (not at runtime). Change both import statements at
the top of the file to use `import type` instead of regular `import` to clarify
that these are type-only imports and align with TypeScript strict mode best
practices.
In `@src/types/metaArtifact/metaArtifact.ts`:
- Around line 7-10: Replace the enum MetaArtifactAssociatingType with an as
const object map containing the same key-value pairs (PATIENT and ENCOUNTER).
Then create a derived union type from the object's values to provide the same
type safety as the enum. This aligns with the codebase convention used in files
like consent.ts and allergyIntolerance.ts for defining string literal types.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 203e778d-e313-4d86-83a7-9ca30cf975da
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (21)
package.jsonpublic/locale/en.jsonpublic/locale/ml.jsonsrc/Routers/routes/ConsultationRoutes.tsxsrc/Routers/routes/PatientRoutes.tsxsrc/components/Common/Drawings/ExcalidrawEditor.tsxsrc/components/Common/Loading.tsxsrc/components/Files/DrawingSubTab.tsxsrc/components/Files/Drawings/DrawingEditor.tsxsrc/components/Files/Drawings/DrawingIcon.tsxsrc/components/Files/Drawings/DrawingPreview.tsxsrc/components/Files/Drawings/DrawingsSubTab.tsxsrc/components/Files/Drawings/usePluginDrawingApplications.tssrc/components/Files/FilesTab.tsxsrc/components/careui/loading-animation-svg.tsxsrc/hooks/useCareApps.tsxsrc/pluginTypes.tssrc/types/metaArtifact/metaArtifact.tssrc/types/metaArtifact/metaArtifactApi.tstests/facility/patient/encounter/files/drawings/encounterDrawing.spec.tsvite.config.mts
💤 Files with no reviewable changes (6)
- src/components/Common/Drawings/ExcalidrawEditor.tsx
- tests/facility/patient/encounter/files/drawings/encounterDrawing.spec.ts
- vite.config.mts
- src/components/Files/DrawingSubTab.tsx
- package.json
- src/Routers/routes/PatientRoutes.tsx
🎭 Playwright Test ResultsStatus: ✅ Passed
📊 Detailed results are available in the playwright-final-report artifact. Run: #9788 |
rithviknishad
force-pushed
the
ENG-570-move-out-excalidraw-to-be-a-pluggable-module-meta-artifacts-to-be-extensible-via-plugs
branch
from
b82e742 to
7c0615f
Compare
Comment on lines
+334
to
+352
| {data?.results.length === 0 ? ( | ||
| <div className="flex flex-col items-center justify-center p-8 text-gray-500"> | ||
| <ImageOffIcon className="mb-2 text-4xl" /> | ||
| <p className="text-lg font-medium">{t("no_drawings_so_far")}</p> | ||
| {canEdit && ( | ||
| <p className="text-sm"> | ||
| {hasApplications | ||
| ? t("create_new_drawing_message") | ||
| : t("no_drawing_applications_available")} | ||
| </p> | ||
| )} | ||
| </div> | ||
| ) : ( | ||
| <div className="ml-1 grid grid-cols-1 gap-4 md:grid-cols-2 xl:grid-cols-3"> | ||
| {data?.results.map((drawing) => ( | ||
| <DrawingCard key={drawing.id} drawing={drawing} /> | ||
| ))} | ||
| </div> | ||
| )} |
Comment on lines
+389
to
+396
| role="button" | ||
| tabIndex={0} | ||
| onClick={() => handleSelect()} | ||
| onKeyDown={(e) => { | ||
| if (e.key === "Enter") { | ||
| handleSelect(); | ||
| } | ||
| }} |
nihal467
requested changes
Jun 24, 2026
nihal467
left a comment
nihal467
left a comment
Member
There was a problem hiding this comment.
- the save and edit is keep on triggering unsaved changed browser pp-up
nihal467
approved these changes
Jun 24, 2026
| import { Suspense, createContext, useContext } from "react"; | ||
|
|
||
| import { PluginErrorBoundary } from "@/components/Common/PluginErrorBoundary"; | ||
| import { FilesTabsProps } from "@/components/Files/FilesTab"; |
| @@ -1,3 +1,4 @@ | |||
| import { FilesTabsProps } from "@/components/Files/FilesTab"; | |||
Comment on lines
2689
to
2693
| "2FA_backup_code": "2FA ബാക്കപ്പ് കോഡ്", | ||
| "ENCOUNTER_TAB__devices": "ഉപകരണങ്ങൾ", | ||
| "ENCOUNTER_TAB__drawings": "രേഖാചിത്രങ്ങൾ", | ||
| "SYSTEM__govt_org_type__district_panchayat": "ജില്ലാ പഞ്ചായത്ത്", | ||
| "add_another": "മറ്റൊന്ന് ചേർക്കുക", | ||
| "add_condition": "കണ്ടീഷനനുസരിച് ചേർക്കുക", |
Comment on lines
+47
to
+50
| const pluginTabs = useCareAppsEncounterFileTabs(); | ||
|
|
||
| const allowedTabs = ["all", "reports", ...Object.keys(pluginTabs)]; | ||
| const tabValue = allowedTabs.includes(qParams.file) ? qParams.file : "all"; |
rithviknishad
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 16 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (1)
src/components/Files/FilesTab.tsx:38
useQueryParams()is used without a generic type here, which effectively drops type-safety forqParams.fileandsetQParams(most other call sites in the repo specify a{...}generic, e.g.src/pages/Encounters/tabs/plots.tsxandsrc/components/Patient/PatientDetailsTab/EncounterHistory.tsx). Adding a minimal{ file?: string }type keeps this tab logic safe and self-documenting.
const [qParams, setQParams] = useQueryParams();
const { hasPermission } = usePermissions();
const { canWritePatient } = getPermissions(
hasPermission,
Comment on lines
+1
to
4
| import { FilesTabsProps } from "@/components/Files/FilesTab"; | ||
| import { NavigationLink } from "@/components/ui/sidebar/nav-main"; | ||
| import type { OverrideCondition } from "@/lib/override"; | ||
| import { PluginEncounterTabProps } from "@/pages/Encounters/EncounterShow"; |
Comment on lines
+76
to
95
| export const useCareAppTabs = <T extends object>(key: keyof PluginManifest) => { | ||
| const careApps = useCareApps(); | ||
|
|
||
| return careApps.reduce<Record<string, React.FC<PluginEncounterTabProps>>>( | ||
| (acc, app) => { | ||
| if (app.isLoading) { | ||
| return acc; | ||
| } | ||
|
|
||
| const appTabs = Object.entries(app.encounterTabs ?? {}).reduce( | ||
| (acc, [key, Component]) => { | ||
| return { ...acc, [key]: withSuspense(Component, app.plugin) }; | ||
| }, | ||
| {}, | ||
| ); | ||
|
|
||
| return { ...acc, ...appTabs }; | ||
| }, | ||
| {}, | ||
| ); | ||
| return careApps.reduce<Record<string, React.FC<T>>>((acc, app) => { | ||
| if (app.isLoading) { | ||
| return acc; | ||
| } | ||
|
|
||
| const tabs = app[key] as Record<string, React.ComponentType<T>> | undefined; | ||
| if (!tabs) { | ||
| return acc; | ||
| } | ||
|
|
||
| const appTabs = Object.entries(tabs).reduce((acc, [key, Component]) => { | ||
| return { ...acc, [key]: withSuspense(Component, app.plugin) }; | ||
| }, {}); | ||
|
|
||
| return { ...acc, ...appTabs }; | ||
| }, {}); | ||
| }; |
Comment on lines
2688
to
2694
| "yesterday": "ഇന്നലെ/കഴിഞ്ഞ ദിവസം", | ||
| "2FA_backup_code": "2FA ബാക്കപ്പ് കോഡ്", | ||
| "ENCOUNTER_TAB__devices": "ഉപകരണങ്ങൾ", | ||
| "ENCOUNTER_TAB__drawings": "രേഖാചിത്രങ്ങൾ", | ||
| "SYSTEM__govt_org_type__district_panchayat": "ജില്ലാ പഞ്ചായത്ത്", | ||
| "add_another": "മറ്റൊന്ന് ചേർക്കുക", | ||
| "add_condition": "കണ്ടീഷനനുസരിച് ചേർക്കുക", | ||
| "add_drawings": "രേഖാചിത്രങ്ങൾ ചേർക്കുക", | ||
| "add_option": "ഓപ്ഷൻ ചേർക്കുക", |
Comment on lines
+46
to
+48
| const pluginTabs = useCareAppTabs<FilesTabsProps>("encounterFileTabs"); | ||
| const allowedTabs = ["all", "reports", ...Object.keys(pluginTabs)]; | ||
| const tabValue = allowedTabs.includes(qParams.file) ? qParams.file : "all"; |
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
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.
Proposed Changes