Skip to content
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

[Fixed]: Scanning modal remains open and blank when camera access is denied (#329) #406

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

coolnj4
Copy link

@coolnj4 coolnj4 commented Oct 19, 2024

Related Issues

Modal remains blank and open when camera access is denied during product scanning #329
hotwax/dxp-components#329

Short Description and Why It's Useful

Introduced an alert pop-up for the case of camera access permission denied. Added hasCameraAccess to track camera permission status.

Implemented a method to request camera access and show an alert if access is denied. The barcode scanner now only loads when camera access is granted.

Screenshots of Visual Changes before/after (If There Are Any)

Before if camera permission is denied
image

After
image

Contribution and Currently Important Rules Acceptance

…denied (hotwax#329)

Introduced an alert pop-up for the case of camera access permission denied. Added hasCameraAccess to track camera permission status.

Implemented a method to request camera access and show an alert if access is denied. The barcode scanner now only loads when camera access is granted.
@ymaheshwari1
Copy link
Contributor

Thanks for the contribution @coolnj4

PR looks good, but I will suggest some improvements here:

  • We can move the hasCameraAccess check in the parent component from where the Scanner component is triggered, instead of handling it in Scanner component.
  • For all the static text, you can use translate function to internationalize the text. You can refer for the usage of this function on other components/views.

@coolnj4
Copy link
Author

coolnj4 commented Oct 20, 2024

I can't seem to find the parent component from which the Scanner component is being triggered. Can you please guide me through it?

@ymaheshwari1
Copy link
Contributor

ymaheshwari1 commented Oct 21, 2024

I can't seem to find the parent component from which the Scanner component is being triggered. Can you please guide me through it?

Its on the ShipmentDetails, ReturnDetails and PurchaseOrderDetails page. So you need to handle this change in all the three views.

…tatic text for product scanning modal (hotwax#329)

Moved the 'hasCameraAccess' check to the parent components (ShipmentDetails, ReturnDetails, and PurchaseOrderDetails), ensuring consistent handling of camera permissions across all views. Also, updated static text with the translate function to support internationalization, improving usability for global users.
@coolnj4
Copy link
Author

coolnj4 commented Oct 21, 2024

I’ve implemented the suggested changes let me know if there’s anything else you'd like me to adjust!

@@ -268,6 +269,10 @@ export default defineComponent({
this.queryString = ''
},
async scanCode () {
if (!hasCameraAccess()) {
showToast(translate("Camera access is required to scan items."));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As we are having a toast message in the hasCameraAccess function, so we do not need to have another toast here. From here we can simply return the flow without any toast or log.

<div class="scanner">
<!-- Conditionally render the "Start Scanning" button based on camera access -->
<ion-button v-if="!hasCameraAccess" @click="openScanner">Start Scanning</ion-button>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do not need to make any change in this component, because once we have added the check of camera access in the parent component then this component will not open and thus there is no use of having any checks in this component.

try {
const stream = await navigator.mediaDevices.getUserMedia({ video: true });
if (stream) {
showToast(translate('Camera access granted.'));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do not need to display toast when the user has camera permission, because if the camera permission is already provided, then the modal will open up, and then displaying a toast will be a redundant behaviour.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, I think we can remove the stream check from here

@@ -249,6 +250,10 @@ export default defineComponent({
return imageModal.present();
},
async scan() {
if (!hasCameraAccess()) {
showToast(translate("Camera access is required to scan items."));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as ReturnDetails.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants