-
Notifications
You must be signed in to change notification settings - Fork 649
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
Added centralized function to handle camera permission error #10989
base: develop
Are you sure you want to change the base?
Changes from 13 commits
2715dfa
a022ccf
2bc136a
77683cb
ba36501
e6f52e1
51a437d
cfb3b28
10e3214
6a80523
e7fdceb
1addf0d
96a18ea
4ef57ec
ac0c657
8440f02
580d6c3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,32 @@ | ||||||||||
import { useCallback, useRef } from "react"; | ||||||||||
import { useTranslation } from "react-i18next"; | ||||||||||
import { toast } from "sonner"; | ||||||||||
|
||||||||||
export const handleCameraPermission = () => { | ||||||||||
const toastShownRef = useRef(false); | ||||||||||
const { t } = useTranslation(); | ||||||||||
|
||||||||||
const requestPermission = useCallback( | ||||||||||
async (device: "camera", cameraFacingMode: string = "user") => { | ||||||||||
try { | ||||||||||
toastShownRef.current = false; | ||||||||||
const constraints = | ||||||||||
device === "camera" | ||||||||||
? { video: { facingMode: cameraFacingMode } } | ||||||||||
: { audio: true }; | ||||||||||
|
||||||||||
await navigator.mediaDevices.getUserMedia(constraints); | ||||||||||
return true; | ||||||||||
} catch (_error) { | ||||||||||
if (!toastShownRef.current) { | ||||||||||
toastShownRef.current = true; | ||||||||||
toast.warning(t("camera_permission_denied")); | ||||||||||
} | ||||||||||
return false; | ||||||||||
} | ||||||||||
}, | ||||||||||
[], | ||||||||||
); | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Include necessary dependencies in the useCallback dependency array The - [],
+ [t], 📝 Committable suggestion
Suggested change
|
||||||||||
|
||||||||||
return { requestPermission }; | ||||||||||
}; | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent 🧩 Analysis chainConfirm this utility is used consistently across the codebase Based on PR objectives and past reviews, ensure this centralized function is applied in all areas where camera functionality is utilized, particularly in 🏁 Script executed: #!/bin/bash
echo "Finding places still using direct camera access that should use the centralized handler:"
rg -g "*.{ts,tsx}" -A 3 -B 3 "navigator\.mediaDevices\.getUserMedia" | grep -v "cameraPermissionHandler"
echo -e "\nPlaces already using the centralized handler:"
rg -g "*.{ts,tsx}" -A 1 "handleCameraPermission" Length of output: 2244 Ensure consistent use of the centralized camera permission utility Our search confirms that while components like
Please update these sections to uniformly leverage |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,6 +15,8 @@ import { | |
|
||
import useBreakpoints from "@/hooks/useBreakpoints"; | ||
|
||
import { handleCameraPermission } from "@/Utils/cameraPermissionHandler"; | ||
|
||
export interface CameraCaptureDialogProps { | ||
open: boolean; | ||
onOpenChange: (open: boolean) => void; | ||
|
@@ -26,6 +28,8 @@ export interface CameraCaptureDialogProps { | |
export default function CameraCaptureDialog(props: CameraCaptureDialogProps) { | ||
const { open, onOpenChange, onCapture, onResetCapture, setPreview } = props; | ||
const isLaptopScreen = useBreakpoints({ lg: true, default: false }); | ||
const { requestPermission } = handleCameraPermission(); | ||
const [stream, setStream] = useState<MediaStream | null>(null); | ||
|
||
const [cameraFacingMode, setCameraFacingMode] = useState( | ||
isLaptopScreen ? "user" : "environment", | ||
|
@@ -41,17 +45,25 @@ export default function CameraCaptureDialog(props: CameraCaptureDialogProps) { | |
|
||
useEffect(() => { | ||
if (!open) return; | ||
let stream: MediaStream | null = null; | ||
|
||
navigator.mediaDevices | ||
.getUserMedia({ video: { facingMode: cameraFacingMode } }) | ||
.then((mediaStream) => { | ||
stream = mediaStream; | ||
}) | ||
.catch(() => { | ||
toast.warning(t("camera_permission_denied")); | ||
const getCameraStream = async () => { | ||
const hasPermission = await requestPermission("camera", cameraFacingMode); | ||
if (!hasPermission) { | ||
onOpenChange(false); | ||
}); | ||
return; | ||
} | ||
|
||
try { | ||
const mediaStream = await navigator.mediaDevices.getUserMedia({ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you can use the hook, it can return mediaStream and hasPermission. |
||
video: { facingMode: cameraFacingMode }, | ||
}); | ||
setStream(mediaStream); | ||
} catch (error) { | ||
console.error("Error accessing camera:", error); | ||
} | ||
}; | ||
|
||
getCameraStream(); | ||
|
||
return () => { | ||
if (stream) { | ||
|
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.
convert to hook