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

auto-close dialog when no access to the camera #93

Merged
merged 5 commits into from
Jan 5, 2023

Conversation

Viterbo
Copy link
Collaborator

@Viterbo Viterbo commented Jan 2, 2023

Fixes #29

Description

The dialog did not react to the denial of access to the camera remaining open and useless. The user had to refresh to come back to the balances page. This PR fixes that idle behavior by closing the dialog when non-access to the camera is detected, showing to the user an appropriate notification message.

Test notes

To test this new behavior you need to follow these steps:

  • clean any pre-set configuration to access the camera (forget your previous answer)
  • Click on the QR button (center between Send and Receive buttons)
  • A dialog opens with a loading spinner (this is new)
  • A browser dialog prompts if you allow the site to access the camera.
  • Click on Deny.
  • The dialog should close automatically and an error message should be shown asking for approval.

##screenshots

image

image

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have cleaned up the code in the areas my change touches
  • My changes generate no new warnings
  • Any dependent changes have been merged and published in downstream modules
  • I have checked my code and corrected any misspellings
  • I have removed any unnecessary console messages

@Viterbo Viterbo self-assigned this Jan 2, 2023
@netlify
Copy link

netlify bot commented Jan 2, 2023

Deploy Preview for wallet-staging ready!

Name Link
🔨 Latest commit fa13c6c
🔍 Latest deploy log https://app.netlify.com/sites/wallet-staging/deploys/63b74e6e0585c200090ff833
😎 Deploy Preview https://deploy-preview-93--wallet-staging.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@netlify
Copy link

netlify bot commented Jan 2, 2023

Deploy Preview for wallet-testnet ready!

Name Link
🔨 Latest commit fa13c6c
🔍 Latest deploy log https://app.netlify.com/sites/wallet-testnet/deploys/63b74e6efb7d1e0008b1552b
😎 Deploy Preview https://deploy-preview-93--wallet-testnet.netlify.app/
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@Viterbo Viterbo requested review from ezra-sg and karynemayer January 2, 2023 06:47
watch: {
showDlg() {
if (this.showDlg) {
setTimeout(async () => { this.activateQR = true; }, 250);
Copy link
Contributor

Choose a reason for hiding this comment

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

arrow function doesn't need to be async as it has no async code

Copy link
Contributor

@ezra-sg ezra-sg left a comment

Choose a reason for hiding this comment

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

looks good, i like the different error messages 👍 left an optional comment

Copy link
Contributor

@karynemayer karynemayer left a comment

Choose a reason for hiding this comment

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

yarn.lock should not be deleted

@Viterbo Viterbo force-pushed the 29-lingering-scan-access-dialog branch from b60b481 to ea6a2aa Compare January 4, 2023 20:29
@donnyquixotic donnyquixotic merged commit a4ead7b into develop Jan 5, 2023
@donnyquixotic donnyquixotic deleted the 29-lingering-scan-access-dialog branch January 5, 2023 22:29
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.

4 participants