Migrated Dialog and ConfirmDialog with shadCN component #10510
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThis PR refactors several modal dialog components across the codebase. The changes uniformly replace the old modal components and prop names (such as Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ModalComponent
participant Dialog
participant Handler
User->>ModalComponent: Triggers close action
ModalComponent->>Handler: Calls onOpenChange(false)
Handler-->>Dialog: Updates dialog state to hidden
Dialog-->>User: Modal is closed
Possibly related PRs
Suggested labels
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
✅ Deploy Preview for care-ohc ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| <> | ||
| <div className="flex flex-1 items-center justify-center rounded-lg"> | ||
| <img | ||
| src={preview || imageUrl} |
Check warning
Code scanning / CodeQL
DOM text reinterpreted as HTML Medium
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
src/hooks/useFileUpload.tsx (1)
365-382: Maintain consistency in dialog props across components.While CameraCaptureDialog has been migrated to use
openandonOpenChange, AudioCaptureDialog still uses the oldshowandonHideprops. Consider migrating AudioCaptureDialog to maintain consistency.<AudioCaptureDialog - show={audioModalOpen} - onHide={() => setAudioModalOpen(false)} + open={audioModalOpen} + onOpenChange={() => setAudioModalOpen(false)} onCapture={(file) => { setFiles((prev) => [...prev, file]); }} autoRecord />
🧹 Nitpick comments (2)
src/components/Files/CameraCaptureDialog.tsx (1)
42-63: Consider enhancing cleanup logic in useEffect.The cleanup function could be more robust by storing the MediaStream in a ref to ensure proper cleanup across re-renders.
+const streamRef = useRef<MediaStream | null>(null); useEffect(() => { if (!open) return; - let stream: MediaStream | null = null; navigator.mediaDevices .getUserMedia({ video: { facingMode: cameraFacingMode } }) .then((mediaStream) => { - stream = mediaStream; + streamRef.current = mediaStream; }) .catch(() => { toast.warning(t("camera_permission_denied")); onOpenChange(false); }); return () => { - if (stream) { - stream.getTracks().forEach((track) => { + if (streamRef.current) { + streamRef.current.getTracks().forEach((track) => { track.stop(); }); + streamRef.current = null; } }; }, [open, cameraFacingMode, onOpenChange]);src/components/Patient/PatientRegistration.tsx (1)
283-287: Simplify dialog visibility control.The dialog's visibility is controlled redundantly through both conditional rendering and the
openprop. This creates confusion about the source of truth for the dialog's visibility state.Apply these changes to simplify the visibility control:
-{showDuplicate && ( - <DuplicatePatientDialog - open={true} - patientList={duplicatePatients} - handleOk={handleDialogClose} - onOpenChange={() => { - handleDialogClose("close"); - }} - /> -)} +<DuplicatePatientDialog + open={showDuplicate} + patientList={duplicatePatients} + onOpenChange={(open) => { + if (!open) { + handleDialogClose("close"); + } + }} +/>Also applies to: 735-744
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
src/components/Common/AvatarEditModal.tsx(4 hunks)src/components/Common/FilePreviewDialog.tsx(6 hunks)src/components/Facility/DuplicatePatientDialog.tsx(2 hunks)src/components/Facility/FacilityHome.tsx(1 hunks)src/components/Files/CameraCaptureDialog.tsx(7 hunks)src/components/Patient/PatientRegistration.tsx(1 hunks)src/components/Users/UserAvatar.tsx(1 hunks)src/hooks/useFileManager.tsx(1 hunks)src/hooks/useFileUpload.tsx(1 hunks)
🧰 Additional context used
🪛 GitHub Check: CodeQL
src/components/Common/AvatarEditModal.tsx
[warning] 199-199: DOM text reinterpreted as HTML
DOM text is reinterpreted as HTML without escaping meta-characters.
🪛 Biome (1.9.4)
src/components/Common/FilePreviewDialog.tsx
[error] 202-204: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 301-352: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment
(lint/complexity/noUselessFragments)
[error] 355-395: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment
(lint/complexity/noUselessFragments)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: cypress-run (1)
- GitHub Check: OSSAR-Scan
🔇 Additional comments (10)
src/components/Users/UserAvatar.tsx (1)
100-100: LGTM! Prop update aligns with shadCN Dialog pattern.The change from
onClosetoonOpenChangefollows the standardized pattern for dialog state management.src/components/Facility/DuplicatePatientDialog.tsx (2)
25-28: LGTM! Props interface follows shadCN pattern.The interface update properly defines the new dialog control props.
38-141: LGTM! Dialog structure follows shadCN component hierarchy.The migration to shadCN Dialog components improves the semantic structure while maintaining functionality.
src/components/Files/CameraCaptureDialog.tsx (1)
18-24: LGTM! Props interface follows shadCN pattern.The interface update properly defines the new dialog control props.
src/components/Common/FilePreviewDialog.tsx (2)
19-24: LGTM! Props updated to align with shadCN Dialog API.The changes standardize the dialog visibility control props to match shadCN's Dialog component API, improving consistency across the codebase.
Also applies to: 46-47
172-183: LGTM! Improved dialog structure with semantic components.The new structure using
DialogContent,DialogHeader, andDialogTitleenhances accessibility and semantic markup.src/components/Facility/FacilityHome.tsx (1)
206-206: LGTM! Updated AvatarEditModal prop to match new API.The change from
onClosetoonOpenChangemaintains consistency with the new dialog component API.src/components/Common/AvatarEditModal.tsx (1)
15-20: LGTM! Clean migration to shadCN Dialog components.The component has been properly migrated to use shadCN's Dialog components with appropriate prop types.
Also applies to: 27-27
src/hooks/useFileManager.tsx (1)
253-254: LGTM! Updated FilePreviewDialog props to match new API.The hook has been updated to use the new
openandonOpenChangeprops consistently.Also applies to: 259-259
src/components/Patient/PatientRegistration.tsx (1)
220-230: 🏁 Script executed:#!/bin/bash echo "Searching for references of handleDialogClose usage across the codebase..." rg -n "handleDialogClose" . echo "" echo "Searching for references of 'transfer' action usage across the codebase..." rg -n "transfer" .Length of output: 6094
| <img | ||
| src={fileUrl} | ||
| alt="file" | ||
| className={`h-full w-full object-contain ${ | ||
| zoom_values[file_state.zoom - 1] | ||
| } ${getRotationClass(file_state.rotation)}`} | ||
| /> |
There was a problem hiding this comment.
Validate and sanitize image URL to prevent XSS.
The image source is directly used without validation or sanitization, which could lead to XSS attacks if the URL contains malicious content.
Apply this diff to add URL validation:
- <img
- src={preview || imageUrl}
- alt="cover-photo"
- className="h-full w-full object-cover"
- />
+ <img
+ src={preview && preview.startsWith('blob:') ? preview :
+ imageUrl && (imageUrl.startsWith('http://') || imageUrl.startsWith('https://')) ? imageUrl : '#'}
+ alt="cover-photo"
+ className="h-full w-full object-cover"
+ onError={(e) => {
+ e.currentTarget.src = '#';
+ console.error('Failed to load image');
+ }}
+ />Committable suggestion skipped: line range outside the PR's diff.
| <DuplicatePatientDialog | ||
| open={true} | ||
| patientList={duplicatePatients} | ||
| handleOk={handleDialogClose} | ||
| handleCancel={() => { | ||
| onOpenChange={() => { | ||
| handleDialogClose("close"); | ||
| }} | ||
| /> |
There was a problem hiding this comment.
Revise dialog props for consistent state management.
The current implementation has a few issues:
- The hardcoded
open={true}will prevent the dialog from being closed. - Mixed usage of
handleOkandonOpenChangeprops is inconsistent with shadcn Dialog patterns.
Apply this diff to fix the issues:
<DuplicatePatientDialog
- open={true}
+ open={showDuplicate}
patientList={duplicatePatients}
- handleOk={handleDialogClose}
onOpenChange={(open) => {
- handleDialogClose("close");
+ if (!open) {
+ handleDialogClose("close");
+ }
}}
/>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
src/components/Users/UserDeleteDialog.tsx (1)
27-48: Validate the naming of onOpenChange handler.Currently, onOpenChange calls “handleCancel”, which semantically suggests only a cancel action. Consider renaming “handleCancel” to “onClose” or “onChange” for clarity. Otherwise, the usage of AlertDialog and its destructive action for deletion is correct.
<AlertDialog open={props.show} - onOpenChange={props.handleCancel}> + onOpenChange={(open) => !open && props.handleCancel()}>src/components/Common/FilePreviewDialog.tsx (3)
198-200: Use optional chaining for cleaner checks.Instead of “uploadedFiles && uploadedFiles[index] && uploadedFiles[index].created_date”, optional chaining reduces nested checks and improves readability.
- {uploadedFiles && uploadedFiles[index] && uploadedFiles[index].created_date && ( + {uploadedFiles?.[index]?.created_date && ( <p className="mt-1 text-sm text-gray-600"> {new Date(uploadedFiles[index].created_date!).toLocaleString("en-US", { dateStyle: "long", timeStyle: "short", })} </p> )}🧰 Tools
🪛 Biome (1.9.4)
[error] 198-200: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
296-347: Remove unnecessary fragment.The fragment here only wraps one mapped array. You could directly render the map without a fragment.
- <> {[ [t("Zoom In"), "l-search-plus", handleZoomIn, file_state.zoom === zoom_values.length], ... ].map((button, index) => ( ... ))} - </>🧰 Tools
🪛 Biome (1.9.4)
[error] 296-347: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment(lint/complexity/noUselessFragments)
350-390: Consider removing the extra fragment.Again, this fragment simply encloses a single mapped array. Streamlining improves readability.
🧰 Tools
🪛 Biome (1.9.4)
[error] 350-390: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment(lint/complexity/noUselessFragments)
public/locale/en.json (1)
393-393: Localization Entry Addition: Delete User Confirmation
The inclusion of the key "are_you_sure_you_want_to_delete_user": "Are you sure you want to delete this user?" provides a clear and user-friendly confirmation prompt. Ensure that all corresponding UI components (for example, the UserDeleteDialog) reference this key to maintain consistency after the migration to the ShadCN Dialog component.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
public/locale/en.json(2 hunks)src/components/Common/ConfirmDialog.tsx(0 hunks)src/components/Common/Dialog.tsx(0 hunks)src/components/Common/FilePreviewDialog.tsx(3 hunks)src/components/Facility/FacilityHome.tsx(1 hunks)src/components/Patient/PatientHome.tsx(0 hunks)src/components/Patient/PatientRegistration.tsx(1 hunks)src/components/Users/UserDeleteDialog.tsx(1 hunks)src/components/Users/UserSummary.tsx(1 hunks)src/hooks/useFileManager.tsx(2 hunks)
💤 Files with no reviewable changes (3)
- src/components/Common/ConfirmDialog.tsx
- src/components/Common/Dialog.tsx
- src/components/Patient/PatientHome.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- src/components/Facility/FacilityHome.tsx
- src/hooks/useFileManager.tsx
- src/components/Patient/PatientRegistration.tsx
🧰 Additional context used
📓 Learnings (2)
src/components/Users/UserSummary.tsx (1)
Learnt from: Jacobjeevan
PR: ohcnetwork/care_fe#9080
File: src/components/Users/UserSummary.tsx:53-71
Timestamp: 2024-11-22T12:04:39.044Z
Learning: In the `UserSummaryTab` component (`src/components/Users/UserSummary.tsx`), when there is an error during user deletion, the delete dialog is intentionally closed to prevent the user from immediately trying to delete the user again.
src/components/Common/FilePreviewDialog.tsx (1)
Learnt from: Jacobjeevan
PR: ohcnetwork/care_fe#9196
File: src/components/Common/FilePreviewDialog.tsx:193-200
Timestamp: 2024-11-26T09:46:12.574Z
Learning: When adding navigation buttons, include aria labels and handle arrow keys (ArrowLeft and ArrowRight) in onKeyDown events to enhance accessibility.
🪛 Biome (1.9.4)
src/components/Common/FilePreviewDialog.tsx
[error] 198-200: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 296-347: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment
(lint/complexity/noUselessFragments)
[error] 350-390: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment
(lint/complexity/noUselessFragments)
🔇 Additional comments (7)
src/components/Users/UserDeleteDialog.tsx (3)
1-15: All imports look good.These new imports for the AlertDialog components from "@/components/ui/alert-dialog" are correctly set up.
21-21: Nice addition of the 'show' prop.This boolean property clarifies the dialog's visibility at the interface level.
25-25: Internationalization usage is appropriate.Using the i18n hook for dynamic text is consistent and helps maintain translation coverage.
src/components/Users/UserSummary.tsx (1)
92-98: Conditional rendering vs. open prop usage.Here, you're both conditionally rendering the dialog and passing the “show” prop. This works fine, but you could rely on only the dialog's "open" state if preferred. Overall, it’s cohesive with your updated confirmation flow.
src/components/Common/FilePreviewDialog.tsx (2)
244-250: Validate and sanitize image URL to prevent XSS.(Repeated from a previous review) The image source is directly used without validation or sanitization, which can lead to XSS attacks if the URL is malicious.
156-163: Rotation normalization logic is correct.The modulo arithmetic ensures the rotation value remains in the [-180, 180] range.
public/locale/en.json (1)
385-385: Localization Entry Addition: Archive File
The new key "archive_file": "Archive File" is concise and clearly conveys the intended UI label. It aligns well with the overall localization structure and supports the author’s refactoring objectives.
| {showDeleteDialog && ( | ||
| <UserDeleteDialog | ||
| show={showDeleteDialog} |
There was a problem hiding this comment.
redundant check here as well
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/components/Common/FilePreviewDialog.tsx (1)
244-250: Validate and sanitize image URL to prevent XSS.The image source is directly used without validation or sanitization, which could lead to XSS attacks if the URL contains malicious content.
Apply this diff to add URL validation:
- <img - src={fileUrl} - alt="file" - className={`h-full w-full object-contain ${ - zoom_values[file_state.zoom - 1] - } ${getRotationClass(file_state.rotation)}`} - /> + <img + src={fileUrl && (fileUrl.startsWith('http://') || fileUrl.startsWith('https://')) ? fileUrl : '#'} + alt="file" + className={`h-full w-full object-contain ${ + zoom_values[file_state.zoom - 1] + } ${getRotationClass(file_state.rotation)}`} + onError={(e) => { + e.currentTarget.src = '#'; + console.error('Failed to load image'); + }} + />src/components/Common/AvatarEditModal.tsx (1)
198-202: Validate and sanitize image URL to prevent XSS.The image source is directly used without validation or sanitization, which could lead to XSS attacks if the URL contains malicious content.
Apply this diff to add URL validation:
- <img - src={preview || imageUrl} - alt="cover-photo" - className="h-full w-full object-cover" - /> + <img + src={preview && preview.startsWith('blob:') ? preview : + imageUrl && (imageUrl.startsWith('http://') || imageUrl.startsWith('https://')) ? imageUrl : '#'} + alt="cover-photo" + className="h-full w-full object-cover" + onError={(e) => { + e.currentTarget.src = '#'; + console.error('Failed to load image'); + }} + />🧰 Tools
🪛 GitHub Check: CodeQL
[warning] 199-199: DOM text reinterpreted as HTML
DOM text is reinterpreted as HTML without escaping meta-characters.
🧹 Nitpick comments (1)
src/components/Common/FilePreviewDialog.tsx (1)
232-240: Add aria-label to navigation buttons for better accessibility.The previous/next file navigation buttons need proper aria-labels for screen readers.
Apply this diff to improve accessibility:
- aria-label="Previous file" + aria-label={t("previous_file")}- aria-label={t("next_file")} + aria-label={t("next_file")}Also applies to: 281-290
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
public/locale/en.json(6 hunks)src/components/Common/AvatarEditModal.tsx(4 hunks)src/components/Common/FilePreviewDialog.tsx(3 hunks)src/components/Facility/FacilityHome.tsx(1 hunks)src/components/Patient/PatientRegistration.tsx(1 hunks)src/components/Users/UserAvatar.tsx(1 hunks)src/components/Users/UserSummary.tsx(1 hunks)src/hooks/useFileUpload.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- src/hooks/useFileUpload.tsx
- src/components/Users/UserSummary.tsx
- src/components/Users/UserAvatar.tsx
- src/components/Facility/FacilityHome.tsx
- src/components/Patient/PatientRegistration.tsx
🧰 Additional context used
🪛 GitHub Check: CodeQL
src/components/Common/AvatarEditModal.tsx
[warning] 199-199: DOM text reinterpreted as HTML
DOM text is reinterpreted as HTML without escaping meta-characters.
🪛 Biome (1.9.4)
src/components/Common/FilePreviewDialog.tsx
[error] 198-200: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 296-347: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment
(lint/complexity/noUselessFragments)
[error] 350-390: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment
(lint/complexity/noUselessFragments)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: OSSAR-Scan
- GitHub Check: cypress-run (1)
🔇 Additional comments (5)
src/components/Common/FilePreviewDialog.tsx (2)
19-24: LGTM! Dialog component imports follow best practices.The imports are properly organized and use the new shadCN Dialog components.
172-178: LGTM! Dialog structure follows shadCN patterns.The Dialog component structure with DialogContent, DialogHeader, and DialogTitle follows the recommended shadCN patterns for modal dialogs.
src/components/Common/AvatarEditModal.tsx (2)
15-20: LGTM! Props interface update follows shadCN patterns.The Props interface correctly replaces onClose with onOpenChange to align with shadCN Dialog patterns.
Also applies to: 27-27
186-190: LGTM! Dialog structure follows shadCN patterns.The Dialog component structure with DialogContent, DialogHeader, and DialogTitle follows the recommended shadCN patterns for modal dialogs.
public/locale/en.json (1)
385-386: LGTM! Translation keys and values follow conventions.The new translation entries follow the established naming conventions and are properly formatted.
Also applies to: 393-394, 695-696, 1348-1349, 1666-1667, 1828-1829
rithviknishad
added
needs testing
needs review
changes required
and removed
changes required
needs testing
needs review
labels
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/components/Facility/DuplicatePatientDialog.tsx (2)
52-79: Enhance table accessibility.Consider adding these accessibility improvements to the table:
- Add
aria-labelto describe the table's purpose- Include
scope="col"on table headers- <Table> + <Table aria-label="Duplicate patient records"> <TableHeader> <TableRow> {[`${t("patient_name")} / ID`, t("gender")].map( (heading, i) => ( - <TableHead key={i}>{heading}</TableHead> + <TableHead key={i} scope="col">{heading}</TableHead> ), )} </TableRow>
82-118: Improve form accessibility and structure.The radio button group should be wrapped in a proper form structure with fieldset and legend for better accessibility.
- <div className="flex flex-col"> + <form className="flex flex-col"> + <fieldset> + <legend className="sr-only">{t("duplicate_patient_action_selection")}</legend> <div className="mb-2 flex items-center"> <label className="mb-2 ml-0 flex w-full rounded-md bg-primary-500 py-2 pr-2 text-white" htmlFor="transfer" > <input type="radio" className="m-3 text-green-600 focus:ring-2 focus:ring-green-500" id="transfer" name="confirm_action" value="transfer" onChange={(e) => setAction(e.target.value)} /> <p>{t("duplicate_patient_record_confirmation")}</p> </label> </div> {/* ... rest of the radio buttons ... */} + </fieldset> - </div> + </form>src/components/Files/CameraCaptureDialog.tsx (1)
136-209: Ensure consistent optional chaining usage forsetPreview.While the mobile/tablet layout is well-organized, there's inconsistent usage of optional chaining for the
setPreviewprop. Some calls use?.while others don't.Apply this diff to ensure consistency:
- onOpenChange(false); - setPreview?.(false); + onOpenChange(false); + setPreview?.(false);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/components/Facility/DuplicatePatientDialog.tsx(2 hunks)src/components/Files/CameraCaptureDialog.tsx(7 hunks)src/components/Patient/PatientRegistration.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/Patient/PatientRegistration.tsx
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Redirect rules - care-ohc
- GitHub Check: Header rules - care-ohc
- GitHub Check: Pages changed - care-ohc
- GitHub Check: cypress-run (1)
🔇 Additional comments (7)
src/components/Facility/DuplicatePatientDialog.tsx (2)
7-13: LGTM! Clean migration to ShadCN Dialog.The imports and interface changes correctly implement ShadCN's Dialog component API, improving the component's maintainability.
Also applies to: 25-30
34-35: LGTM! Props and state management are well-structured.The component correctly handles both the new dialog props and maintains the necessary internal state.
src/components/Files/CameraCaptureDialog.tsx (5)
9-14: LGTM! Props updated to match ShadCN's Dialog API.The interface changes from
show/onHidetoopen/onOpenChangealign with React conventions and ShadCN's Dialog API.Also applies to: 18-24
26-40: LGTM! Clean prop destructuring with maintained state management.The component initialization properly adapts to the new prop names while preserving the existing camera state management logic.
42-63: LGTM! Effect hook properly adapted to new prop names.The effect hook correctly uses the
openprop for conditional execution andonOpenChangefor permission denial handling, while maintaining proper cleanup of camera resources.
95-112: LGTM! Well-structured dialog with semantic components.The new dialog structure using ShadCN components provides better semantics and layout organization. The header with the camera icon and title is nicely styled.
211-270: LGTM! Well-organized laptop layout with proper state management.The laptop-specific button layout is cleanly implemented with proper spacing and consistent state cleanup in button actions.
| @@ -106,17 +103,6 @@ export const FacilityHome = ({ facilityId }: Props) => { | |||
| pathParams: { id: facilityId }, | |||
| }), | |||
| }); | |||
| const { mutate: deleteFacility } = useMutation({ | |||
There was a problem hiding this comment.
Let's not remove the deleteFacility logic. Move the confirm dialog to Dialog, and hide the option for now.
…k#10048/Replace-Dialog-and-ConfirmDialog-component-with-ShadCn-ui-component
|
Conflicts have been detected against the base branch. Please merge the base branch into your branch.
|
…g-component-with-ShadCn-ui-component' of https://github.com/NikhilA8606/care_fe into issues/ohcnetwork#10048/Replace-Dialog-and-ConfirmDialog-component-with-ShadCn-ui-component
Jacobjeevan
added
reviewed
|
@NikhilA8606 Your efforts have helped advance digital healthcare and TeleICU systems. 🚀 Thank you for taking the time out to make CARE better. We hope you continue to innovate and contribute; your impact is immense! 🙌 |
Replaced Dialog and ConfirmDialog with Shadcn Dialog component
@rithviknishad
Merge Checklist
Summary by CodeRabbit
Summary by CodeRabbit
Refactor
New Features
Bug Fixes
Chores
ConfirmDialogcomponent, enhancing overall application clarity.DialogModalcomponent, transitioning to a more modular dialog structure.