From dcc7d295118ee1557850a41af118a3efa5df0287 Mon Sep 17 00:00:00 2001 From: Mark Stacey Date: Thu, 13 Feb 2020 16:07:50 -0400 Subject: [PATCH] Refactor QR scanner to move all error handling within component (#7885) The QR scanner component error handling would sometimes redirect the user to the wrong page. It was also confusingly handled in two places; the action used to open the QR scanner, and the scanner component. The error handling has now been corrected, simplified, and integrated into the QR scanner component itself. The old way of handling an error within the component was to close the modal, then call the action to open it all over again. This action took a route parameter, which instructed the action on which route to open if the fullscreen UI needed to be opened (as the fullscreen UI is the only one where the browser will show the camera permission prompt). This redirection worked just fine for handling the initial opening of the QR scanner modal. But for any subsequent errors the same action was used with a _default route_, meaning the user could click "try again" and find themselves on a completely different screen. Instead, errors now trigger a state change instead of closing and re- opening the modal. The permission checks in the action have been integrated into the component as well. One functional change is that the scenario where you have an invalid QR code has been made an error. Previously this just showed the error message below the video preview, but left the user with no way to try again. There error page has a "Try again" button, so it seemed better suited as an error. Also the message literally started with "Error:". Another functional change is that _all_ errors during initialization will result in the error UI being shown. Previously there was one error case that would instead log to the console and leave the user stuck. --- .../modals/qr-scanner/qr-scanner.component.js | 261 +++++++++++------- .../modals/qr-scanner/qr-scanner.container.js | 15 +- ui/app/pages/send/send.container.js | 6 +- .../add-contact/add-contact.container.js | 5 +- ui/app/store/actions.js | 23 +- 5 files changed, 168 insertions(+), 142 deletions(-) diff --git a/ui/app/components/app/modals/qr-scanner/qr-scanner.component.js b/ui/app/components/app/modals/qr-scanner/qr-scanner.component.js index 1537c0f52..0bd4ac79a 100644 --- a/ui/app/components/app/modals/qr-scanner/qr-scanner.component.js +++ b/ui/app/components/app/modals/qr-scanner/qr-scanner.component.js @@ -1,99 +1,139 @@ import React, { Component } from 'react' import PropTypes from 'prop-types' +import log from 'loglevel' import { BrowserQRCodeReader } from '@zxing/library' +import { getEnvironmentType } from '../../../../../../app/scripts/lib/util' +import { ENVIRONMENT_TYPE_FULLSCREEN } from '../../../../../../app/scripts/lib/enums' import Spinner from '../../../ui/spinner' import WebcamUtils from '../../../../../lib/webcam-utils' import PageContainerFooter from '../../../ui/page-container/page-container-footer/page-container-footer.component' +const READY_STATE = { + ACCESSING_CAMERA: 'ACCESSING_CAMERA', + NEED_TO_ALLOW_ACCESS: 'NEED_TO_ALLOW_ACCESS', + READY: 'READY', +} + export default class QrScanner extends Component { static propTypes = { hideModal: PropTypes.func.isRequired, - qrCodeDetected: PropTypes.func, - scanQrCode: PropTypes.func, - error: PropTypes.bool, - errorType: PropTypes.string, + qrCodeDetected: PropTypes.func.isRequired, } static contextTypes = { t: PropTypes.func, } - constructor (props, context) { + constructor (props) { super(props) - this.state = { - ready: false, - msg: context.t('accessingYourCamera'), - } + this.state = this.getInitialState() this.codeReader = null this.permissionChecker = null - this.needsToReinit = false + this.mounted = false // Clear pre-existing qr code data before scanning this.props.qrCodeDetected(null) } componentDidMount () { + this.mounted = true + this.checkEnvironment() + } + + componentDidUpdate (_, prevState) { + const { ready } = this.state + + if (prevState.ready !== ready) { + if (ready === READY_STATE.READY) { + this.initCamera() + } else if (ready === READY_STATE.NEED_TO_ALLOW_ACCESS) { + this.checkPermissions() + } + } + } + + getInitialState () { + return { + ready: READY_STATE.ACCESSING_CAMERA, + error: null, + } + } + + checkEnvironment = async () => { + try { + const { environmentReady } = await WebcamUtils.checkStatus() + if (!environmentReady && getEnvironmentType() !== ENVIRONMENT_TYPE_FULLSCREEN) { + const currentUrl = new URL(window.location.href) + const currentHash = currentUrl.hash + const currentRoute = currentHash + ? currentHash.substring(1) + : null + global.platform.openExtensionInBrowser(currentRoute) + } + } catch (error) { + if (this.mounted) { + this.setState({ error }) + } + } + // initial attempt is required to trigger permission prompt this.initCamera() } - async checkPermisisions () { - const { permissions } = await WebcamUtils.checkStatus() - if (permissions) { - clearTimeout(this.permissionChecker) - // Let the video stream load first... - setTimeout(_ => { - this.setState({ - ready: true, - msg: this.context.t('scanInstructions'), - }) - if (this.needsToReinit) { - this.initCamera() - this.needsToReinit = false + checkPermissions = async () => { + try { + const { permissions } = await WebcamUtils.checkStatus() + if (permissions) { + // Let the video stream load first... + await new Promise(resolve => setTimeout(resolve, 2000)) + if (!this.mounted) { + return } - }, 2000) - } else { - // Keep checking for permissions - this.permissionChecker = setTimeout(_ => { - this.checkPermisisions() - }, 1000) + this.setState({ ready: READY_STATE.READY }) + } else if (this.mounted) { + // Keep checking for permissions + this.permissionChecker = setTimeout(this.checkPermissions, 1000) + } + } catch (error) { + if (this.mounted) { + this.setState({ error }) + } } } componentWillUnmount () { + this.mounted = false clearTimeout(this.permissionChecker) if (this.codeReader) { this.codeReader.reset() } } - initCamera () { + initCamera = async () => { this.codeReader = new BrowserQRCodeReader() - this.codeReader.getVideoInputDevices() - .then(() => { - clearTimeout(this.permissionChecker) - this.checkPermisisions() - this.codeReader.decodeFromInputVideoDevice(undefined, 'video') - .then(content => { - const result = this.parseContent(content.text) - if (result.type !== 'unknown') { - this.props.qrCodeDetected(result) - this.stopAndClose() - } else { - this.setState({ msg: this.context.t('unknownQrCode') }) - } - }) - .catch(err => { - if (err && err.name === 'NotAllowedError') { - this.setState({ msg: this.context.t('youNeedToAllowCameraAccess') }) - clearTimeout(this.permissionChecker) - this.needsToReinit = true - this.checkPermisisions() - } - }) - }).catch(err => { - console.error('[QR-SCANNER]: getVideoInputDevices threw an exception: ', err) - }) + try { + await this.codeReader.getVideoInputDevices() + const content = await this.codeReader.decodeFromInputVideoDevice(undefined, 'video') + const result = this.parseContent(content.text) + if (!this.mounted) { + return + } else if (result.type !== 'unknown') { + this.props.qrCodeDetected(result) + this.stopAndClose() + } else { + this.setState({ error: new Error(this.context.t('unknownQrCode')) }) + } + } catch (error) { + if (!this.mounted) { + return + } + if (error.name === 'NotAllowedError') { + log.info(`Permission denied: '${error}'`) + this.setState({ ready: READY_STATE.NEED_TO_ALLOW_ACCESS }) + } else { + this.setState({ error }) + } + } } parseContent (content) { @@ -126,89 +166,108 @@ export default class QrScanner extends Component { if (this.codeReader) { this.codeReader.reset() } - this.setState({ ready: false }) this.props.hideModal() } tryAgain = () => { - // close the modal - this.stopAndClose() - // wait for the animation and try again - setTimeout(_ => { - this.props.scanQrCode() - }, 1000) + clearTimeout(this.permissionChecker) + if (this.codeReader) { + this.codeReader.reset() + } + this.setState(this.getInitialState(), () => { + this.checkEnvironment() + }) } - renderVideo () { - return ( -
-
- ) - } + renderError () { + const { t } = this.context + const { error } = this.state - renderErrorModal () { let title, msg - - if (this.props.error) { - if (this.props.errorType === 'NO_WEBCAM_FOUND') { - title = this.context.t('noWebcamFoundTitle') - msg = this.context.t('noWebcamFound') - } else { - title = this.context.t('unknownCameraErrorTitle') - msg = this.context.t('unknownCameraError') - } + if (error.type === 'NO_WEBCAM_FOUND') { + title = t('noWebcamFoundTitle') + msg = t('noWebcamFound') + } else if (error.message === t('unknownQrCode')) { + msg = t('unknownQrCode') + } else { + title = t('unknownCameraErrorTitle') + msg = t('unknownCameraError') } return ( -
-
- + <>
-
- { title } -
+ { + title + ? ( +
+ { title } +
+ ) + : null + }
{msg}
-
+ ) } - render () { + renderVideo () { const { t } = this.context + const { ready } = this.state - if (this.props.error) { - return this.renderErrorModal() + let message + if (ready === READY_STATE.ACCESSING_CAMERA) { + message = t('accessingYourCamera') + } else if (ready === READY_STATE.READY) { + message = t('scanInstructions') + } else if (ready === READY_STATE.NEED_TO_ALLOW_ACCESS) { + message = t('youNeedToAllowCameraAccess') } return ( -
-
+ <>
{ `${t('scanQrCode')}` }
- { this.renderVideo() } +
+
- {this.state.msg} + {message}
+ + ) + } + + render () { + const { error } = this.state + return ( +
+
+ { + error + ? this.renderError() + : this.renderVideo() + }
) } diff --git a/ui/app/components/app/modals/qr-scanner/qr-scanner.container.js b/ui/app/components/app/modals/qr-scanner/qr-scanner.container.js index d553d1b76..2de4aeb73 100644 --- a/ui/app/components/app/modals/qr-scanner/qr-scanner.container.js +++ b/ui/app/components/app/modals/qr-scanner/qr-scanner.container.js @@ -1,24 +1,13 @@ import { connect } from 'react-redux' import QrScanner from './qr-scanner.component' -import { hideModal, qrCodeDetected, showQrScanner } from '../../../../store/actions' -import { - SEND_ROUTE, -} from '../../../../helpers/constants/routes' - -const mapStateToProps = state => { - return { - error: state.appState.modal.modalState.props.error, - errorType: state.appState.modal.modalState.props.errorType, - } -} +import { hideModal, qrCodeDetected } from '../../../../store/actions' const mapDispatchToProps = dispatch => { return { hideModal: () => dispatch(hideModal()), qrCodeDetected: (data) => dispatch(qrCodeDetected(data)), - scanQrCode: () => dispatch(showQrScanner(SEND_ROUTE)), } } -export default connect(mapStateToProps, mapDispatchToProps)(QrScanner) +export default connect(null, mapDispatchToProps)(QrScanner) diff --git a/ui/app/pages/send/send.container.js b/ui/app/pages/send/send.container.js index 7d0aabe66..da2b66f2f 100644 --- a/ui/app/pages/send/send.container.js +++ b/ui/app/pages/send/send.container.js @@ -53,10 +53,6 @@ import { isValidENSAddress, } from '../../helpers/utils/util' -import { - SEND_ROUTE, -} from '../../helpers/constants/routes' - function mapStateToProps (state) { return { addressBook: getAddressBook(state), @@ -111,7 +107,7 @@ function mapDispatchToProps (dispatch) { }, updateSendErrors: newError => dispatch(updateSendErrors(newError)), resetSendState: () => dispatch(resetSendState()), - scanQrCode: () => dispatch(showQrScanner(SEND_ROUTE)), + scanQrCode: () => dispatch(showQrScanner()), qrCodeDetected: (data) => dispatch(qrCodeDetected(data)), updateSendTo: (to, nickname) => dispatch(updateSendTo(to, nickname)), fetchBasicGasEstimates: () => dispatch(fetchBasicGasEstimates()), diff --git a/ui/app/pages/settings/contact-list-tab/add-contact/add-contact.container.js b/ui/app/pages/settings/contact-list-tab/add-contact/add-contact.container.js index 10e2ab653..d6757f76f 100644 --- a/ui/app/pages/settings/contact-list-tab/add-contact/add-contact.container.js +++ b/ui/app/pages/settings/contact-list-tab/add-contact/add-contact.container.js @@ -3,9 +3,6 @@ import { compose } from 'recompose' import { connect } from 'react-redux' import { withRouter } from 'react-router-dom' import { addToAddressBook, showQrScanner, qrCodeDetected } from '../../../../store/actions' -import { - CONTACT_ADD_ROUTE, -} from '../../../../helpers/constants/routes' import { getQrCodeData, } from '../../../send/send.selectors' @@ -19,7 +16,7 @@ const mapStateToProps = state => { const mapDispatchToProps = dispatch => { return { addToAddressBook: (recipient, nickname) => dispatch(addToAddressBook(recipient, nickname)), - scanQrCode: () => dispatch(showQrScanner(CONTACT_ADD_ROUTE)), + scanQrCode: () => dispatch(showQrScanner()), qrCodeDetected: (data) => dispatch(qrCodeDetected(data)), } } diff --git a/ui/app/store/actions.js b/ui/app/store/actions.js index 5dad5deb8..855944f5d 100644 --- a/ui/app/store/actions.js +++ b/ui/app/store/actions.js @@ -12,7 +12,6 @@ import log from 'loglevel' import { ENVIRONMENT_TYPE_NOTIFICATION } from '../../../app/scripts/lib/enums' import { hasUnconfirmedTransactions } from '../helpers/utils/confirm-tx.util' import { setCustomGasLimit } from '../ducks/gas/gas.duck' -import WebcamUtils from '../../lib/webcam-utils' import txHelper from '../../lib/tx-helper' export const actionConstants = { @@ -550,25 +549,11 @@ export function unlockHardwareWalletAccount (index, deviceName, hdPath) { } } -export function showQrScanner (ROUTE) { +export function showQrScanner () { return (dispatch) => { - return WebcamUtils.checkStatus() - .then(status => { - if (!status.environmentReady) { - // We need to switch to fullscreen mode to ask for permission - global.platform.openExtensionInBrowser(`${ROUTE}`, `scan=true`) - } else { - dispatch(showModal({ - name: 'QR_SCANNER', - })) - } - }).catch(e => { - dispatch(showModal({ - name: 'QR_SCANNER', - error: true, - errorType: e.type, - })) - }) + dispatch(showModal({ + name: 'QR_SCANNER', + })) } }