From baa45d935540601c07977e4d786e0953165cf1fa Mon Sep 17 00:00:00 2001 From: Dominik Guzei Date: Fri, 8 Mar 2019 17:34:27 +0100 Subject: Fix/service webview unmounting (#1328) * detach service when underlying webview unmounts * disable no-param-reassign eslint rule --- .eslintrc | 1 + src/actions/service.js | 4 + src/components/services/content/ServiceView.js | 136 ++++++++++++++++++++ src/components/services/content/ServiceWebview.js | 145 ++++------------------ src/components/services/content/Services.js | 7 +- src/containers/layout/AppLayoutContainer.js | 3 + src/stores/ServicesStore.js | 6 + 7 files changed, 180 insertions(+), 122 deletions(-) create mode 100644 src/components/services/content/ServiceView.js diff --git a/.eslintrc b/.eslintrc index e15148e96..743946d35 100644 --- a/.eslintrc +++ b/.eslintrc @@ -2,6 +2,7 @@ "parser": "babel-eslint", "extends": "eslint-config-airbnb", "rules": { + "no-param-reassign": 0, "import/extensions": 0, "import/no-extraneous-dependencies": 0, "import/no-unresolved": [2, { diff --git a/src/actions/service.js b/src/actions/service.js index 5d483b12a..ceaabc31e 100644 --- a/src/actions/service.js +++ b/src/actions/service.js @@ -1,4 +1,5 @@ import PropTypes from 'prop-types'; +import ServiceModel from '../models/Service'; export default { setActive: { @@ -36,6 +37,9 @@ export default { serviceId: PropTypes.string.isRequired, webview: PropTypes.object.isRequired, }, + detachService: { + service: PropTypes.instanceOf(ServiceModel).isRequired, + }, focusService: { serviceId: PropTypes.string.isRequired, }, diff --git a/src/components/services/content/ServiceView.js b/src/components/services/content/ServiceView.js new file mode 100644 index 000000000..5afc54f9d --- /dev/null +++ b/src/components/services/content/ServiceView.js @@ -0,0 +1,136 @@ +import React, { Component, Fragment } from 'react'; +import PropTypes from 'prop-types'; +import { autorun } from 'mobx'; +import { observer } from 'mobx-react'; +import classnames from 'classnames'; + +import ServiceModel from '../../../models/Service'; +import StatusBarTargetUrl from '../../ui/StatusBarTargetUrl'; +import WebviewLoader from '../../ui/WebviewLoader'; +import WebviewCrashHandler from './WebviewCrashHandler'; +import WebviewErrorHandler from './ErrorHandlers/WebviewErrorHandler'; +import ServiceDisabled from './ServiceDisabled'; +import ServiceWebview from './ServiceWebview'; + +export default @observer class ServiceView extends Component { + static propTypes = { + service: PropTypes.instanceOf(ServiceModel).isRequired, + setWebviewReference: PropTypes.func.isRequired, + detachService: PropTypes.func.isRequired, + reload: PropTypes.func.isRequired, + edit: PropTypes.func.isRequired, + enable: PropTypes.func.isRequired, + isActive: PropTypes.bool, + }; + + static defaultProps = { + isActive: false, + }; + + state = { + forceRepaint: false, + targetUrl: '', + statusBarVisible: false, + }; + + autorunDisposer = null; + + componentDidMount() { + this.autorunDisposer = autorun(() => { + if (this.props.service.isActive) { + this.setState({ forceRepaint: true }); + setTimeout(() => { + this.setState({ forceRepaint: false }); + }, 100); + } + }); + } + + componentWillUnmount() { + this.autorunDisposer(); + } + + updateTargetUrl = (event) => { + let visible = true; + if (event.url === '' || event.url === '#') { + visible = false; + } + this.setState({ + targetUrl: event.url, + statusBarVisible: visible, + }); + }; + + render() { + const { + detachService, + service, + setWebviewReference, + reload, + edit, + enable, + } = this.props; + + const webviewClasses = classnames({ + services__webview: true, + 'services__webview-wrapper': true, + 'is-active': service.isActive, + 'services__webview--force-repaint': this.state.forceRepaint, + }); + + let statusBar = null; + if (this.state.statusBarVisible) { + statusBar = ( + + ); + } + + return ( +
+ {service.isActive && service.isEnabled && ( + + {service.hasCrashed && ( + + )} + {service.isEnabled && service.isLoading && service.isFirstLoad && ( + + )} + {service.isError && ( + + )} + + )} + {!service.isEnabled ? ( + + {service.isActive && ( + + )} + + ) : ( + + )} + {statusBar} +
+ ); + } +} diff --git a/src/components/services/content/ServiceWebview.js b/src/components/services/content/ServiceWebview.js index bb577e4cc..7252c695f 100644 --- a/src/components/services/content/ServiceWebview.js +++ b/src/components/services/content/ServiceWebview.js @@ -1,145 +1,50 @@ -import React, { Component, Fragment } from 'react'; +import React, { Component } from 'react'; import PropTypes from 'prop-types'; -import { autorun } from 'mobx'; import { observer } from 'mobx-react'; -import Webview from 'react-electron-web-view'; -import classnames from 'classnames'; +import ElectronWebView from 'react-electron-web-view'; import ServiceModel from '../../../models/Service'; -import StatusBarTargetUrl from '../../ui/StatusBarTargetUrl'; -import WebviewLoader from '../../ui/WebviewLoader'; -import WebviewCrashHandler from './WebviewCrashHandler'; -import WebviewErrorHandler from './ErrorHandlers/WebviewErrorHandler'; -import ServiceDisabled from './ServiceDisabled'; -export default @observer class ServiceWebview extends Component { +@observer +class ServiceWebview extends Component { static propTypes = { service: PropTypes.instanceOf(ServiceModel).isRequired, setWebviewReference: PropTypes.func.isRequired, - reload: PropTypes.func.isRequired, - edit: PropTypes.func.isRequired, - enable: PropTypes.func.isRequired, - isActive: PropTypes.bool, + detachService: PropTypes.func.isRequired, }; - static defaultProps = { - isActive: false, - }; - - state = { - forceRepaint: false, - targetUrl: '', - statusBarVisible: false, - }; - - autorunDisposer = null; - webview = null; - componentDidMount() { - this.autorunDisposer = autorun(() => { - if (this.props.service.isActive) { - this.setState({ forceRepaint: true }); - setTimeout(() => { - this.setState({ forceRepaint: false }); - }, 100); - } - }); - } - componentWillUnmount() { - this.autorunDisposer(); - } - - updateTargetUrl = (event) => { - let visible = true; - if (event.url === '' || event.url === '#') { - visible = false; - } - this.setState({ - targetUrl: event.url, - statusBarVisible: visible, - }); + const { service, detachService } = this.props; + detachService({ service }); } render() { const { service, setWebviewReference, - reload, - edit, - enable, } = this.props; - const webviewClasses = classnames({ - services__webview: true, - 'services__webview-wrapper': true, - 'is-active': service.isActive, - 'services__webview--force-repaint': this.state.forceRepaint, - }); - - let statusBar = null; - if (this.state.statusBarVisible) { - statusBar = ( - - ); - } - return ( -
- {service.isActive && service.isEnabled && ( - - {service.hasCrashed && ( - - )} - {service.isEnabled && service.isLoading && service.isFirstLoad && ( - - )} - {service.isError && ( - - )} - - )} - {!service.isEnabled ? ( - - {service.isActive && ( - - )} - - ) : ( - { this.webview = element; }} - autosize - src={service.url} - preload="./webview/recipe.js" - partition={`persist:service-${service.id}`} - onDidAttach={() => setWebviewReference({ - serviceId: service.id, - webview: this.webview.view, - })} - onUpdateTargetUrl={this.updateTargetUrl} - useragent={service.userAgent} - allowpopups - /> - )} - {statusBar} -
+ { this.webview = webview; }} + autosize + src={service.url} + preload="./webview/recipe.js" + partition={`persist:service-${service.id}`} + onDidAttach={() => { + setWebviewReference({ + serviceId: service.id, + webview: this.webview.view, + }); + }} + onUpdateTargetUrl={this.updateTargetUrl} + useragent={service.userAgent} + allowpopups + /> ); } } + +export default ServiceWebview; diff --git a/src/components/services/content/Services.js b/src/components/services/content/Services.js index 54f16ba12..8f8c38a11 100644 --- a/src/components/services/content/Services.js +++ b/src/components/services/content/Services.js @@ -4,7 +4,7 @@ import { observer, PropTypes as MobxPropTypes } from 'mobx-react'; import { Link } from 'react-router'; import { defineMessages, intlShape } from 'react-intl'; -import Webview from './ServiceWebview'; +import ServiceView from './ServiceView'; import Appear from '../../ui/effects/Appear'; const messages = defineMessages({ @@ -22,6 +22,7 @@ export default @observer class Services extends Component { static propTypes = { services: MobxPropTypes.arrayOrObservableArray, setWebviewReference: PropTypes.func.isRequired, + detachService: PropTypes.func.isRequired, handleIPCMessage: PropTypes.func.isRequired, openWindow: PropTypes.func.isRequired, reload: PropTypes.func.isRequired, @@ -42,6 +43,7 @@ export default @observer class Services extends Component { services, handleIPCMessage, setWebviewReference, + detachService, openWindow, reload, openSettings, @@ -71,11 +73,12 @@ export default @observer class Services extends Component { )} {services.map(service => ( - reload({ serviceId: service.id })} edit={() => openSettings({ path: `services/edit/${service.id}` })} diff --git a/src/containers/layout/AppLayoutContainer.js b/src/containers/layout/AppLayoutContainer.js index 749912c59..5a05ce431 100644 --- a/src/containers/layout/AppLayoutContainer.js +++ b/src/containers/layout/AppLayoutContainer.js @@ -42,6 +42,7 @@ export default @inject('stores', 'actions') @observer class AppLayoutContainer e setActive, handleIPCMessage, setWebviewReference, + detachService, openWindow, reorder, reload, @@ -105,6 +106,7 @@ export default @inject('stores', 'actions') @observer class AppLayoutContainer e services={services.allDisplayedUnordered} handleIPCMessage={handleIPCMessage} setWebviewReference={setWebviewReference} + detachService={detachService} openWindow={openWindow} reload={reload} openSettings={openSettings} @@ -160,6 +162,7 @@ AppLayoutContainer.wrappedComponent.propTypes = { toggleAudio: PropTypes.func.isRequired, handleIPCMessage: PropTypes.func.isRequired, setWebviewReference: PropTypes.func.isRequired, + detachService: PropTypes.func.isRequired, openWindow: PropTypes.func.isRequired, reloadUpdatedServices: PropTypes.func.isRequired, updateService: PropTypes.func.isRequired, diff --git a/src/stores/ServicesStore.js b/src/stores/ServicesStore.js index c63bef196..69e616f0c 100644 --- a/src/stores/ServicesStore.js +++ b/src/stores/ServicesStore.js @@ -44,6 +44,7 @@ export default class ServicesStore extends Store { this.actions.service.deleteService.listen(this._deleteService.bind(this)); this.actions.service.clearCache.listen(this._clearCache.bind(this)); this.actions.service.setWebviewReference.listen(this._setWebviewReference.bind(this)); + this.actions.service.detachService.listen(this._detachService.bind(this)); this.actions.service.focusService.listen(this._focusService.bind(this)); this.actions.service.focusActiveService.listen(this._focusActiveService.bind(this)); this.actions.service.toggleService.listen(this._toggleService.bind(this)); @@ -341,6 +342,11 @@ export default class ServicesStore extends Store { service.isAttached = true; } + @action _detachService({ service }) { + service.webview = null; + service.isAttached = false; + } + @action _focusService({ serviceId }) { const service = this.one(serviceId); -- cgit v1.2.3-70-g09d2