Open
Conversation
✅ Deploy Preview for care-ohc ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
ohc-project-manager
bot
added
needs testing
needs review
and removed
Work in Progress
labels
Contributor
WalkthroughReplaces legacy encounter-specific shortcut hooks with a centralized ShortcutContext-based system, removes old utility APIs, updates keyboardShortcuts.json to make many encounter shortcuts always-active, and updates encounter UI components to use ShortcutBadge and the new shortcut subcontext. Changes
Possibly related PRs
Suggested labels
Suggested reviewers
🚥 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)
📝 Coding Plan
Comment |
Deploying care-preview with
|
| Latest commit: |
a9da69a
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://eb9f6177.care-preview-a7w.pages.dev |
| Branch Preview URL: | https://manyu-fix--14285.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
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/Encounter/EncounterCommandDialog.tsx`:
- Around line 432-434: The dependency array in EncounterCommandDialog uses
inconsistent optional chaining for the required prop; replace the optional chain
usage of encounter?.status with the direct property access encounter.status to
match the prop contract. Update the dependency list where
encounter.encounter_class and encounter?.status are referenced so it reads
encounter.encounter_class and encounter.status, ensuring consistency with the
EncounterCommandDialog prop definition and avoiding unnecessary optional checks.
In `@src/config/keyboardShortcuts.json`:
- Around line 133-155: The handleAction function currently executes
shortcut-triggered actions without checking permissions; add early-return guards
in handleAction that verify canWriteSelectedEncounter (or the appropriate
permission flag) before performing any write/navigation/dialog
actions—specifically gate actions with action names add-allergy, add-diagnosis,
add-symptoms, add-service-request, add-medication-request, update-encounter and
dialog actions mark-as-completed, dispense, restart-encounter, assign-location,
manage-care-team, manage-departments; if the permission check fails, return
early (no-op) and optionally surface a user-visible permission error.
In `@src/pages/Encounters/tabs/overview/FormsDialog.tsx`:
- Around line 139-145: The trigger wrapper is a plain <div> with only onClick
which breaks keyboard accessibility; update the element in FormsDialog (the
wrapper that uses setOpen and renders {trigger}) to be keyboard-accessible by
adding role="button", tabIndex={0}, and an onKeyDown handler that listens for
Enter and Space and calls setOpen(true) (mirroring the onClick), ensuring you
preventDefault where appropriate and keep the existing onClick behavior; this
will provide proper keyboard activation and role semantics for screen readers.
🪄 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: fe557f21-519e-4244-a059-01591b6745d4
📒 Files selected for processing (8)
src/Utils/keyboardShortcutUtils.tssrc/components/Encounter/EncounterCommandDialog.tsxsrc/config/keyboardShortcuts.jsonsrc/hooks/useEncounterShortcuts.tssrc/pages/Encounters/EncounterShow.tsxsrc/pages/Encounters/tabs/overview/FormsDialog.tsxsrc/pages/Encounters/tabs/overview/clinical-history-overview.tsxsrc/pages/Encounters/tabs/overview/quick-actions.tsx
💤 Files with no reviewable changes (1)
- src/hooks/useEncounterShortcuts.ts
Contributor
There was a problem hiding this comment.
Pull request overview
Refactors Encounter-related keyboard shortcuts to use the new shortcut framework (ShortcutContext + data-shortcut-id click targets) instead of the legacy useEncounterShortcuts hook, and updates encounter UI surfaces to render the new shortcut badges.
Changes:
- Replaces legacy encounter shortcut hooks/badges with
ShortcutBadge+useShortcutSubContext("encounter"). - Removes
useEncounterShortcuts.tsand associated utilities, shifting shortcut execution to DOM click targets. - Updates
keyboardShortcuts.jsonencounter bindings and enables index-based tab shortcuts inNavTabs.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/pages/Encounters/tabs/overview/quick-actions.tsx | Migrates Quick Actions shortcut display from old display hook to ShortcutBadge/actionId. |
| src/pages/Encounters/tabs/overview/clinical-history-overview.tsx | Adds ShortcutBadge hint for Clinical History link. |
| src/pages/Encounters/tabs/overview/FormsDialog.tsx | Makes Forms dialog trigger discoverable via data-shortcut-id. |
| src/pages/Encounters/EncounterShow.tsx | Sets shortcut sub-context to encounter, updates Encounter Actions shortcut UI, enables index tab shortcuts. |
| src/hooks/useEncounterShortcuts.ts | Deletes legacy encounter shortcut hook implementation. |
| src/config/keyboardShortcuts.json | Adjusts encounter shortcut bindings/conditions and removes some legacy “go to …” bindings. |
| src/components/Encounter/EncounterCommandDialog.tsx | Switches shortcut display source to useShortcutDisplay and inlines action execution logic. |
| src/Utils/keyboardShortcutUtils.ts | Removes deprecated useShortcutDisplays and simplifies shortcut action click handling. |
🎭 Playwright Test ResultsStatus: ✅ Passed
📊 Detailed results are available in the playwright-final-report artifact. Run: #7445 |
abhimanyurajeesh
force-pushed
the
manyu/fix/#14285
branch
from
7eae035 to
d11e59d
Compare
abhimanyurajeesh
force-pushed
the
manyu/fix/#14285
branch
from
d11e59d to
c4a7117
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Refactors encounter-related keyboard shortcuts to use the newer ShortcutContext/data-shortcut-id click-based shortcut system, and updates encounter UI to display shortcut badges in key places.
Changes:
- Replaces the legacy encounter shortcut hook usage with
useShortcutSubContext("encounter")andShortcutBadgerendering. - Updates
keyboardShortcuts.jsonencounter shortcuts (new keys and “always” conditions) and removes several old “go to …” combos. - Removes legacy helper hooks (
useEncounterShortcuts,useShortcutDisplays) and simplifies shortcut click handling.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/pages/Encounters/tabs/overview/quick-actions.tsx | Switches quick actions to use actionId + ShortcutBadge instead of legacy display hook. |
| src/pages/Encounters/tabs/overview/clinical-history-overview.tsx | Adds a shortcut badge to the clinical history CTA. |
| src/pages/Encounters/tabs/overview/FormsDialog.tsx | Wires “add-questionnaire” via data-shortcut-id instead of custom DOM events. |
| src/pages/Encounters/EncounterShow.tsx | Sets shortcut sub-context to encounter, uses ShortcutBadge, enables tab index shortcuts. |
| src/hooks/useEncounterShortcuts.ts | Removes old encounter shortcut implementation. |
| src/config/keyboardShortcuts.json | Updates encounter shortcut definitions (keys + conditions), removes some older entries. |
| src/components/Scan/GenericQRScanDialog.tsx | Adjusts scanning reset behavior when dialog closes. |
| src/components/Patient/PatientInfoHoverCard.tsx | Adds a “Deceased” badge to the hover card header when applicable. |
| src/components/Encounter/EncounterCommandDialog.tsx | Migrates to useShortcutDisplay and inlines encounter action handling logic. |
| src/Utils/keyboardShortcutUtils.ts | Removes deprecated display hook and simplifies shortcut click handler to element.click(). |
| CLAUDE.md | Expands repository guidance/documentation for Claude Code. |
ohc-project-manager
bot
added
needs testing
needs review
and removed
Work in Progress
labels
Contributor
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/Encounter/EncounterCommandDialog.tsx (2)
102-109:⚠️ Potential issue | 🟡 MinorWrap
JSON.parsein try-catch to handle corrupted localStorage.If localStorage contains invalid JSON (from corruption, browser extensions, or manual tampering),
JSON.parsewill throw aSyntaxErrorand crash the component.🛡️ Proposed defensive fix
const getRecentActions = useCallback((): string[] => { const stored = localStorage.getItem(STORAGE_KEY); if (stored) { - const parsed = JSON.parse(stored); - return Array.isArray(parsed) ? parsed.slice(0, MAX_RECENT_ACTIONS) : []; + try { + const parsed = JSON.parse(stored); + return Array.isArray(parsed) ? parsed.slice(0, MAX_RECENT_ACTIONS) : []; + } catch { + localStorage.removeItem(STORAGE_KEY); + return []; + } } return []; }, []);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Encounter/EncounterCommandDialog.tsx` around lines 102 - 109, getRecentActions currently calls JSON.parse on localStorage.getItem(STORAGE_KEY) without error handling; wrap the JSON.parse call in a try/catch inside getRecentActions so any SyntaxError or other parse error returns an empty array (or a safe fallback) and avoids crashing the component, still respecting MAX_RECENT_ACTIONS when parsed value is an array.
288-291: 🧹 Nitpick | 🔵 TrivialInconsistent optional chaining on required
encounterprop.Line 288 uses
encounter.encounter_class(direct access) while line 289 usesencounter?.status(optional chaining). Sinceencounteris a required prop, the optional chaining is unnecessary and inconsistent.🔧 Proposed fix
{ id: "mark-as-completed", label: encounter.encounter_class === "imp" && - encounter?.status !== "discharged" + encounter.status !== "discharged" ? t("mark_for_discharge") : t("mark_as_completed"),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Encounter/EncounterCommandDialog.tsx` around lines 288 - 291, The code inconsistently uses optional chaining on the required prop `encounter`; update the conditional in EncounterCommandDialog so both properties are accessed consistently (e.g., replace `encounter?.status` with `encounter.status`) and ensure the prop type for `encounter` remains required so no runtime null checks are needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/config/keyboardShortcuts.json`:
- Around line 133-143: Add Playwright regression tests that cover the new
always-on encounter shortcuts defined in keyboardShortcuts.json: create specs
that (1) verify the shortcut triggers the expected behavior when the target
element with the corresponding data-shortcut-id (e.g., "add-allergy",
"add-service-request", "add-medication-request", "add-questionnaire",
"update-encounter", "clinical-history", "open-command-dialog",
"mark-as-completed", "dispense") is present in the DOM, and (2) verify the
shortcut is a no-op when that data-shortcut-id element is not rendered due to
permissions/state gating; ensure tests simulate the relevant keyboard combos
(including modifier combos like "shift+u", "cmd+e"/"ctrl+e") and assert side
effects or lack thereof, relying on DOM presence checks rather than internal
permission calls.
In `@src/pages/Encounters/tabs/overview/summary-panel-actions.tab.tsx`:
- Around line 60-65: Extract the inline object shape into a named interface
(e.g., SummaryPanelAction) with fields label: string, onClick: () => void,
hideOnMobile: boolean, and optional shortcut?: React.ReactNode; then replace the
inline "satisfies { ... }[]" with "satisfies SummaryPanelAction[]" (and add the
new interface declaration near the top of the module or next to related types).
Update any references to the anonymous shape to use SummaryPanelAction to
improve reuse and clarity (ensure the interface name matches the one used in the
satisfies expression).
---
Outside diff comments:
In `@src/components/Encounter/EncounterCommandDialog.tsx`:
- Around line 102-109: getRecentActions currently calls JSON.parse on
localStorage.getItem(STORAGE_KEY) without error handling; wrap the JSON.parse
call in a try/catch inside getRecentActions so any SyntaxError or other parse
error returns an empty array (or a safe fallback) and avoids crashing the
component, still respecting MAX_RECENT_ACTIONS when parsed value is an array.
- Around line 288-291: The code inconsistently uses optional chaining on the
required prop `encounter`; update the conditional in EncounterCommandDialog so
both properties are accessed consistently (e.g., replace `encounter?.status`
with `encounter.status`) and ensure the prop type for `encounter` remains
required so no runtime null checks are needed.
🪄 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: 776ae918-9890-4ddc-886e-a12fde1ff2aa
📒 Files selected for processing (7)
src/components/Encounter/EncounterCommandDialog.tsxsrc/config/keyboardShortcuts.jsonsrc/pages/Encounters/MarkEncounterAsCompletedDialog.tsxsrc/pages/Encounters/tabs/overview/FormsDialog.tsxsrc/pages/Encounters/tabs/overview/summary-panel-actions.tab.tsxsrc/pages/Encounters/tabs/overview/summary-panel-details-tab/encounter-details.tsxsrc/pages/Encounters/tabs/overview/summary-panel-details-tab/summary-panel-encounter-details.tsx
💤 Files with no reviewable changes (1)
- src/pages/Encounters/tabs/overview/FormsDialog.tsx
Greptile SummaryThis PR refactors the encounter keyboard shortcut system, replacing the dedicated Key points to note:
Confidence Score: 4/5
Important Files Changed
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
src/pages/Encounters/tabs/overview/summary-panel-details-tab/encounter-details.tsx
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Pull request overview
Refactors Encounter-page keyboard shortcuts to use the new ShortcutContext + ShortcutBadge mechanism (DOM data-shortcut-id click handlers) instead of the legacy useEncounterShortcuts hook, aiming for consistent shortcut badges across encounter actions.
Changes:
- Removes the legacy encounter shortcut hook (
useEncounterShortcuts) and wires Encounter pages into the new shortcut sub-context (useShortcutSubContext("encounter")). - Adds
ShortcutBadgeinstances across encounter overview UI (update encounter, clinical history, quick actions) and summary panel actions. - Updates
keyboardShortcuts.jsonfor theencountercontext with new key mappings and removes older multi-key/prefix mappings.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/pages/Encounters/EncounterShow.tsx | Switches Encounter to useShortcutSubContext("encounter"), updates command dialog trigger to use ShortcutBadge, enables tab index shortcuts. |
| src/components/Encounter/EncounterCommandDialog.tsx | Replaces legacy encounter shortcut hooks with useShortcutDisplay and local action execution helpers. |
| src/config/keyboardShortcuts.json | Updates encounter shortcut definitions (new keys + “always” conditions) and removes older ones. |
| src/pages/Encounters/tabs/overview/quick-actions.tsx | Migrates quick actions to ShortcutBadge/actionId instead of legacy display hook. |
| src/pages/Encounters/tabs/overview/summary-panel-actions.tab.tsx | Adds shortcut badge rendering for dispense in the actions list. |
| src/pages/Encounters/tabs/overview/clinical-history-overview.tsx | Adds shortcut badge to the “See clinical history” link. |
| src/pages/Encounters/tabs/overview/summary-panel-details-tab/* | Adds shortcut badge to the “Update encounter” action and hidden triggers for some actions. |
| src/pages/Encounters/tabs/overview/FormsDialog.tsx | Removes the legacy custom-event listener used by old shortcut plumbing. |
| src/pages/Encounters/MarkEncounterAsCompletedDialog.tsx | Adds a shortcut badge to the confirm action button. |
| src/Utils/keyboardShortcutUtils.ts | Removes deprecated shortcut display utilities and simplifies action handling to element.click(). |
|
@nihal467 this looks fine |
Contributor
Author
@parvathyns-creator can you please test all the shortcuts, across CARE ? |
@abhimanyurajeesh -The ESC (cancel) works only after entering values. If no values are entered, the shortcut isn't working. This can be seen in inventory transfers |
Contributor
Author
@parvathyns-creator if the focus is in an input field the shortcut wont work. |
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.
fixes #14285
Tagging: @ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit
New Features
Improvements
Behavior Changes