-
Notifications
You must be signed in to change notification settings - Fork 1k
fix: fixed DialogContent requires a DialogTitle using SheetTitle #15098
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for care-ohc ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
WalkthroughCamera capture dialog UI was reorganized and made permission-aware: a visually hidden SheetTitle was added for accessibility; camera-selection UI now branches on camera permission and device/screen conditions, using a DropdownMenu for multi-device laptop cases and a switch Button otherwise; capture button placement/styling adjusted. Changes
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
🚀 Preview Deployment Ready!
This preview will be automatically updated when you push new commits to this PR. |
Jacobjeevan
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Converted to draft
c972532 to
f0b53d9
Compare
f0b53d9 to
7cca2af
Compare
|
Hi @Jacobjeevan , now the camera controls wont be rendered if the camera permission is denied. here is the ss of the after:
Sorry for the force-pushes... I had realized that my git config was incorrect so first force-push was for that, and after I had mistakenly pushed an md file so I had to remove it with another force-push. |
| {cameraPermission !== "denied" && ( | ||
| <> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is the real change
| <div className="size-16 rounded-full bg-white border-2 border-black flex items-center justify-center"></div> | ||
| </Button> | ||
| </> | ||
| )} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and this is the closing tag
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @src/components/Files/CameraCaptureDialog.tsx:
- Around line 179-191: The camera option divs are not keyboard-accessible;
update the element that uses selectedDeviceId, setSelectedDeviceId, and
setShowCameraSelector to support keyboard activation by adding role="button" and
tabIndex={0}, provide an onKeyDown handler that triggers the same logic as
onClick when Enter or Space is pressed, and include an appropriate ARIA state
(e.g., aria-pressed or aria-selected tied to selectedDeviceId ===
camera.deviceId) so screen readers convey selection; alternatively replace the
div with an accessible component such as DropdownMenuItem from shadcn/ui.
- Around line 231-237: The mobile camera switch Button uses className "size-12"
which mismatches the laptop dropdown trigger's "size-13"; update the mobile
Button in CameraCaptureDialog (the Button that calls handleSwitchCamera and
renders <SwitchCamera />) to use the same size class ("size-13") as the dropdown
trigger (or vice-versa if you prefer smaller), ensuring both controls use an
identical size class for visual consistency.
📜 Review details
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/components/Files/CameraCaptureDialog.tsx
🧰 Additional context used
📓 Path-based instructions (12)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx}: Write concise, technical TypeScript code with accurate examples
Use TypeScript for all code; prefer interfaces over types
Avoid enums; use maps instead
Use TanStack Query for data fetching from the API along with query and mutate utilities for the queryFn and mutationFn
Use raviger for routing
**/*.{ts,tsx}: Use TypeScript with strict mode and ES2022 target
Useinterfacefor defining object types in TypeScript
Avoid explicitanytype in TypeScript
Use proper nullability annotations in TypeScript types
Use dedicated error handlers and TypeScript strict null checks
**/*.{ts,tsx}: Use TypeScript for all new code
Prefer interfaces over types for object definitions in TypeScript
Avoid usinganytype; use proper type definitions in TypeScript
Use type inference where possible in TypeScript
Use TanStack Query for API data management
Prefer React Context for global state management
**/*.{ts,tsx}: Use TanStack Query with thequeryandmutateutilities from@/Utils/request/
Use appropriate query keys following the resource pattern for TanStack Query
Leverage TanStack Query built-in features for pagination and debouncing
Implement proper error handling using the global error handler for TanStack Query operations
UseuseQueryhook withqueryKeyandqueryFnparameters, wherequeryFnwraps the API route with thequery()utility
UseuseMutationhook withmutationFnparameter wrapping API routes with themutate()utility, and implementonSuccesscallbacks to invalidate related queries
Support path parameters in TanStack Query using thepathParamsoption in query/mutate utilities
Support query parameters in TanStack Query using thequeryParamsoption in query/mutate utilities
Use thesilent: trueoption to suppress global error notifications when custom error handling is needed
**/*.{ts,tsx}: Use TypeScript with strict mode and ES2022 target
Useinterfacefor defining object types
Avoid explicitanytypes and maint...
Files:
src/components/Files/CameraCaptureDialog.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx,js,jsx}: Use functional and declarative programming patterns; avoid classes
Prefer iteration and modularization over code duplication
Use descriptive variable names with auxiliary verbs (e.g., isLoading, hasError)
Use the "function" keyword for pure functions
Avoid unnecessary curly braces in conditionals; use concise syntax for simple statements
**/*.{ts,tsx,js,jsx}: Use path aliases for imports from src (@/*)
Use double quotes for strings
Semicolons are required at end of statements
Order imports as: 3rd-party → library → CAREUI → UI → components → hooks → utils → relative
Use PascalCase for component and class names
Use camelCase for variable and function names
**/*.{ts,tsx,js,jsx}: Use path aliases@/*for imports from src directory
Use double quotes for strings
Require semicolons at end of statements
Order imports: 3rd-party → library → CAREUI → UI → components → hooks → utils → relative
Use PascalCase for component and class names
Use camelCase for variable and function names
Files:
src/components/Files/CameraCaptureDialog.tsx
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (.cursorrules)
Use declarative JSX
Files:
src/components/Files/CameraCaptureDialog.tsx
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.{ts,tsx}: Use strict TypeScript configuration for medical data safety in all TypeScript source files
All literal strings must use i18next for multi-language support in healthcare interfaces
Use comprehensive error boundaries and user-friendly error messages for medical workflow debugging without exposing PHI
Follow ESLint configured rules for React hooks, accessibility, and code quality
Use @tanstack/react-query for server state management in API client code
Localize date and time formats for medical timestamps using locale-aware formatting functions
Use date-fns for date handling and manipulation with locale awareness for medical timestamps
Files:
src/components/Files/CameraCaptureDialog.tsx
src/components/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/components/**/*.{ts,tsx}: Implement WCAG 2.1 AA accessibility compliance in medical applications with screen reader compatibility and keyboard navigation
Include medical use cases, accessibility notes, and WCAG compliance documentation in component documentation
Follow established React component patterns and folder structure organized by feature and domain
src/components/**/*.{ts,tsx}: Use path aliases:@/components/,@/types/,@/lib/,@/pages/for imports
Follow import order: External packages →@/components/ui/→@/components/→@/CAREUI/→@/types/→@/lib/→ relative imports
Use named exports fromlucide-reactfor icons (e.g.,import { SettingsIcon } from "lucide-react")
ImportuseTranslationfromreact-i18nextfor internationalization
Use React 19.1.1 hooks pattern - Functional components only
Always define TypeScript Props interfaces (e.g., PatientInfoCardProps) for component props
Use handle prefix for event handlers (e.g., handleSubmit, handleTagsUpdate, handlePatientSelect)
Use shadcn/ui components as primary UI system (Button, Card, Badge, etc.) from@/components/ui/
Use healthcare-specific CAREUI custom components (Calendar, WeekdayCheckbox, Zoom) from@/CAREUI/
UsebuttonVariantsfrom@/components/ui/buttonwith CVA patterns for button styling
Follow<Card><CardHeader>pattern for consistent card layouts
UsePatientReadtype from@/types/emr/patient/patientfor patient data handling
ImplementTagAssignmentSheetwithTagEntityTypefor patient/facility tags
UsePatientHoverCardfor patient info overlays
Use Badge components to display patient status, facility capacity, medication dosage with color variants
Usecva()from class variance authority for variant-based component styling
Usecn()from@/lib/utilsfor conditional class composition
Follow Tailwind CSS 4.1.3 color system: primary-700, gray-900, red-500 pattern with dark mode variants
Use Tailwind shadow system: shadow-sm, shadow-xs for el...
Files:
src/components/Files/CameraCaptureDialog.tsx
src/**/*.{ts,tsx,css}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use Prettier for consistent code formatting across the healthcare application
Files:
src/components/Files/CameraCaptureDialog.tsx
src/**/*.{tsx,ts}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.{tsx,ts}: Use React 19.1.1 features and patterns following React 19 Documentation for healthcare application development
Use shadcn/ui as primary component system and CAREUI for healthcare-specific components
Use framer-motion for animations in healthcare UI interfaces
Files:
src/components/Files/CameraCaptureDialog.tsx
src/**/*.{tsx,css}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use Tailwind CSS 4.1.3 utility classes with custom healthcare-specific design system for styling
Files:
src/components/Files/CameraCaptureDialog.tsx
**/*
📄 CodeRabbit inference engine (CLAUDE.md)
Use 2-space indentation
Files:
src/components/Files/CameraCaptureDialog.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/02-coding-standards.mdc)
**/*.tsx: Use functional components with proper type definitions in React
One component per file is preferred
Prefer default exports for React components
Follow the component naming pattern:ComponentName.tsxfor React components
Use Tailwind CSS for styling
Use Shadcn UI components when available
Use local state for component-specific data
Use PascalCase for component file names (e.g.,AuthWizard.tsx)
Files:
src/components/Files/CameraCaptureDialog.tsx
**/*.{tsx,ts}
📄 CodeRabbit inference engine (.cursor/rules/04-ui-components.mdc)
**/*.{tsx,ts}: Use Shadcn UI components as the primary component library
Follow the component documentation for proper usage
Customize components using Tailwind CSS
UsenavigateanduseRedirectfrom raviger for navigation and redirects
Use Tailwind's responsive classes and follow mobile-first approach
Implement proper ARIA attributes for accessibility
Ensure keyboard navigation works in components
Define component props using TypeScript interfaces
Use translation keys fromsrc/Locale/for internationalization
Support RTL languages in components
Use proper date/time formatting for multiple language support
Files:
src/components/Files/CameraCaptureDialog.tsx
**/*.{ts,tsx,js,jsx,json,css,scss,html}
📄 CodeRabbit inference engine (AGENTS.md)
Use 2-space indentation
Files:
src/components/Files/CameraCaptureDialog.tsx
🧠 Learnings (9)
📓 Common learnings
Learnt from: Rustix69
Repo: ohcnetwork/care_fe PR: 11093
File: src/Utils/imageCaptureUtils.ts:0-0
Timestamp: 2025-04-06T07:20:39.655Z
Learning: A new hook `useImageCapture` has been implemented to handle image processing functionality, replacing the deprecated utility functions in `imageCaptureUtils.ts`. The common image processing logic has been extracted to `imageProcessingCore.ts`.
📚 Learning: 2025-04-06T07:20:39.655Z
Learnt from: Rustix69
Repo: ohcnetwork/care_fe PR: 11093
File: src/Utils/imageCaptureUtils.ts:0-0
Timestamp: 2025-04-06T07:20:39.655Z
Learning: A new hook `useImageCapture` has been implemented to handle image processing functionality, replacing the deprecated utility functions in `imageCaptureUtils.ts`. The common image processing logic has been extracted to `imageProcessingCore.ts`.
Applied to files:
src/components/Files/CameraCaptureDialog.tsx
📚 Learning: 2025-11-25T13:52:51.914Z
Learnt from: CR
Repo: ohcnetwork/care_fe PR: 0
File: .github/instructions/react-components.instructions.md:0-0
Timestamp: 2025-11-25T13:52:51.914Z
Learning: Applies to src/components/**/*.{ts,tsx} : Use shadcn/ui components as primary UI system (Button, Card, Badge, etc.) from `@/components/ui/`
Applied to files:
src/components/Files/CameraCaptureDialog.tsx
📚 Learning: 2025-11-25T13:50:10.786Z
Learnt from: CR
Repo: ohcnetwork/care_fe PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T13:50:10.786Z
Learning: Applies to src/**/*.{tsx,ts} : Use shadcn/ui as primary component system and CAREUI for healthcare-specific components
Applied to files:
src/components/Files/CameraCaptureDialog.tsx
📚 Learning: 2025-11-25T13:50:54.683Z
Learnt from: CR
Repo: ohcnetwork/care_fe PR: 0
File: .github/instructions/common.instructions.md:0-0
Timestamp: 2025-11-25T13:50:54.683Z
Learning: Applies to src/common/**/Permissions.tsx : Support emergency override permissions for critical care situations in Permissions.tsx
Applied to files:
src/components/Files/CameraCaptureDialog.tsx
📚 Learning: 2025-11-25T13:54:35.875Z
Learnt from: CR
Repo: ohcnetwork/care_fe PR: 0
File: .cursor/rules/04-ui-components.mdc:0-0
Timestamp: 2025-11-25T13:54:35.875Z
Learning: Applies to **/*.{tsx,ts} : Use Shadcn UI components as the primary component library
Applied to files:
src/components/Files/CameraCaptureDialog.tsx
📚 Learning: 2025-11-25T13:49:43.065Z
Learnt from: CR
Repo: ohcnetwork/care_fe PR: 0
File: .cursorrules:0-0
Timestamp: 2025-11-25T13:49:43.065Z
Learning: Applies to components/**/*.{ts,tsx} : Use Shadcn UI, Radix, and Tailwind for components and styling
Applied to files:
src/components/Files/CameraCaptureDialog.tsx
📚 Learning: 2025-12-17T19:47:58.152Z
Learnt from: AdityaJ2305
Repo: ohcnetwork/care_fe PR: 14821
File: src/components/Location/LocationNavigation.tsx:161-163
Timestamp: 2025-12-17T19:47:58.152Z
Learning: To ensure the remove-unused-i18n.js cleanup script detects i18n keys, avoid wrapping keys inside a ternary expression passed directly to t(). Instead, call t() separately in each branch of a conditional, e.g. condition ? t("key1") : t("key2"), or compute the key in a variable before passing to t(). For LocationNavigation.tsx (and similar TSX files in the repo), prefer explicit separate i18n calls per branch or a shared precomputed key to guarantee all keys are statically detectable by the script.
Applied to files:
src/components/Files/CameraCaptureDialog.tsx
📚 Learning: 2025-12-22T10:16:36.690Z
Learnt from: Jacobjeevan
Repo: ohcnetwork/care_fe PR: 14804
File: src/pages/Facility/services/pharmacy/DispensesView.tsx:117-135
Timestamp: 2025-12-22T10:16:36.690Z
Learning: In the care_fe repository, prefer the native HTML title attribute for simple tooltips over using shadcn/ui Tooltip components. Use Tooltip components only when the tooltip requires richer content, interactivity, or accessibility enhancements that cannot be achieved with a plain title. For simple cases, add a title attribute to the element (e.g., <span title="...">). If a Tooltip is used, ensure it is accessible and necessary, and avoid duplicating content available in the title attribute.
Applied to files:
src/components/Files/CameraCaptureDialog.tsx
🪛 Biome (2.1.2)
src/components/Files/CameraCaptureDialog.tsx
[error] 179-192: Static Elements should not be interactive.
To add interactivity such as a mouse or key event listener to a static element, give the element an appropriate role value.
(lint/a11y/noStaticElementInteractions)
[error] 179-192: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event.
Actions triggered using mouse events should have corresponding keyboard events to account for keyboard-only navigation.
(lint/a11y/useKeyWithClickEvents)
🔇 Additional comments (3)
src/components/Files/CameraCaptureDialog.tsx (3)
20-20: LGTM! Accessibility fix properly implemented.The
SheetTitlewithsr-onlyclass correctly addresses the accessibility requirement from issue #15004. This provides screen reader users with a dialog title while keeping it visually hidden.Also applies to: 132-132
240-258: LGTM! Permission-aware control rendering.The capture button is correctly hidden when camera permission is denied, while the close button remains accessible. This aligns with the PR objective to not render camera controls when permission is denied.
152-251: All i18n keys used in the camera selection UI are already defined in the locale files. No action needed.
| <div | ||
| key={camera.deviceId} | ||
| className={`p-3 rounded-lg border cursor-pointer transition-all hover:bg-gray-50 ${ | ||
| selectedDeviceId === camera.deviceId | ||
| ? "border-green-500 bg-green-50" | ||
| : "border-gray-200" | ||
| }`} | ||
| onClick={() => { | ||
| setSelectedDeviceId(camera.deviceId); | ||
| setShowCameraSelector( | ||
| !showCameraSelector, | ||
| ); | ||
| }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add keyboard accessibility to camera selection items.
The static analysis correctly identifies that these interactive div elements lack keyboard support. Per WCAG 2.1 AA compliance requirements, add proper keyboard navigation.
♿ Proposed fix for keyboard accessibility
<div
key={camera.deviceId}
+ role="button"
+ tabIndex={0}
className={`p-3 rounded-lg border cursor-pointer transition-all hover:bg-gray-50 ${
selectedDeviceId === camera.deviceId
? "border-green-500 bg-green-50"
: "border-gray-200"
}`}
onClick={() => {
setSelectedDeviceId(camera.deviceId);
setShowCameraSelector(
!showCameraSelector,
);
}}
+ onKeyDown={(e) => {
+ if (e.key === "Enter" || e.key === " ") {
+ e.preventDefault();
+ setSelectedDeviceId(camera.deviceId);
+ setShowCameraSelector(!showCameraSelector);
+ }
+ }}
>Alternatively, consider using DropdownMenuItem from the shadcn/ui dropdown menu components, which provides keyboard accessibility out of the box.
🧰 Tools
🪛 Biome (2.1.2)
[error] 179-192: Static Elements should not be interactive.
To add interactivity such as a mouse or key event listener to a static element, give the element an appropriate role value.
(lint/a11y/noStaticElementInteractions)
[error] 179-192: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event.
Actions triggered using mouse events should have corresponding keyboard events to account for keyboard-only navigation.
(lint/a11y/useKeyWithClickEvents)
🤖 Prompt for AI Agents
In @src/components/Files/CameraCaptureDialog.tsx around lines 179 - 191, The
camera option divs are not keyboard-accessible; update the element that uses
selectedDeviceId, setSelectedDeviceId, and setShowCameraSelector to support
keyboard activation by adding role="button" and tabIndex={0}, provide an
onKeyDown handler that triggers the same logic as onClick when Enter or Space is
pressed, and include an appropriate ARIA state (e.g., aria-pressed or
aria-selected tied to selectedDeviceId === camera.deviceId) so screen readers
convey selection; alternatively replace the div with an accessible component
such as DropdownMenuItem from shadcn/ui.
| {selectedDeviceId === camera.deviceId | ||
| ? t("selected") | ||
| : camera.kind === "videoinput" | ||
| ? t("built_in") | ||
| : t("external")} | ||
| </Badge> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dead code: t("external") branch is unreachable.
Since videoDevices is already filtered to only include device.kind === "videoinput" (line 64), the condition camera.kind === "videoinput" is always true, making the t("external") branch unreachable.
If the intent is to distinguish built-in vs external cameras, consider checking the device label instead:
🔧 Suggested fix
{selectedDeviceId === camera.deviceId
? t("selected")
- : camera.kind === "videoinput"
- ? t("built_in")
- : t("external")}
+ : camera.label.toLowerCase().includes("built-in") ||
+ camera.label.toLowerCase().includes("integrated")
+ ? t("built_in")
+ : t("external")}📝 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.
| {selectedDeviceId === camera.deviceId | |
| ? t("selected") | |
| : camera.kind === "videoinput" | |
| ? t("built_in") | |
| : t("external")} | |
| </Badge> | |
| {selectedDeviceId === camera.deviceId | |
| ? t("selected") | |
| : camera.label.toLowerCase().includes("built-in") || | |
| camera.label.toLowerCase().includes("integrated") | |
| ? t("built_in") | |
| : t("external")} | |
| </Badge> |
| <Button | ||
| variant="secondary" | ||
| onClick={handleSwitchCamera} | ||
| className="rounded-full size-12" | ||
| > | ||
| <SwitchCamera className="size-4" /> | ||
| </Button> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Minor: Button size inconsistency.
The mobile switch button uses size-12 (line 234) while the laptop dropdown trigger uses size-13 (line 163). Consider aligning both to the same size for visual consistency.
🔧 Suggested fix
<Button
variant="secondary"
onClick={handleSwitchCamera}
- className="rounded-full size-12"
+ className="rounded-full size-13"
>
<SwitchCamera className="size-4" />
</Button>🤖 Prompt for AI Agents
In @src/components/Files/CameraCaptureDialog.tsx around lines 231 - 237, The
mobile camera switch Button uses className "size-12" which mismatches the laptop
dropdown trigger's "size-13"; update the mobile Button in CameraCaptureDialog
(the Button that calls handleSwitchCamera and renders <SwitchCamera />) to use
the same size class ("size-13") as the dropdown trigger (or vice-versa if you
prefer smaller), ensuring both controls use an identical size class for visual
consistency.

Proposed Changes
Fixes #15004
SheetTitleinCameraCaptureDialog.tsxSheetTitleinSheetContentwith classsr-onlyto hide it and fix the issue.Tagging: @ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit
Bug Fixes
New Features
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.