GSoC - Doctor Notes Enhancement #8016
Conversation
|
@UdaySagar-Git is attempting to deploy a commit to the Open Healthcare Network Team on Vercel. A member of the Team first needs to authorize it. |
✅ Deploy Preview for care-egov-staging ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
Hi, This pr has been automatically marked as stale because it has not had any recent activity. It will be automatically closed if no further activity occurs for 7 more days. Thank you for your contributions. |
|
@rithviknishad can you please remove stale label |
|
👋 Hi, @UdaySagar-Git, This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there. |
|
👋 Hi, @UdaySagar-Git, This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there. |
|
[update] fixed, looks like it was issue with my local git config @rithviknishad @khavinshankar git isn't tracking file paths correctly , it is showing differntly in my local and in git 
my local path even changing the location of files not changing path in git |
UdaySagar-Git
force-pushed
the
gsoc-doctor-notes
branch
from
90a82d7
to
618a407
Compare
|
👋 Hi, @UdaySagar-Git, This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there. |
|
👋 Hi, @UdaySagar-Git, This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there. |
WalkthroughThis pull request introduces several enhancements across various components within the application. Key changes include the addition of a new notification event, Changes
Possibly related PRs
Suggested reviewers
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
src/components/Common/MarkdownPreview.tsxOops! Something went wrong! :( ESLint: 8.57.1 Error: Failed to load parser '@typescript-eslint/parser' declared in '.eslintrc.json': Cannot find module '@typescript-eslint/parser'
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 24
🧹 Outside diff range and nitpick comments (38)
src/common/hooks/useAuthUser.ts (1)
10-10: Consider adding JSDoc documentation for refetchUser.Since this method is critical for user profile updates and file upload features, adding documentation would help other developers understand its purpose and usage patterns.
+ /** Refreshes the current user data from the backend. + * Used after profile updates, file uploads, or when user data needs synchronization. + * @returns A promise containing the updated user data or error details + */ refetchUser: () => Promise<RequestResult<UserModel>>;src/components/Facility/DoctorNoteReplyPreviewCard.tsx (2)
23-51: Consider accessibility and structural improvements.While the layout works, there are opportunities for enhancement:
- Add ARIA labels for better screen reader support
- Consider extracting the user avatar into a separate component
- Enhance the tooltip implementation with proper ARIA attributes
Consider applying these improvements:
- <div className="mt-3 flex flex-col rounded-lg border border-gray-300 bg-gray-200/50 pt-2 text-gray-800"> + <div + className="mt-3 flex flex-col rounded-lg border border-gray-300 bg-gray-200/50 pt-2 text-gray-800" + role="article" + aria-label="Note reply preview" + > <div className="flex flex-col"> <div className="flex justify-between pl-3"> <div className="flex gap-2"> - <div className="mt-1 flex h-9 w-9 items-center justify-center rounded-full bg-primary text-lg font-semibold text-white"> + <div + className="mt-1 flex h-9 w-9 items-center justify-center rounded-full bg-primary text-lg font-semibold text-white" + role="img" + aria-label={`${parentNote.created_by_object?.first_name || "Unknown"}'s avatar`} + > {parentNote.created_by_object?.first_name[0]} </div>
64-65: Improve content truncation UX.The current implementation truncates content at a fixed height without indicating if content is hidden or providing a way to view more.
Consider adding an expand/collapse mechanism:
- <div className="max-h-14 overflow-hidden pb-2 pl-14 text-sm text-gray-700"> + <div className="pb-2 pl-14 text-sm text-gray-700"> + <div className={`${isExpanded ? '' : 'max-h-14 overflow-hidden'}`}> <MarkdownPreview markdown={parentNote.note} /> + </div> + {!isExpanded && contentHeight > 56 && ( + <button + onClick={() => setIsExpanded(true)} + className="mt-1 text-primary hover:text-primary-dark" + > + Show more + </button> + )} </div>Note: This would require adding state management for
isExpandedand measuring content height.src/components/Facility/PatientConsultationNotesList.tsx (2)
18-19: LGTM! Consider adding JSDoc comments.The new interface properties are well-typed. Consider adding JSDoc comments to document the purpose of these props and their expected usage.
+ /** View mode for the notes list. Defaults to "default-view" */ mode?: "thread-view" | "default-view"; + /** Callback to update the currently viewed thread note */ setThreadViewNote?: (noteId: string) => void;
83-86: Consider memoizing setThreadViewNote to prevent unnecessary re-renders.While the effect logic is correct, if
setThreadViewNoteis recreated frequently in the parent component, it could lead to unnecessary re-renders. Consider memoizing the callback in the parent component usinguseCallback.// In parent component: const setThreadViewNote = useCallback((noteId: string) => { // implementation }, [/* dependencies */]);src/components/Facility/DoctorNote.tsx (2)
13-14: LGTM! Consider adding JSDoc comments.The new props are well-typed and maintain backward compatibility. Consider adding JSDoc comments to document the purpose and usage of these props.
interface DoctorNoteProps { + /** Determines the display mode of notes. "thread-view" shows threaded discussions, "default-view" shows flat list */ mode?: "thread-view" | "default-view"; + /** Callback function to set the currently viewed thread note */ setThreadViewNote?: (noteId: string) => void;
30-30: Consider using dynamic height for better responsiveness.The fixed height of
500pxmight not work well across different screen sizes and devices. Consider using viewport-relative units or dynamic sizing.- className="mt-4 flex h-[500px] grow flex-col-reverse overflow-y-scroll bg-white sm:ml-2" + className="mt-4 flex h-[calc(100vh-200px)] min-h-[500px] grow flex-col-reverse overflow-y-scroll bg-white sm:ml-2"src/components/Common/MentionDropdown.tsx (2)
26-37: Consider scroll position in dropdown positioning.The current position calculation doesn't account for scroll position, which could lead to misalignment. Also, the selected index should be reset when the filter changes.
const [dropdownPosition, setDropdownPosition] = useState({ top: 0, left: 0 }); const [selectedIndex, setSelectedIndex] = useState<number | null>(null); useEffect(() => { if (editorRef.current) { + const scrollTop = window.pageYOffset || document.documentElement.scrollTop; + const scrollLeft = window.pageXOffset || document.documentElement.scrollLeft; setDropdownPosition({ - top: position.top, - left: position.left, + top: position.top + scrollTop, + left: position.left + scrollLeft, }); } }, [position, editorRef]); +useEffect(() => { + setSelectedIndex(null); +}, [filter]);
38-42: Enhance user filtering flexibility.The current filtering only matches username starts-with. Consider implementing a more flexible search that includes partial matches and possibly searching across other user fields (e.g., first_name, last_name).
const filteredUsers = useMemo(() => { - return users.filter((user) => - user.username.toLowerCase().startsWith(filter.toLowerCase()), - ); + const searchTerm = filter.toLowerCase(); + return users.filter((user) => { + const username = user.username.toLowerCase(); + const firstName = user.first_name.toLowerCase(); + const lastName = user.last_name?.toLowerCase() || ''; + return username.includes(searchTerm) || + firstName.includes(searchTerm) || + lastName.includes(searchTerm); + }); }, [users, filter]);src/components/Patient/DiscussionNotesFiles.tsx (4)
29-38: Review and clean up unused interface properties.The
StateInterfacecontains properties likezoom,rotation, and zoom-related flags that don't appear to be used in this component. Consider removing unused properties or documenting their purpose if they're needed for future implementation.
40-51: Enhance code maintainability with constants and utility functions.Consider the following improvements:
- Extract "UNARCHIVED" as a constant since it's used in multiple places.
- Move the offset calculation to a reusable utility function.
+const DEFAULT_TAB = "UNARCHIVED" as const; +const calculateOffset = (page: number, limit: number) => (page - 1) * limit; + export const DiscussionNotesFiles = (props: DiscussionNotesProps) => { const { t } = useTranslation(); const [currentPage, setCurrentPage] = useState(1); const [offset, setOffset] = useState(0); - const [tab, setTab] = useState("UNARCHIVED"); + const [tab, setTab] = useState(DEFAULT_TAB); const handlePagination = (page: number, limit: number) => { - const offset = (page - 1) * limit; + const offset = calculateOffset(page, limit); setCurrentPage(page); setOffset(offset); };
83-86: Use translations for tab labels.The tab labels are hardcoded in English. Use the translation function for internationalization support.
const tabs = [ - { text: "Active Files", value: "UNARCHIVED" }, - { text: "Archived Files", value: "ARCHIVED" }, + { text: t("active_files"), value: "UNARCHIVED" }, + { text: t("archived_files"), value: "ARCHIVED" }, ];
117-121: Simplify editable permission logic.The editable permission check could be simplified and moved to a separate function for better maintainability.
+const canEditFile = (file: FileUploadModel, user: AuthUser) => + file?.uploaded_by?.username === user.username || + ["DistrictAdmin", "StateAdmin"].includes(user.user_type); + <FileBlock file={item} key={item.id} fileManager={fileManager} associating_id={item.associating_id} - editable={ - item?.uploaded_by?.username === authUser.username || - authUser.user_type === "DistrictAdmin" || - authUser.user_type === "StateAdmin" - } + editable={canEditFile(item, authUser)} archivable={true} />src/Utils/useFileUpload.tsx (3)
Line range hint
142-186: Enhance error handling and state management for better UX.Based on reported issues in the PR:
- The error messages could be more user-friendly
- The state should be properly restored after failed uploads
- Consider adding retry logic for failed uploads
Consider this improvement:
const uploadfile = async (data: CreateFileResponse, file: File) => { const url = data.signed_url; const internal_name = data.internal_name; const newFile = new File([file], `${internal_name}`); - return new Promise<void>((resolve, reject) => { + return new Promise<void>(async (resolve, reject) => { + const maxRetries = 3; + let attempts = 0; + + while (attempts < maxRetries) { + try { uploadFile( url, newFile, "PUT", { "Content-Type": file.type }, (xhr: XMLHttpRequest) => { if (xhr.status >= 200 && xhr.status < 300) { setProgress(null); Notification.Success({ - msg: t("file_uploaded"), + msg: t("file_uploaded_success", { fileName: file.name }), }); setError(null); onUpload && onUpload(data); resolve(); } else { + attempts++; Notification.Error({ - msg: t("file_error__dynamic", { statusText: xhr.statusText }), + msg: t("file_error__dynamic", { + fileName: file.name, + statusText: xhr.statusText, + attempt: attempts, + maxRetries + }), }); setProgress(null); - reject(); + if (attempts === maxRetries) reject(); } }, setProgress as any, () => { + attempts++; Notification.Error({ - msg: t("file_error__network"), + msg: t("file_error__network", { + fileName: file.name, + attempt: attempts, + maxRetries + }), }); setProgress(null); - reject(); + if (attempts === maxRetries) reject(); }, ); + break; // Exit loop if upload succeeds + } catch (error) { + if (attempts === maxRetries) throw error; + } + // Add exponential backoff + await new Promise(r => setTimeout(r, Math.pow(2, attempts) * 1000)); + } }); };
Line range hint
108-141: Enhance security measures for medical file uploads.Given this handles medical data:
- The 100MB (
10e7) file size limit might be too permissive- Consider adding virus scanning integration
- Implement strict content-type verification
Consider these security enhancements:
const validateFileUpload = () => { if (files.length === 0) { setError(t("file_error__choose_file")); return false; } for (const file of files) { const filenameLength = file.name.trim().length; if (filenameLength === 0) { setError(t("file_error__file_name")); return false; } - if (file.size > 10e7) { + // Define size limits per file type + const maxSizes = { + 'image': 5 * 1024 * 1024, // 5MB for images + 'audio': 25 * 1024 * 1024, // 25MB for audio + 'document': 10 * 1024 * 1024 // 10MB for documents + }; + const maxSize = maxSizes[type] || 10 * 1024 * 1024; + if (file.size > maxSize) { setError(t("file_error__file_size")); return false; } + // Verify content type matches extension + const declaredType = file.type; + const extension = file.name.split(".").pop()?.toLowerCase(); + const validTypes = { + 'image': /^image\/(jpeg|png|gif|bmp|webp)$/, + 'audio': /^audio\/(mpeg|wav|ogg)$/, + 'document': /^application\/(pdf|msword|vnd.openxmlformats-officedocument.*)$/ + }; + if (!declaredType.match(validTypes[type])) { + setError(t("file_error__invalid_content_type")); + return false; + }
Line range hint
187-229: Improve user experience with better feedback and preview handling.Based on PR feedback:
- Add file preview capabilities
- Improve progress feedback
- Handle network interruptions gracefully
Consider adding a preview component:
type PreviewProps = { file: File; onRemove: () => void; progress: number | null; }; const FilePreview: React.FC<PreviewProps> = ({ file, onRemove, progress }) => { const isImage = ExtImage.includes(file.name.split('.').pop()?.toLowerCase() || ''); return ( <div className="file-preview"> {isImage && ( <img src={URL.createObjectURL(file)} alt={file.name} className="preview-thumbnail" /> )} <div className="preview-info"> <span>{file.name}</span> {progress !== null && ( <progress value={progress} max={100} /> )} </div> <button onClick={onRemove}>Remove</button> </div> ); };Then expose it through the hook:
return { progress, error, validateFiles: validateFileUpload, handleCameraCapture: () => setCameraModalOpen(true), handleAudioCapture: () => setAudioModalOpen(true), handleFileUpload: handleUpload, Dialogues, Input, fileNames: uploadFileNames, files: files, + FilePreview, // Add the preview component setFileNames: setUploadFileNames, setFileName: (name: string, index = 0) => {src/components/Common/FilePreviewDialog.tsx (2)
Line range hint
153-160: Enhance iframe security configuration.The current iframe implementation has security concerns:
- Empty sandbox attribute is too restrictive and might break legitimate previews.
- Direct URL usage without validation could pose security risks.
Consider these security enhancements:
<iframe - sandbox="" + sandbox="allow-scripts allow-same-origin" title="Source Files" - src={fileUrl} + src={validateFileUrl(fileUrl)} className="h-[75vh] w-full" />Add a URL validation helper:
const validateFileUrl = (url: string): string => { try { const validatedUrl = new URL(url); // Ensure URL is from trusted domains if (!validatedUrl.origin.endsWith('your-domain.com')) { throw new Error('Invalid file URL'); } return url; } catch { console.error('Invalid file URL'); return ''; } };
Line range hint
161-167: Enhance accessibility and user experience.Consider the following improvements for better accessibility and user experience:
- Add more descriptive error messages for unsupported file types.
- Improve screen reader support with ARIA labels.
- Add keyboard shortcuts for common actions.
Here's a suggested implementation:
<div className="flex h-full w-full flex-col items-center justify-center"> <CareIcon icon="l-file" - className="mb-4 text-5xl text-secondary-600" + className="mb-4 text-5xl text-secondary-600" + aria-hidden="true" /> - {t("file_preview_not_supported")} + <p role="alert"> + {t("file_preview_not_supported_description", { + fileType: file_state.extension.toUpperCase(), + supportedTypes: previewExtensions.join(', ').toUpperCase() + })} + </p> </div>Also consider adding keyboard shortcuts:
useEffect(() => { const handleKeyPress = (e: KeyboardEvent) => { if (e.key === '+' && e.ctrlKey) handleZoomIn(); if (e.key === '-' && e.ctrlKey) handleZoomOut(); if (e.key === 'r' && e.ctrlKey) handleRotate(90); }; window.addEventListener('keydown', handleKeyPress); return () => window.removeEventListener('keydown', handleKeyPress); }, []);src/components/Facility/PatientNotesSlideover.tsx (2)
12-16: Consider removing .js extensions from importsTypeScript/webpack typically handles module resolution without requiring explicit .js extensions. These extensions might cause issues with some bundlers and are generally unnecessary in TypeScript projects.
-import { PATIENT_NOTES_THREADS } from "@/common/constants.js"; -import DoctorNoteReplyPreviewCard from "./DoctorNoteReplyPreviewCard.js"; -import useNotificationSubscriptionState from "@/common/hooks/useNotificationSubscriptionState.js"; -import AuthorizedChild from "../../CAREUI/misc/AuthorizedChild.js"; +import { PATIENT_NOTES_THREADS } from "@/common/constants"; +import DoctorNoteReplyPreviewCard from "./DoctorNoteReplyPreviewCard"; +import useNotificationSubscriptionState from "@/common/hooks/useNotificationSubscriptionState"; +import AuthorizedChild from "../../CAREUI/misc/AuthorizedChild";
Line range hint
79-95: Consider enhancing error handlingThe function implementation looks good overall, but consider adding explicit error handling for failed requests.
const onAddNote = async () => { if (!/\S+/.test(noteField)) { Notification.Error({ msg: "Note Should Contain At Least 1 Character", }); return; } const { res, data } = await request(routes.addPatientNote, { pathParams: { patientId: patientId }, body: { note: noteField, consultation: consultationId, thread, reply_to: reply_to?.id, }, }); + if (!res?.ok) { + Notification.Error({ + msg: "Failed to add note. Please try again.", + }); + return; + } if (res?.status === 201) { Notification.Success({ msg: "Note added successfully" }); setNoteField(""); setState({ ...state, cPage: 1 }); setReload(true); setReplyTo(undefined); } return data?.id; };src/components/Notifications/NotificationsList.tsx (1)
76-78: Consider adding data validation for required properties.The URL construction looks good, but consider adding validation for required properties (facility, patient, consultation) to prevent undefined values in the URL.
case "PATIENT_NOTE_ADDED": + if (!data.facility || !data.patient || !data.consultation) { + console.warn('Missing required properties for PATIENT_NOTE_ADDED URL'); + return '#'; + } return `/facility/${data.facility}/patient/${data.patient}/consultation/${data.consultation}/notes`; case "PATIENT_NOTE_MENTIONED": + if (!data.facility || !data.patient || !data.consultation) { + console.warn('Missing required properties for PATIENT_NOTE_MENTIONED URL'); + return '#'; + } return `/facility/${data.facility}/patient/${data.patient}/consultation/${data.consultation}/notes`;src/components/Users/UserProfile.tsx (5)
Line range hint
13-14: Improve date validation logic and error message.The current date validation has two issues:
- The age requirement (17 years) should be configurable rather than hardcoded
- The error message doesn't clearly communicate the age requirement to users
Consider this improvement:
-if (!dayjs(states.form[field]).isValid() || - dayjs(states.form[field]).isAfter(dayjs().subtract(17, "year"))) { - errors[field] = "Enter a valid date of birth"; +const MIN_AGE_REQUIREMENT = 17; // Move to constants +if (!dayjs(states.form[field]).isValid() || + dayjs(states.form[field]).isAfter(dayjs().subtract(MIN_AGE_REQUIREMENT, "year"))) { + errors[field] = `You must be at least ${MIN_AGE_REQUIREMENT} years old`;
Line range hint
201-211: Extract password validation logic into a reusable function.The password validation logic is duplicated between the UI validation rules and the
validateNewPasswordfunction. This could lead to inconsistencies if validation rules change.Consider extracting the validation into a shared utility:
// passwordUtils.ts export const PASSWORD_RULES = { minLength: 8, requireNumbers: true, requireMixedCase: true }; export function validatePassword(password: string): { isValid: boolean; errors: string[] } { const errors: string[] = []; if (password.length < PASSWORD_RULES.minLength) { errors.push(`Password must be at least ${PASSWORD_RULES.minLength} characters`); } if (!/\d/.test(password)) { errors.push("Password must contain at least one number"); } if (password === password.toLowerCase() || password === password.toUpperCase()) { errors.push("Password must contain both uppercase and lowercase letters"); } return { isValid: errors.length === 0, errors }; }
Line range hint
321-368: Add loading state and prevent concurrent submissions.The form submission handler doesn't prevent multiple concurrent submissions or provide visual feedback during submission.
Consider these improvements:
+const [isSubmitting, setIsSubmitting] = useState(false); const handleSubmit = async (e: FormEvent) => { e.preventDefault(); + if (isSubmitting) return; + const validForm = validateForm(); if (validForm) { + setIsSubmitting(true); try { const data = { // ... existing data preparation }; const { res } = await request(routes.partialUpdateUser, { pathParams: { username: authUser.username }, body: data, }); if (res?.ok) { Notification.Success({ msg: "Details updated successfully", }); await refetchUserData(); setShowEdit(false); } + } catch (error) { + Notification.Error({ + msg: "Failed to update profile. Please try again.", + }); + } finally { + setIsSubmitting(false); + } } };Then update the submit button:
-<Submit onClick={handleSubmit} label={t("update")} disabled={!dirty} /> +<Submit + onClick={handleSubmit} + label={isSubmitting ? t("updating...") : t("update")} + disabled={!dirty || isSubmitting} +/>
Line range hint
481-507: Modernize file upload implementation.The current implementation uses XMLHttpRequest directly and could benefit from modern approaches and better error handling.
Consider using the Fetch API with proper error handling:
const handleAvatarUpload = async (file: File, onError: () => void) => { const formData = new FormData(); formData.append("profile_picture", file); try { const response = await fetch( `${careConfig.apiUrl}/api/v1/users/${authUser.username}/profile_picture/`, { method: 'POST', headers: { 'Authorization': `Bearer ${localStorage.getItem(LocalStorageKeys.accessToken)}` }, body: formData } ); if (!response.ok) { throw new Error(`Upload failed: ${response.statusText}`); } await sleep(1000); await refetchUser(); Notification.Success({ msg: "Profile picture updated." }); setEditAvatar(false); } catch (error) { console.error('Upload failed:', error); onError(); Notification.Error({ msg: "Failed to upload profile picture. Please try again." }); } };
Line range hint
590-650: Enhance form accessibility.The form lacks proper ARIA attributes and keyboard navigation support.
Consider these accessibility improvements:
<form + role="form" + aria-label="User Profile Form" action="#" method="POST" > <div className="shadow sm:rounded-md"> <div className="bg-white px-4 pt-5"> <div + role="group" + aria-labelledby="personal-info-heading" className="grid grid-cols-6 gap-4" > + <h2 + id="personal-info-heading" + className="sr-only" + > + Personal Information + </h2> {/* Existing form fields */} </div> </div> <div className="bg-secondary-50 px-4 py-3 text-right sm:px-6"> <Submit onClick={handleSubmit} label={t("update")} disabled={!dirty} + aria-label="Update profile information" /> </div> </div> </form>src/Redux/api.tsx (1)
1167-1173: Consider adding a create endpoint for consultation file uploads.While the list endpoint is well-implemented, consider adding a corresponding create endpoint to handle file uploads specifically for consultations. This would provide a more complete API surface for the consultation files feature.
Example addition:
// Consultation FileUploads listConsultationFileUploads: { path: "/api/v1/consultation/{consultation_external_id}/files/", method: "GET", TRes: Type<PaginatedResponse<FileUploadModel>>(), }, + createConsultationFileUpload: { + path: "/api/v1/consultation/{consultation_external_id}/files/", + method: "POST", + TBody: Type<CreateFileRequest>(), + TRes: Type<CreateFileResponse>(), + },src/components/Common/MarkdownPreview.tsx (2)
6-9: ExtendCustomLinkPropsto Include Standard Anchor AttributesCurrently,
CustomLinkPropsonly includesclassNameanddata-username. To ensure that all standard anchor tag properties are available in theCustomLinkcomponent, consider extendingCustomLinkPropsfromReact.ComponentProps<"a">.Apply this diff to update the interface:
-interface CustomLinkProps { - className?: string; - "data-username"?: string; -} +interface CustomLinkProps extends React.ComponentProps<"a"> { + "data-username"?: string; +}
38-45: Improve Username Matching in RegexThe current regex
/@(\w+)/gmay not match all valid usernames if they include characters like dots, dashes, or underscores. If usernames can contain such characters, consider updating the regex to accommodate them.For example:
-.replace(/@(\w+)/g, (_, username) => { +.replace(/@([a-zA-Z0-9._-]+)/g, (_, username) => {src/components/Facility/PatientNotesDetailedView.tsx (1)
112-119: Enhance accessibility by adding aria-labels to buttons.To improve accessibility, consider adding
aria-labelattributes to the close button for screen readers.Apply this diff:
<button onClick={() => setThreadViewNote?.("")} className="btn btn-default" + aria-label="Close detailed view" > <CareIcon icon="l-times" className="mr-1 text-lg" /> Close </button>src/components/Facility/ConsultationDoctorNotes/index.tsx (3)
115-132: Consider extracting theTabscomponent into a separate file.The
Tabscomponent is a reusable UI component that could be extracted into a separate file for better modularity and maintainability. This would also make it easier to reuse the component in other parts of the application.
Line range hint
133-193: Improve the layout and responsiveness of the component.The changes introduce a new layout structure using a combination of flexbox and conditional rendering based on the
threadViewNotestate. While this enhances the user interface, there are a few areas that could be improved:
- Consider using CSS grid instead of flexbox for a more responsive layout that adapts to different screen sizes.
- Ensure that the component is fully responsive and provides a good user experience on smaller screens, such as mobile devices.
- Use semantic HTML elements, such as
<section>or<article>, to improve the structure and accessibility of the component.
Based on the search results, I can now provide a final response regarding the error handling in the
onAddNotefunction.Error handling needs improvement in the
onAddNotefunction.The current implementation has basic validation for empty notes but lacks proper error handling for the API request. The function should:
- Handle network errors and failed requests
- Show appropriate error notifications to users
- Maintain consistent state in case of failures
Here's what's missing:
- No try-catch block around the
requestcall- No error handling for non-201 status codes
- No state cleanup in case of failures
🔗 Analysis chain
Line range hint
66-86: Verify error handling for theonAddNotefunction.The
onAddNotefunction now returns theidof the added note, which is a useful enhancement for further processing. However, it's important to ensure proper error handling in case the request fails or returns an unexpected status code.To verify the error handling, run the following script:
Let me try a different search pattern to locate the
onAddNotefunction implementation.
Let me check how the
requestfunction handles errors to complete the verification.
Let me try a different search pattern to find error handling patterns.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify error handling in the `onAddNote` function. # Test: Search for the `onAddNote` function. # Expect: The function should have a catch block or error handling logic. ast-grep --pattern $'onAddNote = async () => { $$$ }'Length of output: 484
Script:
#!/bin/bash # Search for the onAddNote function implementation ast-grep --pattern $'const onAddNote = async $_$ => { $$$ }' # Also search with ripgrep to ensure we don't miss any implementation rg "onAddNote\s*=\s*async" -A 20Length of output: 6492
Script:
#!/bin/bash # Search for the request function implementation and error handling rg "request\s*=.*=>" -A 10 # Also search for any try-catch blocks around request calls rg "try\s*{.*request" -A 10Length of output: 164
Script:
#!/bin/bash # Search for try-catch blocks with request rg "try.*\{" -A 15 --multiline # Search for the request function definition rg "export.*request" -A 10Length of output: 85309
src/components/Facility/PatientNoteCard.tsx (2)
205-205: Simplify function call with optional chainingYou can simplify the function call by using optional chaining.
Apply this diff to simplify the function call:
-setReplyTo && setReplyTo(note); +setReplyTo?.(note);🧰 Tools
🪛 Biome
[error] 205-205: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
173-173: Safely access the first character of the first nameTo prevent potential errors if
first_nameis undefined or an empty string, consider safely accessing the first character using optional chaining and a fallback value.Apply this diff to enhance safety:
-{note.created_by_object?.first_name[0]} +{note.created_by_object?.first_name?.[0] || ''}src/components/Common/RichTextEditor.tsx (3)
29-35: Consider renaming the 'onChange' prop for clarityThe prop
onChangeis being aliased tosetMarkdown, which may cause confusion sinceonChangeis commonly associated with event handlers. To improve clarity, consider either:
- Renaming the prop to
setMarkdownin the parent component, reflecting its purpose as a state setter.- Keeping the original prop name
onChangeand using it consistently throughout the component.This will enhance readability and maintain consistency in naming conventions.
402-467: Enhance accessibility by adding 'aria-label' attributes to toolbar buttonsTo improve accessibility for users utilizing assistive technologies:
- Add descriptive
aria-labelattributes to each toolbar button to convey their function.- Ensure the labels are clear and concise (e.g.,
aria-label="Bold"for the bold button).This will make the editor more inclusive and user-friendly.
205-293: Refactor list handling functions to reduce code duplicationThe functions
handleOrderedListandhandleUnorderedListcontain similar logic for manipulating list formatting. To improve maintainability:
- Abstract the common functionality into a helper function that accepts parameters to distinguish between ordered and unordered lists.
- Reduce repeated code, which simplifies future updates and decreases the potential for bugs.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (22)
src/Locale/en.json(1 hunks)src/Redux/api.tsx(3 hunks)src/Utils/useFileUpload.tsx(1 hunks)src/common/constants.tsx(1 hunks)src/common/hooks/useAuthUser.ts(1 hunks)src/components/Common/FilePreviewDialog.tsx(1 hunks)src/components/Common/MarkdownPreview.tsx(1 hunks)src/components/Common/MentionDropdown.tsx(1 hunks)src/components/Common/RichTextEditor.tsx(1 hunks)src/components/Facility/ConsultationDetails/index.tsx(2 hunks)src/components/Facility/ConsultationDoctorNotes/index.tsx(7 hunks)src/components/Facility/DoctorNote.tsx(2 hunks)src/components/Facility/DoctorNoteReplyPreviewCard.tsx(2 hunks)src/components/Facility/PatientConsultationNotesList.tsx(4 hunks)src/components/Facility/PatientNoteCard.tsx(5 hunks)src/components/Facility/PatientNotesDetailedView.tsx(1 hunks)src/components/Facility/PatientNotesSlideover.tsx(6 hunks)src/components/Facility/models.tsx(3 hunks)src/components/Notifications/NotificationsList.tsx(1 hunks)src/components/Patient/DiscussionNotesFiles.tsx(1 hunks)src/components/Patient/PatientNotes.tsx(5 hunks)src/components/Users/UserProfile.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/Locale/en.json
🧰 Additional context used
🪛 Biome
src/components/Facility/PatientNoteCard.tsx
[error] 205-205: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (36)
src/common/hooks/useAuthUser.ts (1)
10-10: LGTM! The refetchUser signature is well-typed.
The Promise<RequestResult> return type properly captures the asynchronous nature of user data refresh and aligns well with TypeScript best practices.
src/components/Facility/DoctorNoteReplyPreviewCard.tsx (2)
Line range hint 1-17: LGTM! Well-structured imports and interface definition.
The component's interface is clearly defined with proper TypeScript types, and the imports are well-organized.
19-21: LGTM! Good use of early return pattern.
The early return pattern improves code readability and provides a clean fallback to children when parentNote is undefined.
src/components/Facility/PatientConsultationNotesList.tsx (2)
33-34: LGTM!
The props destructuring is correct, with appropriate default values.
116-117: Verify DoctorNote component interface.
Please ensure that the DoctorNote component's interface has been updated to accept these new props.
✅ Verification successful
Props are properly typed and used in DoctorNote component
The verification shows that:
DoctorNotePropsinterface correctly includes both props:mode?: "thread-view" | "default-view"setThreadViewNote?: (noteId: string) => void
- The props are properly used in the component's logic for conditional rendering and event handling
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if DoctorNote component properly types these props
# Test: Search for DoctorNote interface/props type definition
ast-grep --pattern 'interface DoctorNoteProps {
$$$
}'
# Test: Search for mode and setThreadViewNote usage in DoctorNote
rg -A 5 'mode.*thread-view|setThreadViewNote' src/components/Facility/DoctorNote.tsx
Length of output: 1551
src/components/Facility/DoctorNote.tsx (3)
18-26: LGTM!
Clean and consistent props destructuring.
42-42: LGTM!
Appropriate styling for a chat-like interface with inverse scrolling.
Line range hint 1-91: Verify integration with all PR features.
While the component handles the reply feature well, please ensure it properly integrates with other features mentioned in the PR objectives:
- User profile images
- Markdown support
- File upload options
- User tagging mechanism
✅ Verification successful
All PR features are properly integrated through PatientNoteCard component
Based on the verification results, the DoctorNote component correctly integrates with all required features through its child component PatientNoteCard:
- Markdown support: Implemented via
<MarkdownPreview>component - File uploads: Handled through
filesprop and FilePreviewDialog - User tagging: Supported via
mentioned_usersprop in MarkdownPreview - Profile images: Basic avatar support shown through user initials in a circular div
- Reply feature: Implemented through
allowReplyandsetReplyToprops
The component architecture properly delegates these features to PatientNoteCard, which contains the necessary implementations for all required functionality.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify integration with other features
# Test 1: Check if PatientNoteCard component handles markdown and file previews
echo "Checking PatientNoteCard implementation..."
ast-grep --pattern 'interface PatientNoteCardProps {
$$$
}'
# Test 2: Check for user tagging related props
echo "Checking user tagging integration..."
rg -l "userMentions|taggedUsers|onMention" --type typescript
# Test 3: Check for file upload related components
echo "Checking file upload integration..."
rg -l "FileUpload|uploadFile|handleFileUpload" --type typescript
Length of output: 522
Script:
#!/bin/bash
# Let's try a different approach with correct file extensions and more specific patterns
# Check PatientNoteCard implementation
echo "Checking PatientNoteCard.tsx for feature support..."
rg -A 10 "interface PatientNoteCardProps" --type ts --type tsx
# Check for markdown-related imports and components
echo "Checking markdown support..."
rg -l "markdown|Markdown|ReactMarkdown" --type ts --type tsx
# Check for file upload and attachments
echo "Checking file attachments..."
rg -l "attachment|Attachment|upload" --type ts --type tsx
# Check for user mentions/tagging
echo "Checking user mentions..."
rg -l "@mention|mention|tagged" --type ts --type tsx
# Check for avatar/profile image
echo "Checking profile images..."
rg -l "avatar|Avatar|profile.*image" --type ts --type tsx
Length of output: 813
Script:
#!/bin/bash
# Let's try without specifying file types since ripgrep seems to have issues with tsx/ts types
# Check PatientNoteCard implementation
echo "Checking PatientNoteCard for feature support..."
rg -A 10 "interface PatientNoteCardProps"
# Check for markdown-related imports and components
echo "Checking markdown support..."
rg -l "markdown|Markdown|ReactMarkdown"
# Check for file upload and attachments
echo "Checking file attachments..."
rg -l "attachment|Attachment|upload"
# Check for user mentions/tagging
echo "Checking user mentions..."
rg -l "@mention|mention|tagged"
# Check for avatar/profile image
echo "Checking profile images..."
rg -l "avatar|Avatar|profile.*image"
# Also check the specific PatientNoteCard file content
echo "Checking PatientNoteCard content..."
fd -g "*PatientNoteCard.tsx" -x cat {}
Length of output: 16716
src/components/Common/MentionDropdown.tsx (2)
1-11: LGTM! Well-structured interface and imports.
The props interface is well-defined with clear types, and all imports are necessary for the component's functionality.
1-113: Verify integration with markdown editor.
Let's ensure this component is properly integrated with the markdown editor component.
✅ Verification successful
Integration with markdown editor is properly implemented
The MentionsDropdown component is correctly integrated with RichTextEditor:
- All required props are properly passed:
onSelect,position,editorRef, andfilter - Mentions functionality is properly handled with cursor positioning and text insertion
- Component is rendered conditionally based on
showMentionsstate - Keyboard navigation and event handling are implemented correctly
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify integration between MentionsDropdown and markdown editor
# Find markdown editor files that might use MentionsDropdown
echo "Searching for markdown editor integration..."
rg -l "MentionsDropdown" --type ts --type tsx
# Check for proper props passing
echo -e "\nChecking proper props usage..."
ast-grep --pattern 'import { $_ } from "$_/MentionsDropdown"
$$$
<MentionsDropdown
$$$
/>'
Length of output: 344
Script:
#!/bin/bash
# Let's try a different approach to find the integration
# Search for files containing MentionsDropdown with proper file extensions
echo "Searching for MentionsDropdown usage..."
rg -l "MentionsDropdown" --type-add 'tsx:*.tsx' --type-add 'ts:*.ts' --type tsx --type ts
# Look for markdown editor related files
echo -e "\nSearching for markdown editor files..."
rg -l "markdown|editor" --type-add 'tsx:*.tsx' --type-add 'ts:*.ts' --type tsx --type ts
# Get content of files that might show the integration
echo -e "\nChecking content of potential integration files..."
rg -A 5 -B 5 "MentionsDropdown" --type-add 'tsx:*.tsx' --type-add 'ts:*.ts' --type tsx --type ts
Length of output: 3903
Script:
#!/bin/bash
# Let's examine the RichTextEditor integration more closely
# Get the full content of RichTextEditor to verify proper integration
echo "Checking RichTextEditor implementation..."
cat src/components/Common/RichTextEditor.tsx
# Look for any related test files
echo -e "\nSearching for related test files..."
fd -e test.tsx -e test.ts -e spec.tsx -e spec.ts RichTextEditor
Length of output: 23959
src/components/Patient/DiscussionNotesFiles.tsx (1)
20-27: Consider making critical ModalDetails properties required.
The ModalDetails interface has all optional properties. If certain properties are always needed when using this interface (e.g., id, name), consider making them required by removing the ? optional modifier.
src/components/Patient/PatientNotes.tsx (4)
16-16: LGTM! Type safety and i18n improvements.
Good improvements:
- Proper typing for props instead of using
any - Integration of i18n support for better internationalization
Also applies to: 20-21
26-26: LGTM! Proper translation hook setup.
The translation hook is correctly initialized and integrated with the component.
Line range hint 1-166: Verify integration with other PR features.
This component should integrate with several features mentioned in the PR objectives:
- File upload options
- User tagging mechanism
- Profile image support
#!/bin/bash
# Check for file upload integration
echo "Checking for file upload components..."
ast-grep --pattern 'import $_ from $file' | rg 'upload|file'
# Check for user tagging integration
echo "Checking for user tagging components..."
ast-grep --pattern 'import $_ from $file' | rg 'tag|mention|user'
# Check for profile image integration
echo "Checking for profile image components..."
ast-grep --pattern 'import $_ from $file' | rg 'profile|avatar|image'112-113: Verify mobile responsiveness of the notes container.
The styling changes look good, but given the PR feedback about mobile view issues:
✅ Verification successful
Mobile responsiveness implementation looks appropriate
The component implements responsive design patterns correctly:
- Uses Tailwind's responsive prefixes (
sm:) to adjust margins and padding for different screen sizes - Has proper flex layout with
h-screenfor full viewport height - Container adapts with smaller margins and padding on mobile (
mx-3 my-2 p-2) and larger ones on desktop (sm:mx-10 sm:my-5 sm:p-5)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for responsive design patterns in the codebase
echo "Checking for responsive design patterns..."
rg -l 'sm:|md:|lg:|xl:' src/components/Patient/
# Search for mobile-specific styles or components
echo "Checking for mobile-specific implementations..."
rg -l 'mobile|responsive|screen' src/components/Patient/
Length of output: 469
Script:
#!/bin/bash
# Let's try a more specific search for responsive patterns in PatientNotes.tsx
echo "Checking PatientNotes.tsx for responsive patterns..."
rg 'sm:|md:|lg:|xl:' src/components/Patient/PatientNotes.tsx -A 2
# Check for mobile-specific styles in the component
echo "Checking mobile-specific styles in PatientNotes.tsx..."
rg 'mobile|screen' src/components/Patient/PatientNotes.tsx
# Look for viewport-related utilities
echo "Checking for viewport utilities..."
rg 'useWindowSize|useMediaQuery|window\.innerWidth' src/components/Patient/PatientNotes.tsx
Length of output: 860
src/Utils/useFileUpload.tsx (1)
Line range hint 64-74: Consider enhancing file type security measures.
While exporting the image extensions is useful for reusability, consider adding additional security measures for medical-related file uploads:
- Add MIME type validation in addition to extension checks
- Consider using a content-type detection library
- Document the security implications of allowing certain formats like SVG
Let's verify if MIME type validation is consistently applied:
src/components/Facility/PatientNotesSlideover.tsx (3)
29-29: LGTM! Good internationalization practice
The addition of useTranslation hook and its usage throughout the component ensures proper internationalization support.
180-182: LGTM! Appropriate width adjustment
The width increase to 430px provides more space for the enhanced features while maintaining a reasonable size for the slideover panel.
224-241: Verify accessibility of the rich text editor
The RichTextEditor integration looks good and properly handles authorization. However, we should verify its accessibility features.
✅ Verification successful
Rich text editor has adequate keyboard accessibility implementation
The RichTextEditor component implements sufficient keyboard accessibility features:
- Proper keyboard navigation with Enter key handling for lists and quotes
- Escape key support for closing mentions dropdown
- Full keyboard support for text formatting and link insertion
- ARIA-compatible toolbar buttons with tooltips
- Custom text area with maintained focus management
The implementation follows accessibility best practices by:
- Preserving focus states after operations
- Providing clear visual feedback through tooltips
- Maintaining proper keyboard navigation flow
- Including descriptive labels and IDs for form controls
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for aria attributes and role definitions in the RichTextEditor component
rg -A 5 'aria-|role=' src/components/Common/RichTextEditor.tsx
# Search for keyboard event handlers that might affect accessibility
ast-grep --pattern 'onKeyDown|onKeyPress|onKeyUp' src/components/Common/RichTextEditor.tsx
Length of output: 157
Script:
#!/bin/bash
# First, let's verify if the RichTextEditor file exists and check its contents
fd RichTextEditor.tsx
# If it exists in a different location, let's search the entire codebase for rich text editor accessibility patterns
rg -i "aria-|role=|keyboard|a11y" -g "**/*RichTextEditor*"
# Let's also check if we're using any known rich text editor libraries that might handle accessibility
rg -l "@tiptap|slate-react|draft-js|react-quill" package.json
# Search for any accessibility-related tests
rg -l "accessibility|a11y" -g "*test*" -g "*spec*"
Length of output: 444
Script:
#!/bin/bash
# Let's examine the keyboard event handlers in RichTextEditor.tsx
rg -A 10 "handleEscape|onKeyDown" src/components/Common/RichTextEditor.tsx
# Check if we're using any markdown editor component that might handle accessibility
cat src/components/Common/RichTextEditor.tsx
# Look for any editor-related components that might be imported
rg -l "import.*Editor" src/components/Common/RichTextEditor.tsx
Length of output: 25341
src/components/Facility/ConsultationDetails/index.tsx (3)
39-39: LGTM!
The import statement follows the project's import organization pattern and uses appropriate relative path notation.
60-60: LGTM! Clean integration with existing tab system.
The new tab is well-integrated into the existing system, leveraging the flexible tab architecture without requiring changes to the core navigation logic.
60-60: Verify translation key for the new tab.
Ensure that the translation key CONSULTATION_TAB__DISCUSSION_NOTES_FILES is defined in the localization files.
✅ Verification successful
Translation key is properly defined
The translation key CONSULTATION_TAB__DISCUSSION_NOTES_FILES is correctly defined in the localization files, supporting the new tab implementation.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if translation key exists in localization files
# Search for the translation key in localization files
rg "CONSULTATION_TAB__DISCUSSION_NOTES_FILES" -g "*.json"
Length of output: 408
Script:
#!/bin/bash
# Check for translation keys in all locale files
fd -e json -d 2 . src/Locale --exec cat {} \; | jq -r 'select(.CONSULTATION_TAB__DISCUSSION_NOTES_FILES != null) | keys[] | select(. == "CONSULTATION_TAB__DISCUSSION_NOTES_FILES")'
Length of output: 226
src/components/Notifications/NotificationsList.tsx (2)
76-78: LGTM: Seamless integration with existing notification system.
The new notification types are well-integrated with the existing notification system. They will automatically work with all features including:
- Read/unread status management
- Notification filtering
- Push notifications
77-78: Verify if PATIENT_NOTE_MENTIONED requires specific note targeting.
Both PATIENT_NOTE_ADDED and PATIENT_NOTE_MENTIONED use identical URLs. Verify if the mention notification should navigate to the specific note where the user was mentioned.
✅ Verification successful
Based on the search results, I can now provide a final response:
PATIENT_NOTE_MENTIONED should navigate to the general notes section
The current implementation is correct. Here's why:
- The consultation notes are displayed in a single view at
/facility/${data.facility}/patient/${data.patient}/consultation/${data.consultation}/notes - There is no specific note targeting in the URL structure for individual notes
- The notification is meant to direct users to the consultation notes section where they can view all notes, including mentions
- The same URL pattern is used for both PATIENT_NOTE_ADDED and PATIENT_NOTE_MENTIONED, which is appropriate since they both lead to the same notes view
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if there's any specific handling for note mentions in the codebase
# Look for any specific handling of note mentions in components or routes
rg -A 5 "PATIENT_NOTE_MENTIONED" --type ts --type tsx
# Check for any note-specific URL parameters or routes
rg -A 5 "consultation/.*/notes/.*" --type ts --type tsx
Length of output: 167
Script:
#!/bin/bash
# Search for PATIENT_NOTE_MENTIONED without file type restrictions
rg -A 5 "PATIENT_NOTE_MENTIONED"
# Search for note-specific URL patterns without file type restrictions
rg -A 5 "consultation.*notes"
# Search for any note-specific routing or navigation logic
rg -A 5 "note.*id|noteId"
Length of output: 31678
src/components/Facility/models.tsx (3)
570-570: LGTM! Property addition aligns with profile image feature.
The new read_profile_picture_url property follows existing naming conventions and correctly supports the user profile images feature.
586-586: LGTM! Property addition supports reply feature.
The optional reply_to property correctly supports the hierarchical reply structure for doctor notes.
583-583:
Fix typo in interface name: "PaitentNotesReplyModel".
Rename the interface from "PaitentNotesReplyModel" to "PatientNotesReplyModel" to maintain consistency and correctness.
-export interface PaitentNotesReplyModel {
+export interface PatientNotesReplyModel {Likely invalid or redundant comment.
src/Redux/api.tsx (2)
824-828: LGTM: Well-structured endpoint for retrieving patient notes.
The endpoint follows RESTful conventions and is properly typed with PatientNotesModel as the response type.
Line range hint 1143-1166: LGTM: Comprehensive file upload API endpoints.
The endpoints provide complete CRUD operations with proper typing and follow RESTful conventions. This implementation supports the PR objectives for file upload functionality.
src/common/constants.tsx (1)
672-676: LGTM! The new notification event is well-structured.
The addition of the PATIENT_NOTE_MENTIONED notification event follows the existing pattern and uses appropriate values for the notification type.
src/components/Facility/ConsultationDoctorNotes/index.tsx (4)
44-47: Ensure type safety for the mode state variable.
The mode state variable is correctly typed as a union of string literals, ensuring type safety. This is a good practice to prevent unexpected values and catch potential bugs at compile time.
173-180: Verify the integration of the RichTextEditor component.
The RichTextEditor component has replaced the previous text input field, providing a more robust editing experience for notes. However, it's important to verify that the component is properly integrated and functions as expected.
To verify the integration of the RichTextEditor component, run the following script:
#!/bin/bash
# Description: Verify the integration of the `RichTextEditor` component.
# Test: Search for the usage of the `RichTextEditor` component.
# Expect: The component should be used consistently across the codebase.
rg --type tsx? --type jsx? $'RichTextEditor'184-193: Ensure proper error handling and loading states for the PatientNotesDetailedView component.
The PatientNotesDetailedView component is conditionally rendered based on the threadViewNote state. While this allows for detailed viewing of specific notes, it's important to ensure that the component handles error states and loading states gracefully.
To verify the error handling and loading states, run the following script:
#!/bin/bash
# Description: Verify error handling and loading states in the `PatientNotesDetailedView` component.
# Test: Search for the `PatientNotesDetailedView` component.
# Expect: The component should have error handling and loading states.
ast-grep --pattern $'PatientNotesDetailedView'2-18: Ensure consistent import paths and extensions.
The import statements have been updated to include .js extensions for various modules. While this improves clarity regarding file types, it's important to maintain consistency across the codebase. Verify that the same convention is followed in other files and consider updating the project's coding guidelines to reflect this change.
To verify the consistency of import paths and extensions across the codebase, run the following script:
src/components/Common/RichTextEditor.tsx (2)
159-203: Verify 'Enter' key handling allows normal line breaks
The onKeyDown handler prevents the default action of the Enter key to manage list and quote formatting. This might unintentionally prevent users from inserting line breaks in normal text. Ensure that:
- Users can insert new lines when not typing within a list or quote.
- The handler only prevents the default action when necessary for formatting purposes.
611-611:
Fix incorrect method for clearing textarea content
Using editorRef.current.innerHTML = "" will not clear the content of a <textarea> element since it does not have an innerHTML property that affects user input. Instead, you should set the value property to an empty string to clear the content:
Apply this diff to correct the issue:
- editorRef.current.innerHTML = "";
+ editorRef.current.value = "";Likely invalid or redundant comment.
| setIsLoading(true); | ||
|
|
||
| const { data } = await request(routes.getPatientNote, { | ||
| pathParams: { | ||
| patientId: patientId, | ||
| noteId, | ||
| }, | ||
| query: { | ||
| consultation: consultationId, | ||
| thread, | ||
| }, | ||
| }); | ||
|
|
||
| if (data) { | ||
| setState(data); | ||
| } | ||
| setIsLoading(false); | ||
| setReload?.(false); | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Implement error handling in 'fetchNotes' function.
Currently, if the API request fails, the component does not handle the error or provide feedback to the user. Adding error handling will improve user experience and make debugging easier.
Apply this diff to handle errors:
const fetchNotes = async () => {
setIsLoading(true);
- const { data } = await request(routes.getPatientNote, {
+ const { res, data } = await request(routes.getPatientNote, {
pathParams: {
patientId: patientId,
noteId,
},
query: {
consultation: consultationId,
thread,
},
});
+ if (res?.ok && data) {
setState(data);
- }
+ } else {
+ Notification.Error({ msg: "Failed to fetch notes." });
+ }
setIsLoading(false);
- setReload?.(false);
+ setReload(false);
};📝 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.
| setIsLoading(true); | |
| const { data } = await request(routes.getPatientNote, { | |
| pathParams: { | |
| patientId: patientId, | |
| noteId, | |
| }, | |
| query: { | |
| consultation: consultationId, | |
| thread, | |
| }, | |
| }); | |
| if (data) { | |
| setState(data); | |
| } | |
| setIsLoading(false); | |
| setReload?.(false); | |
| }; | |
| setIsLoading(true); | |
| const { res, data } = await request(routes.getPatientNote, { | |
| pathParams: { | |
| patientId: patientId, | |
| noteId, | |
| }, | |
| query: { | |
| consultation: consultationId, | |
| thread, | |
| }, | |
| }); | |
| if (res?.ok && data) { | |
| setState(data); | |
| } else { | |
| Notification.Error({ msg: "Failed to fetch notes." }); | |
| } | |
| setIsLoading(false); | |
| setReload(false); |
| setDownloadURL(URL.createObjectURL(blob)); | ||
| }); | ||
| }; |
There was a problem hiding this comment.
Revoke object URLs to prevent memory leaks
When using URL.createObjectURL, it's important to revoke the object URL with URL.revokeObjectURL when it's no longer needed to prevent memory leaks.
Apply these diffs to revoke the object URL:
In the downloadFileUrl function:
-fetch(url)
.then((res) => res.blob())
.then((blob) => {
- setDownloadURL(URL.createObjectURL(blob));
+ const objectURL = URL.createObjectURL(blob);
+ setDownloadURL(objectURL);
});In the handleClose function:
const handleClose = () => {
+ if (downloadURL) {
+ URL.revokeObjectURL(downloadURL);
+ }
setDownloadURL("");
setFileState({
...file_state,
open: false,
zoom: 4,
isZoomInDisabled: false,
isZoomOutDisabled: false,
});
};Also applies to: 107-115
| if (allowThreadView && setThreadViewNote) | ||
| setThreadViewNote(note.id); | ||
| }} | ||
| className={`pl-11 text-sm text-gray-700 ${allowThreadView && "cursor-pointer"}`} |
There was a problem hiding this comment.
Avoid including 'false' in className when condition is false
Using logical && in template literals may include the string 'false' in the className when the condition is false, leading to unintended class names. It's better to use a ternary operator to ensure only the desired class is included.
Apply this diff to correct the className:
-className={`pl-11 text-sm text-gray-700 ${allowThreadView && "cursor-pointer"}`}
+className={`pl-11 text-sm text-gray-700 ${allowThreadView ? "cursor-pointer" : ""}`}📝 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.
| className={`pl-11 text-sm text-gray-700 ${allowThreadView && "cursor-pointer"}`} | |
| className={`pl-11 text-sm text-gray-700 ${allowThreadView ? "cursor-pointer" : ""}`} |
| onChange={(e) => { | ||
| setMarkdown(e.target.value); | ||
| }} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid redundant state updates in the 'onChange' handler
The onChange event updates setMarkdown directly, and handleInput (attached to onInput) also updates setMarkdown. This can lead to redundant state updates. To optimize performance and prevent potential side effects, consider:
- Removing the
onChangehandler ifhandleInputsufficiently handles all input changes. - Ensuring that
setMarkdownis called only once per input event.
| {isPreviewMode ? ( | ||
| <div className="max-h-[400px] min-h-[70px] overflow-y-auto p-4"> | ||
| <MarkdownPreview markdown={markdown} /> | ||
| </div> | ||
| ) : ( |
There was a problem hiding this comment.
Ensure Markdown rendering is secure against XSS attacks
When rendering user-generated Markdown content in MarkdownPreview, it's crucial to prevent Cross-Site Scripting (XSS) vulnerabilities. Please verify that:
- The Markdown rendering library sanitizes input effectively.
- Any HTML content within the Markdown is properly escaped or filtered.
- Additional security measures are in place if the library does not handle sanitization adequately.
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Outside diff range and nitpick comments (10)
src/components/Facility/ConsultationDetails/DiscussionNotesFilesTab.tsx (4)
31-40: Consider refactoring StateInterface to match actual usage.The
StateInterfaceincludes properties for image manipulation (zoom, rotation) that don't appear to be used in this component. Consider moving these properties to a separate interface if they're meant for a different component, or remove them if unused.
42-54: Consider improving state management and type safety.A few suggestions to enhance the code:
- Consider consolidating pagination state into a single object.
- Use an enum or const object for tab values to prevent typos.
Example implementation:
type TabType = 'UNARCHIVED' | 'ARCHIVED'; interface PaginationState { currentPage: number; offset: number; } const [paginationState, setPaginationState] = useState<PaginationState>({ currentPage: 1, offset: 0 }); const [tab, setTab] = useState<TabType>('UNARCHIVED');
119-124: Extract permission check logic to a separate function.The permission check logic for file editability should be extracted to improve readability and reusability.
Example implementation:
const canEditFile = (uploadedByUsername: string) => { return ( uploadedByUsername === authUser.username || authUser.user_type === "DistrictAdmin" || authUser.user_type === "StateAdmin" ); }; // Usage in JSX editable={canEditFile(item?.uploaded_by?.username)}
96-147: Consider adding accessibility improvements.The component could benefit from improved accessibility:
- Add ARIA labels to interactive elements
- Ensure proper heading hierarchy
- Add keyboard navigation support for the file list
Example improvements:
-<div className={`md:p-4 ${props.className}`}> +<div className={`md:p-4 ${props.className}`} role="region" aria-label={t('discussion_notes_files')}> -<h3>Discussion Notes Files</h3> +<h2>Discussion Notes Files</h2>src/components/Common/RichTextEditor.tsx (2)
14-20: Add JSDoc documentation for the component props interface.Adding detailed documentation for each prop would improve maintainability and help other developers understand the component's API.
+/** + * Props for the RichTextEditor component + * @interface RichTextEditorProps + * @property {string} [initialMarkdown] - Initial markdown content + * @property {(markdown: string) => void} onChange - Callback when markdown content changes + * @property {() => Promise<string | undefined>} onAddNote - Callback to handle note addition + * @property {boolean} [isAuthorized] - Whether the user is authorized to edit + * @property {() => void} [onRefetch] - Callback to refetch data after changes + */ interface RichTextEditorProps { initialMarkdown?: string; onChange: (markdown: string) => void; onAddNote: () => Promise<string | undefined>; isAuthorized?: boolean; onRefetch?: () => void; }
33-39: Optimize component performance with React.memo and useCallback.The component handles complex state and renders many elements. Consider:
- Wrapping the component with React.memo
- Memoizing callback props
- Breaking down into smaller, memoized components
+const MemoizedToolbar = React.memo(({ onBold, onItalic, ... }) => { + return ( + <div className="flex items-center..."> + {/* toolbar buttons */} + </div> + ); +}); -const RichTextEditor: React.FC<RichTextEditorProps> = ({ +const RichTextEditor = React.memo<RichTextEditorProps>(({ initialMarkdown: markdown = "", onChange: setMarkdown, onAddNote, isAuthorized = true, onRefetch, -}) => { +}) => {src/components/Facility/ConsultationDoctorNotes/index.tsx (4)
49-52: Enhance Type Safety by Defining a Shared Type formodeThe
modestate is defined with a union type and initialized to"default-view". However, when updatingmodein theonTabChangehandler, you are using a type assertion (as). To improve type safety and avoid unnecessary assertions, consider defining a shared type formode.Apply the following changes:
+ type ViewMode = "thread-view" | "default-view"; - const [mode, setMode] = useState<"thread-view" | "default-view">( + const [mode, setMode] = useState<ViewMode>( "default-view", ); ... - onTabChange={(tab) => setMode(tab as "thread-view" | "default-view")} + onTabChange={(tab: ViewMode) => setMode(tab)}This ensures that
tabis correctly typed throughout your component, enhancing maintainability and reducing potential runtime errors.
Line range hint
71-91: Handle Errors When Adding a NoteIn the
onAddNotefunction, there is no error handling for unsuccessful API calls. If the request fails, the user won't receive any feedback, anddata?.idmight beundefined, which could lead to issues downstream.Consider adding error handling to notify the user of failures:
if (res?.status === 201) { Notification.Success({ msg: "Note added successfully" }); setState({ ...state, cPage: 1 }); setNoteField(""); setReload(true); setReplyTo(undefined); + return data?.id; } else { + Notification.Error({ msg: "Failed to add note. Please try again." }); + return null; }This provides feedback to the user and ensures that your function handles both success and failure cases appropriately.
33-33: Internationalize All User-Facing StringsWhile you've introduced
useTranslation, some user-facing strings are still hardcoded. Internationalizing all strings ensures consistency and allows for localization.For instance, update the
Pagetitle and tab texts:// At line 120~ - title="Discussion Notes" + title={t("discussion_notes")} // At lines 131-132 - { text: "Thread View", value: "thread-view" }, - { text: "Default View", value: "default-view" }, + { text: t("thread_view"), value: "thread-view" }, + { text: t("default_view"), value: "default-view" },Remember to add these keys to your translation files.
Line range hint
138-198: Optimize Re-renders by Memoizing ComponentsThe layout includes components like
PatientConsultationNotesListandPatientNotesDetailedViewthat could benefit from memoization to prevent unnecessary re-renders.Wrap these components with
React.memo:export default React.memo(PatientConsultationNotesList);Additionally, ensure that any functions passed as props are memoized using
useCallbackto avoid causing re-renders due to changing references.Example:
const memoizedSetReplyTo = useCallback((note) => setReplyTo(note), [setReplyTo]); // Then pass memoizedSetReplyTo instead of setReplyTo <PatientConsultationNotesList ... setReplyTo={memoizedSetReplyTo} />This improves performance, especially in complex interfaces.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (10)
public/locale/en.json(1 hunks)src/Utils/request/api.tsx(3 hunks)src/common/constants.tsx(1 hunks)src/components/Common/FilePreviewDialog.tsx(1 hunks)src/components/Common/MarkdownPreview.tsx(1 hunks)src/components/Common/MentionDropdown.tsx(1 hunks)src/components/Common/RichTextEditor.tsx(1 hunks)src/components/Facility/ConsultationDetails/DiscussionNotesFilesTab.tsx(1 hunks)src/components/Facility/ConsultationDetails/index.tsx(2 hunks)src/components/Facility/ConsultationDoctorNotes/index.tsx(7 hunks)
✅ Files skipped from review due to trivial changes (1)
- public/locale/en.json
🚧 Files skipped from review as they are similar to previous changes (5)
- src/common/constants.tsx
- src/components/Common/FilePreviewDialog.tsx
- src/components/Common/MarkdownPreview.tsx
- src/components/Common/MentionDropdown.tsx
- src/components/Facility/ConsultationDetails/index.tsx
🔇 Additional comments (5)
src/components/Common/RichTextEditor.tsx (2)
514-518: Remove redundant onChange handler.
The onChange handler is redundant as onInput already updates the markdown state. This could lead to unnecessary re-renders.
<textarea
id="discussion_notes_textarea"
ref={editorRef}
className="max-h-[300px] min-h-[70px] w-full resize-none overflow-y-auto border-none p-2 align-middle text-sm outline-none focus:outline-none focus:ring-0"
value={markdown}
- onChange={(e) => {
- setMarkdown(e.target.value);
- }}
onInput={handleInput}
onKeyDown={onKeyDown}
/>504-507:
Ensure markdown rendering is secure against XSS attacks.
The MarkdownPreview component should sanitize the markdown content before rendering to prevent XSS attacks.
src/Utils/request/api.tsx (3)
814-818: LGTM! Well-structured endpoint definition.
The getPatientNote endpoint follows RESTful conventions and maintains consistency with other patient note endpoints.
Line range hint 1128-1151: LGTM! Comprehensive file upload endpoints.
The FileUploads section provides well-structured CRUD operations with proper type safety and error handling.
1152-1158: LGTM! Verify consultation file upload integration.
The consultation file upload endpoint aligns with the PR objectives for enhancing file upload capabilities.
✅ Verification successful
Consultation file upload integration verified successfully.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the consultation file upload endpoint integration
# Test: Search for components using the listConsultationFileUploads endpoint
# Expect: Usage in consultation/patient note related components
# Search for direct usage of the endpoint
rg "listConsultationFileUploads" -t tsx -t ts
# Search for components handling consultation files
ast-grep --pattern 'import { FileUploadModel } from "@/components/Patient/models"'
Length of output: 683
Script:
#!/bin/bash
# Description: Verify the consultation file upload endpoint integration with corrected ripgrep usage
# Test: Search for components using the listConsultationFileUploads endpoint
# Expect: Usage in consultation/patient note related components
# Search for direct usage of the endpoint in .ts and .tsx files
rg "listConsultationFileUploads" -g "*.ts" -g "*.tsx"
# Search for components handling consultation files
ast-grep --pattern 'import { FileUploadModel } from "@/components/Patient/models"'
Length of output: 1016
| const type = "NOTES"; | ||
| const activeFilesQuery = useQuery(routes.listConsultationFileUploads, { | ||
| query: { | ||
| is_archived: false, | ||
| limit: RESULTS_PER_PAGE_LIMIT, | ||
| offset: offset, | ||
| }, | ||
| pathParams: { consultation_external_id: props.consultationId }, | ||
| }); | ||
|
|
||
| const archivedFilesQuery = useQuery(routes.listConsultationFileUploads, { | ||
| query: { | ||
| is_archived: true, | ||
| limit: RESULTS_PER_PAGE_LIMIT, | ||
| offset: offset, | ||
| }, | ||
| pathParams: { consultation_external_id: props.consultationId }, | ||
| }); | ||
|
|
||
| const queries = { | ||
| UNARCHIVED: activeFilesQuery, | ||
| ARCHIVED: archivedFilesQuery, | ||
| }; | ||
|
|
||
| const loading = Object.values(queries).some((q) => q.loading); | ||
| const refetchAll = async () => | ||
| Promise.all(Object.values(queries).map((q) => q.refetch())); | ||
|
|
||
| const fileQuery = queries[tab as keyof typeof queries]; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for queries and consider consolidating duplicate query logic.
The queries for active and archived files share similar configuration. Consider:
- Adding error handling for failed queries
- Consolidating the query logic to reduce duplication
Example implementation:
const createFileQuery = (isArchived: boolean) => useQuery(routes.listConsultationFileUploads, {
query: {
is_archived: isArchived,
limit: RESULTS_PER_PAGE_LIMIT,
offset,
},
pathParams: { consultation_external_id: props.consultationId },
onError: (error) => {
// Handle error appropriately
console.error('Failed to fetch files:', error);
}
});
const queries = {
UNARCHIVED: createFileQuery(false),
ARCHIVED: createFileQuery(true),
};| const tabs = [ | ||
| { text: "Active Files", value: "UNARCHIVED" }, | ||
| { text: "Archived Files", value: "ARCHIVED" }, | ||
| ]; | ||
|
|
||
| const fileManager = useFileManager({ | ||
| type, | ||
| onArchive: refetchAll, | ||
| onEdit: refetchAll, | ||
| }); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Internationalize tab labels and add error handling for file operations.
Consider the following improvements:
- Use translation keys for tab labels
- Add error handling for file manager operations
Example implementation:
const tabs = [
{ text: t('active_files'), value: 'UNARCHIVED' },
{ text: t('archived_files'), value: 'ARCHIVED' },
];
const fileManager = useFileManager({
type,
onArchive: async () => {
try {
await refetchAll();
} catch (error) {
console.error('Failed to archive:', error);
// Handle error appropriately
}
},
onEdit: async () => {
try {
await refetchAll();
} catch (error) {
console.error('Failed to edit:', error);
// Handle error appropriately
}
},
});| {(fileQuery?.data?.results || []).length > RESULTS_PER_PAGE_LIMIT && ( | ||
| <div className="mt-4 flex w-full justify-center"> | ||
| <Pagination | ||
| cPage={currentPage} | ||
| defaultPerPage={RESULTS_PER_PAGE_LIMIT} | ||
| data={{ totalCount: (fileQuery?.data?.results || []).length }} | ||
| onChange={handlePagination} | ||
| /> | ||
| </div> | ||
| )} |
There was a problem hiding this comment.
Fix pagination condition and consider extracting permission logic.
The pagination condition fileQuery?.data?.results.length > RESULTS_PER_PAGE_LIMIT might be incorrect. It should compare with the total count instead of the page limit.
Apply this fix:
-{(fileQuery?.data?.results || []).length > RESULTS_PER_PAGE_LIMIT && (
+{fileQuery?.data?.count > RESULTS_PER_PAGE_LIMIT && (
<div className="mt-4 flex w-full justify-center">
<Pagination
cPage={currentPage}
defaultPerPage={RESULTS_PER_PAGE_LIMIT}
- data={{ totalCount: (fileQuery?.data?.results || []).length }}
+ data={{ totalCount: fileQuery?.data?.count }}
onChange={handlePagination}
/>
</div>
)}📝 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.
| {(fileQuery?.data?.results || []).length > RESULTS_PER_PAGE_LIMIT && ( | |
| <div className="mt-4 flex w-full justify-center"> | |
| <Pagination | |
| cPage={currentPage} | |
| defaultPerPage={RESULTS_PER_PAGE_LIMIT} | |
| data={{ totalCount: (fileQuery?.data?.results || []).length }} | |
| onChange={handlePagination} | |
| /> | |
| </div> | |
| )} | |
| {fileQuery?.data?.count > RESULTS_PER_PAGE_LIMIT && ( | |
| <div className="mt-4 flex w-full justify-center"> | |
| <Pagination | |
| cPage={currentPage} | |
| defaultPerPage={RESULTS_PER_PAGE_LIMIT} | |
| data={{ totalCount: fileQuery?.data?.count }} | |
| onChange={handlePagination} | |
| /> | |
| </div> | |
| )} |
| <button | ||
| onClick={() => insertMarkdown("**")} | ||
| className="tooltip rounded bg-gray-200/50 p-1" | ||
| > | ||
| <CareIcon icon="l-bold" className="text-lg" /> | ||
| <span className="tooltip-text tooltip-top -translate-x-4">Bold</span> | ||
| </button> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance accessibility with ARIA labels and keyboard support.
Interactive elements like buttons lack proper ARIA labels and keyboard support:
- Missing
aria-labelattributes - No keyboard focus indicators
- Missing role attributes where appropriate
<button
onClick={() => insertMarkdown("**")}
- className="tooltip rounded bg-gray-200/50 p-1"
+ className="tooltip rounded bg-gray-200/50 p-1 focus:ring-2 focus:ring-primary-500"
+ aria-label="Bold text"
+ role="button"
>
<CareIcon icon="l-bold" className="text-lg" />
<span className="tooltip-text tooltip-top -translate-x-4">Bold</span>
</button>Also applies to: 433-441, 453-460
| const fileUpload = useFileUpload({ | ||
| type: "NOTES", | ||
| category: "UNSPECIFIED", | ||
| multiple: true, | ||
| allowAllExtensions: true, | ||
| }); | ||
|
|
There was a problem hiding this comment.
Enhance file upload security measures.
The current file upload configuration lacks important security restrictions:
- No file size limits
- No file type validation
allowAllExtensions: truecould be a security risk
Consider adding these restrictions to prevent potential security issues and improve user experience.
const fileUpload = useFileUpload({
type: "NOTES",
category: "UNSPECIFIED",
multiple: true,
- allowAllExtensions: true,
+ allowAllExtensions: false,
+ maxFileSize: 5 * 1024 * 1024, // 5MB
+ allowedFileTypes: [
+ 'image/*',
+ 'audio/*',
+ 'application/pdf',
+ 'text/plain'
+ ],
});📝 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.
| const fileUpload = useFileUpload({ | |
| type: "NOTES", | |
| category: "UNSPECIFIED", | |
| multiple: true, | |
| allowAllExtensions: true, | |
| }); | |
| const fileUpload = useFileUpload({ | |
| type: "NOTES", | |
| category: "UNSPECIFIED", | |
| multiple: true, | |
| allowAllExtensions: false, | |
| maxFileSize: 5 * 1024 * 1024, // 5MB | |
| allowedFileTypes: [ | |
| 'image/*', | |
| 'audio/*', | |
| 'application/pdf', | |
| 'text/plain' | |
| ], | |
| }); |
| onClick={async () => { | ||
| if (!editorRef.current) return; | ||
| const id = await onAddNote(); | ||
| if (!id) return; | ||
| await fileUpload.handleFileUpload(id); | ||
| onRefetch?.(); | ||
| fileUpload.clearFiles(); | ||
| editorRef.current.innerHTML = ""; | ||
| setIsPreviewMode(false); | ||
| }} |
There was a problem hiding this comment.
Add error handling for async operations.
The async operations in the submit handler lack proper error handling, which could lead to a poor user experience if operations fail.
onClick={async () => {
if (!editorRef.current) return;
- const id = await onAddNote();
- if (!id) return;
- await fileUpload.handleFileUpload(id);
- onRefetch?.();
- fileUpload.clearFiles();
- editorRef.current.innerHTML = "";
- setIsPreviewMode(false);
+ try {
+ const id = await onAddNote();
+ if (!id) return;
+ await fileUpload.handleFileUpload(id);
+ onRefetch?.();
+ fileUpload.clearFiles();
+ editorRef.current.innerHTML = "";
+ setIsPreviewMode(false);
+ } catch (error) {
+ console.error('Failed to add note:', error);
+ // TODO: Add error notification
+ }
}}Committable suggestion skipped: line range outside the PR's diff.
| <div className="right-8 top-0 max-sm:my-1 sm:mx-2 md:absolute"> | ||
| <Tabs | ||
| className="mt-1 w-full gap-8 lg:w-full" | ||
| tabs={[ | ||
| { text: "Thread View", value: "thread-view" }, | ||
| { text: "Default View", value: "default-view" }, | ||
| ]} | ||
| currentTab={mode} | ||
| onTabChange={(tab) => setMode(tab as "thread-view" | "default-view")} | ||
| /> | ||
| </div> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve Accessibility of Tabs Component
The tab navigation for switching between views may not be fully accessible to keyboard and screen reader users.
To enhance accessibility:
- Add ARIA Attributes: Include
role="tablist"on the container androle="tab"on each tab button. - Manage Focus: Ensure that tab navigation can be performed using the keyboard (e.g., with arrow keys).
- Provide Labels: Add
aria-selected,aria-controls, andidattributes to manage the active state and associate tabs with their content.
Example modification:
<div
+ role="tablist"
className="mt-1 w-full gap-8 lg:w-full"
>
{tabs.map((tab) => (
<button
+ role="tab"
+ id={`tab-${tab.value}`}
+ aria-selected={mode === tab.value}
+ aria-controls={`tabpanel-${tab.value}`}
key={tab.value}
...
>
{tab.text}
</button>
))}
</div>Ensure the corresponding tab panels have role="tabpanel" and aria-labelledby attributes.
Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Outside diff range and nitpick comments (10)
src/components/Common/MarkdownPreview.tsx (2)
12-25: Enhance UserCard accessibility and edge case handling.Consider these improvements for the UserCard component:
- Add ARIA labels for better screen reader support
- Handle edge cases where first_name might be empty
const UserCard = ({ user }: { user: UserBareMinimum }) => ( - <div className="z-10 flex w-64 items-center space-x-3 rounded-lg bg-gray-200 px-3 pb-3 shadow-lg"> + <div + className="z-10 flex w-64 items-center space-x-3 rounded-lg bg-gray-200 px-3 pb-3 shadow-lg" + role="tooltip" + aria-label={`User details for ${user.first_name} ${user.last_name}`} + > <div className="flex h-10 w-10 items-center justify-center rounded-full bg-primary text-lg font-semibold text-white"> - {user.first_name[0]} + {user.first_name?.[0] || user.username[0]} </div>
87-97: Optimize component performance with memoization.Consider memoizing the component to prevent unnecessary re-renders, especially when handling large markdown content.
-const MarkdownPreview = ({ +const MarkdownPreview = React.memo(({ markdown, mentioned_users, }: { markdown: string; mentioned_users?: UserBareMinimum[]; -}) => { +}) => { // ... component implementation ... -}; +});Also consider memoizing the processed markdown if the content is large:
+ const processedMarkdownMemo = React.useMemo( + () => markdown + .replace(/@([a-zA-Z0-9_]{3,30})/g, ...) + .replace(/~~(.*?)~~/g, ...), + [markdown, mentioned_users] + );src/components/Facility/ConsultationDetails/DiscussionNotesFilesTab.tsx (3)
24-42: Add JSDoc documentation and consider property validation.The interfaces
ModalDetailsandStateInterfacewould benefit from:
- JSDoc documentation explaining their purpose and usage
- Required vs optional properties validation for
ModalDetails- Type constraints for numeric properties in
StateInterfaceExample implementation:
/** * Represents the details required for file modal operations * @property name - The name of the file * ... */ export interface ModalDetails { name: string; // Make required if always needed id: string; // Make required if always needed reason?: string; userArchived?: string; archiveTime?: string; associatedId: string; // Make required if always needed } /** * Manages the state of file preview functionality * @property zoom - Zoom level between 0.1 and 5.0 * @property rotation - Rotation angle in degrees (0-360) */ export interface StateInterface { open: boolean; isImage: boolean; name: string; extension: string; zoom: number; // Add constraints: 0.1 <= zoom <= 5.0 isZoomInDisabled: boolean; isZoomOutDisabled: boolean; rotation: number; // Add constraints: 0 <= rotation < 360 }
51-55: Enhance pagination logic with validation and error handling.The pagination logic could be improved by:
- Validating input parameters
- Handling edge cases
- Using URL parameters for bookmarkable pages
Example implementation:
const handlePagination = (page: number, limit: number) => { try { if (page < 1) throw new Error('Page must be positive'); if (limit !== RESULTS_PER_PAGE_LIMIT) { throw new Error('Invalid page limit'); } const offset = (page - 1) * limit; setCurrentPage(page); setOffset(offset); // Update URL for bookmarkable pages const searchParams = new URLSearchParams(window.location.search); searchParams.set('page', page.toString()); window.history.replaceState(null, '', `?${searchParams.toString()}`); } catch (error) { console.error('Pagination error:', error); // Reset to first page setCurrentPage(1); setOffset(0); } };
110-112: Enhance loading state UI with better skeleton loading.The current loading state could be improved to better match the actual content layout.
Example implementation:
const LoadingSkeleton = () => ( <> {[1, 2, 3].map((key) => ( <div key={key} className="flex animate-pulse gap-4 rounded-lg bg-gray-100 p-4"> <div className="h-12 w-12 rounded-full bg-gray-200" /> <div className="flex-1 space-y-2"> <div className="h-4 w-3/4 rounded bg-gray-200" /> <div className="h-4 w-1/2 rounded bg-gray-200" /> </div> </div> ))} </> ); // In component: {!(fileQuery?.data?.results || []).length && loading && <LoadingSkeleton />}src/components/Facility/ConsultationDoctorNotes/index.tsx (2)
Line range hint
64-92: Enhance error handling and i18n support in onAddNoteThe function has several areas for improvement:
- The error message should use the translation system
- The function should handle API failures explicitly
- The note validation could be more comprehensive
Consider applying these changes:
const onAddNote = async () => { if (!/\S+/.test(noteField)) { Notification.Error({ - msg: "Note Should Contain At Least 1 Character", + msg: t('error_messages.note_empty'), }); return; } const { res, data } = await request(routes.addPatientNote, { pathParams: { patientId: patientId, }, body: { note: noteField, thread, consultation: consultationId, reply_to: reply_to?.id, }, }); if (res?.status === 201) { Notification.Success({ msg: t('success_messages.note_added') }); setState({ ...state, cPage: 1 }); setNoteField(""); setReload(true); setReplyTo(undefined); + return data?.id; } - return data?.id; + Notification.Error({ msg: t('error_messages.note_add_failed') }); + return undefined; };
128-138: LGTM: Well-structured responsive layoutThe UI implementation is clean and follows responsive design best practices. The conditional rendering for thread view and the tab-based navigation are well-implemented.
Consider adding loading states to improve user experience during view transitions:
<div className="flex flex-1 overflow-hidden"> <div className={classNames( "flex flex-1 flex-col", threadViewNote && "max-sm:hidden", + isLoading && "opacity-50" )} >Also applies to: 157-199
src/components/Facility/PatientNotesSlideover.tsx (2)
Line range hint
37-86: Consider consolidating state managementThe component manages multiple pieces of state (thread, show, patientActive, reload, reply_to, noteField) which increases complexity and makes the component harder to maintain. Consider:
- Using useReducer for related state transitions
- Extracting notification logic into a custom hook
- Creating a dedicated hook for note field persistence
Example refactor for the notification state:
// useNotificationState.ts export function useNotificationState() { const notificationState = useNotificationSubscriptionState(); useEffect(() => { if (notificationState === "unsubscribed") { Notification.Warn({ msg: "Please subscribe to notifications to get live updates on discussion notes." }); } else if (notificationState === "subscribed_on_other_device") { Notification.Warn({ msg: "Please subscribe to notifications on this device to get live updates on discussion notes." }); } }, [notificationState]); return notificationState; }
Line range hint
87-103: Enhance error handling and validationThe current note addition logic has several areas for improvement:
- The empty note validation is basic and doesn't handle whitespace-only content properly
- No validation of markdown content
- No comprehensive error handling for API failures
Consider applying these improvements:
const onAddNote = async () => { - if (!/\S+/.test(noteField)) { + const trimmedNote = noteField.trim(); + if (!trimmedNote) { Notification.Error({ msg: "Note Should Contain At Least 1 Character", }); return; } + try { const { res, data } = await request(routes.addPatientNote, { pathParams: { patientId: patientId }, body: { - note: noteField, + note: trimmedNote, consultation: consultationId, thread, reply_to: reply_to?.id, }, }); if (res?.status === 201) { Notification.Success({ msg: "Note added successfully" }); setNoteField(""); setState({ ...state, cPage: 1 }); setReload(true); setReplyTo(undefined); return data?.id; } + throw new Error('Failed to add note'); + } catch (error) { + Notification.Error({ + msg: error.message || "Failed to add note. Please try again.", + }); + return undefined; + } };src/components/Facility/PatientNotesDetailedView.tsx (1)
37-38: Improve error message capitalizationFor better readability and consistency, the error message should follow standard sentence capitalization.
Apply this diff:
Notification.Error({ - msg: "Note Should Contain At Least 1 Character", + msg: "Note should contain at least 1 character", });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (8)
package.json(1 hunks)src/components/Common/MarkdownPreview.tsx(1 hunks)src/components/Common/MentionDropdown.tsx(1 hunks)src/components/Common/RichTextEditor.tsx(1 hunks)src/components/Facility/ConsultationDetails/DiscussionNotesFilesTab.tsx(1 hunks)src/components/Facility/ConsultationDoctorNotes/index.tsx(7 hunks)src/components/Facility/PatientNotesDetailedView.tsx(1 hunks)src/components/Facility/PatientNotesSlideover.tsx(7 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/Common/MentionDropdown.tsx
🔇 Additional comments (8)
package.json (1)
94-94: LGTM! Dependencies are up-to-date and secure.
The added markdown-related dependencies are using their latest stable versions and are well-maintained.
Let's verify the bundle size impact and ensure all required plugins are included:
Also applies to: 97-97
src/components/Facility/ConsultationDoctorNotes/index.tsx (2)
2-2: LGTM: Clean state management and i18n setup
The new state management for thread view and the integration of i18n translations are well-implemented. The TypeScript types are properly defined for the mode state.
Also applies to: 5-6, 50-53
129-137:
Address accessibility concerns in tab navigation
The previous review comment about accessibility is still valid. Additionally, consider adding keyboard navigation support:
Add keyboard event handlers:
<Tabs
className="mt-1 w-full gap-8 lg:w-full"
+ onKeyDown={(e) => {
+ if (e.key === 'ArrowRight') {
+ setMode('thread-view');
+ } else if (e.key === 'ArrowLeft') {
+ setMode('default-view');
+ }
+ }}
tabs={[
{ text: "Thread View", value: "thread-view" },
{ text: "Default View", value: "default-view" },
]}
currentTab={mode}
onTabChange={(tab) => setMode(tab as "thread-view" | "default-view")}
/>src/components/Facility/PatientNotesSlideover.tsx (1)
241-258: Ensure rich text editor meets accessibility standards
The new rich text editor integration should be tested for accessibility compliance:
- Keyboard navigation
- Screen reader compatibility
- ARIA labels and roles
- Color contrast for markdown syntax
Consider implementing the following accessibility enhancements:
- Add aria-label to the editor
- Ensure proper focus management when switching between reply and edit modes
- Add keyboard shortcuts for common formatting actions
- Include screen reader announcements for state changes
src/components/Common/RichTextEditor.tsx (4)
515-520: 🛠️ Refactor suggestion
Optimize state updates in event handlers
The onChange handler updates setMarkdown directly while handleInput (attached to onInput) also updates setMarkdown. This leads to redundant state updates.
- onChange={(e) => {
- setMarkdown(e.target.value);
- }}
+ onChange={handleInput}
onInput={handleInput}
onKeyDown={onKeyDown}Likely invalid or redundant comment.
609-618:
Add error handling for async operations
The async operations lack proper error handling, which could lead to a poor user experience if operations fail.
onClick={async () => {
if (!editorRef.current) return;
+ try {
const id = await onAddNote();
if (!id) return;
await fileUpload.handleFileUpload(id);
onRefetch?.();
fileUpload.clearFiles();
editorRef.current.innerHTML = "";
setIsPreviewMode(false);
+ } catch (error) {
+ console.error('Failed to add note:', error);
+ // TODO: Add user notification for failure
+ }
}}Likely invalid or redundant comment.
407-413: 🛠️ Refactor suggestion
Enhance accessibility with ARIA labels
Interactive elements lack proper accessibility attributes:
- Missing
aria-labelattributes - No keyboard focus indicators
- Missing role attributes
<button
onClick={() => insertMarkdown("**")}
- className="tooltip rounded bg-gray-200/50 p-1"
+ className="tooltip rounded bg-gray-200/50 p-1 focus:ring-2 focus:ring-primary-500"
+ aria-label="Bold text"
+ role="button"
>
<CareIcon icon="l-bold" className="text-lg" />
<span className="tooltip-text tooltip-top -translate-x-4">Bold</span>
</button>Likely invalid or redundant comment.
505-509: Verify XSS protection in Markdown preview
When rendering user-generated Markdown content, ensure proper XSS protection is in place.
| const processedMarkdown = markdown | ||
| .replace(/@(\w+)/g, (_, username) => { | ||
| const user = MentionedUsers[username]; | ||
| if (user) { | ||
| return `<a href="/user/profile/${username}" data-username="${username}">@${username}</a>`; | ||
| } else { | ||
| return `@${username}`; | ||
| } | ||
| }) | ||
| .replace(/~~(.*?)~~/g, (_, text) => `<del>${text}</del>`); |
There was a problem hiding this comment.
Strengthen input validation for user mentions.
The current regex pattern for usernames (\w+) is too permissive and could lead to security issues:
- It allows any alphanumeric characters without length limits
- No validation against actual username format requirements
const processedMarkdown = markdown
- .replace(/@(\w+)/g, (_, username) => {
+ .replace(/@([a-zA-Z0-9_]{3,30})/g, (_, username) => {
const user = MentionedUsers[username];
if (user) {
+ const sanitizedUsername = username.replace(/[<>"'&]/g, '');
return `<a href="/user/profile/${username}" data-username="${username}">@${username}</a>`;
} else {
return `@${username}`;
}
})Committable suggestion skipped: line range outside the PR's diff.
| const CustomLink = (props: CustomLinkProps) => { | ||
| const [isHovered, setIsHovered] = useState(false); | ||
|
|
||
| if (props["data-username"]) { | ||
| const username = props["data-username"]; | ||
| return ( | ||
| <span | ||
| className="relative inline-block" | ||
| onMouseEnter={() => setIsHovered(true)} | ||
| onMouseLeave={() => setIsHovered(false)} | ||
| > | ||
| <span | ||
| onClick={(e) => { | ||
| e.stopPropagation(); | ||
| }} | ||
| className="cursor-pointer rounded bg-blue-100 px-1 font-normal text-slate-800 no-underline" | ||
| > | ||
| @{username} | ||
| </span> | ||
| {MentionedUsers[username] && isHovered && ( | ||
| <div className="tooltip-text absolute bottom-full z-10 mb-2 transition-opacity duration-300 ease-in-out"> | ||
| <UserCard user={MentionedUsers[username]} /> | ||
| <div className="absolute left-2 top-full border-8 border-solid border-transparent border-t-gray-200 shadow-md"></div> | ||
| </div> | ||
| )} | ||
| </span> | ||
| ); | ||
| } | ||
| return ( | ||
| <a | ||
| {...props} | ||
| target="_blank" | ||
| onClick={(e) => e.stopPropagation()} | ||
| className="text-blue-500 underline" | ||
| /> | ||
| ); | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve tooltip positioning and event handling.
The current tooltip implementation has several potential issues:
- Fixed positioning might cause tooltips to be cut off at screen edges
- Click event handling could be more robust
Consider using a tooltip library like @floating-ui/react for better positioning, or implement boundary detection:
- <div className="tooltip-text absolute bottom-full z-10 mb-2 transition-opacity duration-300 ease-in-out">
+ <div
+ className="tooltip-text absolute bottom-full z-10 mb-2 transition-opacity duration-300 ease-in-out"
+ style={{
+ transform: `translate(max(calc(-50% + 20px), calc(-100% + ${window.innerWidth - 320}px)))`
+ }}
+ >Also, improve click handling:
- onClick={(e) => {
- e.stopPropagation();
- }}
+ onClick={(e) => {
+ e.preventDefault();
+ e.stopPropagation();
+ window.location.href = `/user/profile/${username}`;
+ }}Committable suggestion skipped: line range outside the PR's diff.
| const activeFilesQuery = useQuery(routes.listConsultationFileUploads, { | ||
| query: { | ||
| is_archived: false, | ||
| limit: RESULTS_PER_PAGE_LIMIT, | ||
| offset: offset, | ||
| }, | ||
| pathParams: { consultation_external_id: props.consultationId }, | ||
| }); | ||
|
|
||
| const archivedFilesQuery = useQuery(routes.listConsultationFileUploads, { | ||
| query: { | ||
| is_archived: true, | ||
| limit: RESULTS_PER_PAGE_LIMIT, | ||
| offset: offset, | ||
| }, | ||
| pathParams: { consultation_external_id: props.consultationId }, | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add TypeScript types for query responses and implement error boundaries.
The queries would benefit from:
- Proper TypeScript types for the API response
- Error boundary implementation for graceful error handling
Example implementation:
interface FileQueryResponse {
results: FileUploadModel[];
count: number;
next?: string;
previous?: string;
}
// Add error boundary component
class FileListErrorBoundary extends React.Component<{children: React.ReactNode}> {
state = { hasError: false };
static getDerivedStateFromError() {
return { hasError: true };
}
render() {
if (this.state.hasError) {
return <div className="text-error-500">Error loading files. Please try again.</div>;
}
return this.props.children;
}
}
// Type the query response
const activeFilesQuery = useQuery<FileQueryResponse>(routes.listConsultationFileUploads, {
// ... existing config
});| const fileManager = useFileManager({ | ||
| type, | ||
| onArchive: refetchAll, | ||
| onEdit: refetchAll, | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve file manager error handling and add loading states.
The file manager callbacks should handle errors and show appropriate loading states.
Example implementation:
const [isOperationInProgress, setOperationInProgress] = useState(false);
const fileManager = useFileManager({
type,
onArchive: async () => {
try {
setOperationInProgress(true);
await refetchAll();
} catch (error) {
console.error('Archive operation failed:', error);
// Show error toast/notification
} finally {
setOperationInProgress(false);
}
},
onEdit: async () => {
try {
setOperationInProgress(true);
await refetchAll();
} catch (error) {
console.error('Edit operation failed:', error);
// Show error toast/notification
} finally {
setOperationInProgress(false);
}
},
});| editable={ | ||
| item?.uploaded_by?.username === authUser.username || | ||
| authUser.user_type === "DistrictAdmin" || | ||
| authUser.user_type === "StateAdmin" | ||
| } | ||
| archivable={true} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Extract authorization logic into a custom hook.
The authorization check should be moved to a custom hook for better reusability and maintainability.
Example implementation:
const useFilePermissions = (uploadedByUsername: string) => {
const authUser = useAuthUser();
return {
canEdit: useMemo(() =>
uploadedByUsername === authUser.username ||
['DistrictAdmin', 'StateAdmin'].includes(authUser.user_type)
, [uploadedByUsername, authUser]),
canArchive: true
};
};
// In component:
const { canEdit, canArchive } = useFilePermissions(item?.uploaded_by?.username);
return (
<FileBlock
// ...other props
editable={canEdit}
archivable={canArchive}
/>
);| import { Link } from "raviger"; | ||
| import { Dispatch, SetStateAction, useEffect, useState } from "react"; | ||
| import useKeyboardShortcut from "use-keyboard-shortcut"; | ||
| import { useTranslation } from "react-i18next"; |
There was a problem hiding this comment.
💡 Codebase verification
Keyboard Shortcut Implementations Still Present
The following files still contain keyboard shortcut implementations:
- src/components/Form/SearchInput.tsx
- src/CAREUI/interactive/KeyboardShortcut.tsx
🔗 Analysis chain
Verify UX impact of removing keyboard shortcuts
The removal of keyboard shortcuts in favor of the rich text editor represents a significant change in user interaction. While the rich text editor adds more functionality, we should ensure this doesn't negatively impact power users who relied on keyboard shortcuts.
Also applies to: 6-6, 8-8
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining keyboard shortcut implementations or documentation
rg -g '!{node_modules,dist,build}/**' -l 'useKeyboardShortcut|keyboard.*shortcut'
Length of output: 159
Script:
#!/bin/bash
# Corrected script to search for any remaining keyboard shortcut implementations or documentation
rg -g '!{node_modules,dist,build}/**' -l "useKeyboardShortcut|keyboard.*shortcut"
Length of output: 247
| const fileUpload = useFileUpload({ | ||
| type: "NOTES", | ||
| category: "UNSPECIFIED", | ||
| multiple: true, | ||
| allowAllExtensions: true, | ||
| }); |
There was a problem hiding this comment.
Add file upload restrictions for security
The current file upload configuration lacks important security measures:
- No file size limits
- No file type restrictions
allowAllExtensions: truecould be a security risk
const fileUpload = useFileUpload({
type: "NOTES",
category: "UNSPECIFIED",
multiple: true,
- allowAllExtensions: true,
+ allowAllExtensions: false,
+ maxFileSize: 5 * 1024 * 1024, // 5MB limit
+ allowedFileTypes: [
+ 'image/*',
+ 'audio/*',
+ 'application/pdf',
+ 'text/plain'
+ ],
});📝 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.
| const fileUpload = useFileUpload({ | |
| type: "NOTES", | |
| category: "UNSPECIFIED", | |
| multiple: true, | |
| allowAllExtensions: true, | |
| }); | |
| const fileUpload = useFileUpload({ | |
| type: "NOTES", | |
| category: "UNSPECIFIED", | |
| multiple: true, | |
| allowAllExtensions: false, | |
| maxFileSize: 5 * 1024 * 1024, // 5MB limit | |
| allowedFileTypes: [ | |
| 'image/*', | |
| 'audio/*', | |
| 'application/pdf', | |
| 'text/plain' | |
| ], | |
| }); |
This PR is created to track my progress on the GSoC project (#7159) and to merge all PRs related to Doctor notes.
Backend RR: ohcnetwork/care#2295
walk_through_discussion_notes_enhancement.mp4
File Upload Option
User Tagging Mechanism
Summary by CodeRabbit
Release Notes
New Features
MarkdownPreviewcomponent.MentionsDropdowncomponent.ConsultationDoctorNotescomponent.DiscussionNotesFilestab in theConsultationDetailscomponent for managing discussion notes files.PatientNotesDetailedViewcomponent for detailed viewing of patient notes and replies.PatientNotescomponent to improve type safety and internationalization support.Bug Fixes
Documentation
Chores