Remove Manual keyboard-shortcut handle #9816
Conversation
WalkthroughThis pull request focuses on improving keyboard shortcut implementations and internationalization across multiple components. The changes introduce the Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
✅ Deploy Preview for care-ohc ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
Could you please let me know if any components are missing, @rithviknishad? |
|
@rithviknishad in some components it's not possible to implement due to the dynamic nature of keys. |
|
Tried this 😃 |
|
Yup, just checked the implementation of SearchByMultipleFields. Let's leave that out then. |
|
LGTM, add i18n for all texts in these files and should be good to merge |
…f github.com:JavidSumra/care_fe into issues/9641/cleanup-keyboard-shortcut-implementation
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/components/Patient/PatientIndex.tsx (1)
Line range hint
56-66: Consider extracting keyboard shortcut configuration.The keyboard shortcut implementation is good, but consider extracting the shortcut key "shift+p" into a constant for better maintainability.
+const CREATE_PATIENT_SHORTCUT = ["shift", "p"] as const; - useKeyboardShortcut(["shift", "p"], handleCreatePatient); + useKeyboardShortcut(CREATE_PATIENT_SHORTCUT, handleCreatePatient);src/components/Common/FilePreviewDialog.tsx (1)
156-160: Consider consolidating keyboard shortcuts into a single hook call.The current implementation uses two separate hook calls for left and right navigation. Consider consolidating them into a single hook call for better maintainability.
- useKeyboardShortcut(["ArrowLeft"], () => index > 0 && handleNext(index - 1)); - useKeyboardShortcut( - ["ArrowRight"], - () => index < (uploadedFiles?.length || 0) - 1 && handleNext(index + 1), - ); + useKeyboardShortcut({ + "ArrowLeft": () => index > 0 && handleNext(index - 1), + "ArrowRight": () => index < (uploadedFiles?.length || 0) - 1 && handleNext(index + 1) + });src/components/ui/sidebar.tsx (1)
103-106: Consider extracting keyboard shortcut configuration.The keyboard shortcut implementation is good, but consider extracting the platform-specific modifier key logic into a constant for better maintainability.
+const SIDEBAR_MODIFIER_KEY = isAppleDevice ? "Meta" : "Control"; +const SIDEBAR_TOGGLE_SHORTCUT = [SIDEBAR_MODIFIER_KEY, SIDEBAR_KEYBOARD_SHORTCUT] as const; - useKeyboardShortcut( - [isAppleDevice ? "Meta" : "Control", SIDEBAR_KEYBOARD_SHORTCUT], - toggleSidebar, - ); + useKeyboardShortcut(SIDEBAR_TOGGLE_SHORTCUT, toggleSidebar);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
public/locale/en.json(5 hunks)src/components/Common/FilePreviewDialog.tsx(3 hunks)src/components/Patient/PatientIndex.tsx(11 hunks)src/components/ui/sidebar.tsx(3 hunks)
🧰 Additional context used
📓 Learnings (1)
src/components/Common/FilePreviewDialog.tsx (2)
Learnt from: Jacobjeevan
PR: ohcnetwork/care_fe#9196
File: src/components/Common/FilePreviewDialog.tsx:193-200
Timestamp: 2024-11-26T09:46:12.574Z
Learning: When adding navigation buttons, include aria labels and handle arrow keys (ArrowLeft and ArrowRight) in onKeyDown events to enhance accessibility.
Learnt from: Jacobjeevan
PR: ohcnetwork/care_fe#9196
File: src/components/Common/FilePreviewDialog.tsx:193-200
Timestamp: 2024-11-26T09:48:54.707Z
Learning: In navigation buttons, handle only the 'ArrowLeft' and 'ArrowRight' keys in onKeyDown events; handling the 'Enter' key is not necessary.
🔇 Additional comments (7)
src/components/Patient/PatientIndex.tsx (2)
4-7: LGTM! Clean import organization.The imports are well organized, with related imports grouped together.
76-76: LGTM! Comprehensive internationalization implementation.All user-facing strings have been properly internationalized using the translation function. The translation keys are descriptive and follow a consistent naming pattern.
Also applies to: 95-95, 130-130, 158-158, 161-161, 185-185, 188-188, 199-199, 201-202, 240-240, 242-242, 248-248, 263-263, 265-265
src/components/Common/FilePreviewDialog.tsx (2)
14-14: LGTM! Clean import.The useKeyboardShortcut hook is properly imported.
194-194: LGTM! Proper internationalization.The "created_on" string has been properly internationalized.
src/components/ui/sidebar.tsx (1)
5-5: LGTM! Clean imports.The imports are well organized and properly placed.
Also applies to: 23-23
public/locale/en.json (2)
1258-1259: LGTM! Clear and user-friendly messages.The translation strings for patient record management are clear and provide good user guidance.
1358-1358: LGTM! Consistent patient verification messages.The translation strings for patient verification maintain consistency in terminology and provide clear instructions.
Also applies to: 1870-1870, 1888-1888
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
public/locale/en.json (1)
1363-1363: Verify consistent terminology usage across patient identity verification messages.The messages related to patient identity verification are clear but could be more consistent:
- "patient's year of birth" vs "year of birth"
- "verify" vs "verify patient identity"
Consider standardizing the terminology:
- "patient_birth_year_for_identity": "Please enter the patient's year of birth to verify their identity", + "patient_birth_year_for_identity": "Please enter year of birth to verify patient identity",Also applies to: 1876-1876, 1886-1886, 1894-1894
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
public/locale/en.json(5 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Test
- GitHub Check: CodeQL-Build
- GitHub Check: cypress-run (1)
- GitHub Check: OSSAR-Scan
🔇 Additional comments (3)
public/locale/en.json (3)
1263-1264: LGTM! Clear and actionable messages for patient record search.The messages are user-friendly and provide clear next steps when no records are found.
1629-1629: LGTM! Clear search page instruction.The message clearly explains both available actions (search and create) to the user.
Line range hint
1263-1894: Verify completeness of patient verification related translations.Let's ensure all related translation keys are present for a complete user experience.
✅ Verification successful
Patient verification translations are complete and comprehensive
The translations cover all necessary aspects of the patient verification flow including main actions, validation, error handling, and user guidance. The TODO comments found are unrelated to translations and are in configuration files.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for missing related translations in the patient verification flow # Common prefixes for patient verification echo "Checking for patient verification related keys..." rg -U 'patient.*identity|patient.*verify|verify.*patient|birth.*year' --type json # Check for any TODO/FIXME comments that might indicate missing translations echo "Checking for TODO/FIXME comments..." rg -U 'TODO|FIXME' --type jsonLength of output: 1335
|
LGTM |
|
@JavidSumra 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! 🙌 |
Proposed Changes
useKeyboardShortcuthook to handle key board events.@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit
Release Notes
Localization
User Interface
Keyboard Shortcuts
useKeyboardShortcut.Usability