From eeb2e0c54698f24eb31fe4761c7996b763b5380f Mon Sep 17 00:00:00 2001 From: wiadev Date: Wed, 16 Aug 2017 19:36:38 +0200 Subject: [PATCH] Checked for memory leaks, event handlers cleanup #1071 --- src/components/app/Modal.js | 73 +++++++++++--------- src/components/app/QuickActions.js | 18 +++-- src/components/app/RawModal.js | 26 ++++--- src/components/dashboard/DraggableWrapper.js | 44 +++++++----- src/components/header/Header.js | 12 +++- src/components/header/MenuOverlay.js | 26 ++++--- src/components/widget/List/List.js | 18 +++-- src/components/widget/List/RawList.js | 12 ++++ src/components/widget/MasterWidget.js | 16 +++-- src/containers/MasterWindow.js | 16 +++-- 10 files changed, 172 insertions(+), 89 deletions(-) diff --git a/src/components/app/Modal.js b/src/components/app/Modal.js index c1bb24436..2156c93c6 100644 --- a/src/components/app/Modal.js +++ b/src/components/app/Modal.js @@ -55,16 +55,11 @@ class Modal extends Component { // but we have to change scope of scrollbar document.body.style.overflow = 'hidden'; - const modalContent = document.querySelector('.js-panel-modal-content') - - modalContent && - modalContent.addEventListener('scroll', this.handleScroll); + this.initEventListeners(); } componentWillUnmount() { - const modalContent = document.querySelector('.js-panel-modal-content') - modalContent && - modalContent.removeEventListener('scroll', this.handleScroll); + this.removeEventListeners(); } componentDidUpdate (prevProps) { @@ -102,6 +97,22 @@ class Modal extends Component { return { shortcuts: shortcutManager } } + initEventListeners = () => { + const modalContent = document.querySelector('.js-panel-modal-content'); + + if (modalContent) { + modalContent.addEventListener('scroll', this.handleScroll); + } + } + + removeEventListeners = () => { + const modalContent = document.querySelector('.js-panel-modal-content') + + if (modalContent) { + modalContent.removeEventListener('scroll', this.handleScroll); + } + } + init = () => { const { dispatch, windowType, dataId, tabId, rowId, modalType, selected, @@ -138,20 +149,6 @@ class Modal extends Component { } } - handleClose = () => { - const { - modalSaveStatus, modalType - } = this.props; - - if(modalType === 'process') { - return this.closeModal(); - } - - if(modalSaveStatus || window.confirm('Do you really want to leave?')){ - this.closeModal(); - } - } - closeModal = () => { const { dispatch, closeCallback, dataId, windowType, relativeType, @@ -176,6 +173,30 @@ class Modal extends Component { } } + removeModal = () => { + const {dispatch, rawModalVisible} = this.props; + + dispatch(closeModal()); + + if (!rawModalVisible){ + document.body.style.overflow = 'auto'; + } + } + + handleClose = () => { + const { + modalSaveStatus, modalType + } = this.props; + + if(modalType === 'process') { + return this.closeModal(); + } + + if(modalSaveStatus || window.confirm('Do you really want to leave?')){ + this.closeModal(); + } + } + handleScroll = (event) => { const target = event.srcElement; let scrollTop = target && target.body.scrollTop; @@ -229,16 +250,6 @@ class Modal extends Component { }); } - removeModal = () => { - const {dispatch, rawModalVisible} = this.props; - - dispatch(closeModal()); - - if (!rawModalVisible){ - document.body.style.overflow = 'auto'; - } - } - renderModalBody = () => { const { data, layout, tabId, rowId, dataId, modalType, windowType, diff --git a/src/components/app/QuickActions.js b/src/components/app/QuickActions.js index 489b45b55..582c4a5ad 100644 --- a/src/components/app/QuickActions.js +++ b/src/components/app/QuickActions.js @@ -25,12 +25,7 @@ class QuickActions extends Component { constructor(props){ super(props); - this.state = { - actions: [], - isDropdownOpen: false, - isTooltip: false, - loading: false - } + this.clearComponentState(); const {fetchOnInit} = this.props; @@ -40,6 +35,8 @@ class QuickActions extends Component { } componentDidMount = () => { + this.clearComponentState(); + this.mounted = true; } @@ -94,6 +91,15 @@ class QuickActions extends Component { } } + clearComponentState = () => { + this.state = { + actions: [], + isDropdownOpen: false, + isTooltip: false, + loading: false + } + } + getChildContext = () => { return { shortcuts: shortcutManager } } diff --git a/src/components/app/RawModal.js b/src/components/app/RawModal.js index 9c286b410..9d75820b5 100644 --- a/src/components/app/RawModal.js +++ b/src/components/app/RawModal.js @@ -37,17 +37,11 @@ class RawModal extends Component { // but we have to change scope of scrollbar document.body.style.overflow = 'hidden'; - const modalContent = document.querySelector('.js-panel-modal-content') - - modalContent && - modalContent.addEventListener('scroll', this.handleScroll); + this.initEventListeners(); } componentWillUnmount() { - const modalContent = document.querySelector('.js-panel-modal-content'); - - modalContent && - modalContent.removeEventListener('scroll', this.handleScroll); + this.removeEventListeners(); } toggleTooltip = (visible) => { @@ -60,6 +54,22 @@ class RawModal extends Component { return { shortcuts: shortcutManager } } + initEventListeners = () => { + const modalContent = document.querySelector('.js-panel-modal-content') + + if (modalContent) { + modalContent.addEventListener('scroll', this.handleScroll); + } + } + + removeEventListeners = () => { + const modalContent = document.querySelector('.js-panel-modal-content'); + + if (modalContent) { + modalContent.removeEventListener('scroll', this.handleScroll); + } + } + handleScroll = (event) => { const scrollTop = event.srcElement.scrollTop; diff --git a/src/components/dashboard/DraggableWrapper.js b/src/components/dashboard/DraggableWrapper.js index 4130863f3..377f88b35 100644 --- a/src/components/dashboard/DraggableWrapper.js +++ b/src/components/dashboard/DraggableWrapper.js @@ -37,34 +37,24 @@ import { export class DraggableWrapper extends Component { constructor(props) { super(props); - this.state = { - cards: [], - indicators: [], - idMaximized: null, - websocketEndpoint: null, - chartOptions: false, - captionHandler: '', - when: '', - interval: '', - currentId: '', - isIndicator: '' - }; + + this.clearComponentState(); } - + componentDidMount = () => { this.getDashboard(); this.getIndicators(); } - + componentDidUpdate = (prevProps, prevState) => { const {websocketEndpoint} = this.state; - if( + if ( websocketEndpoint !== null && prevState.websocketEndpoint !== websocketEndpoint - ){ + ) { connectWS.call(this, websocketEndpoint, msg => { msg.events.map(event => { - switch(event.widgetType){ + switch (event.widgetType) { case 'TargetIndicator': this.getIndicators(); break; @@ -76,11 +66,27 @@ export class DraggableWrapper extends Component { }) } } - + componentWillUnmount = () => { + this.clearComponentState(); disconnectWS.call(this); } - + + clearComponentState = () => { + this.state = { + cards: [], + indicators: [], + idMaximized: null, + websocketEndpoint: null, + chartOptions: false, + captionHandler: '', + when: '', + interval: '', + currentId: '', + isIndicator: '' + }; + } + getType = (entity) => entity === 'cards' ? 'kpis' : 'targetIndicators'; getIndicators = () => { diff --git a/src/components/header/Header.js b/src/components/header/Header.js index eb2061a9a..e091c7dd7 100644 --- a/src/components/header/Header.js +++ b/src/components/header/Header.js @@ -54,12 +54,12 @@ class Header extends Component { } componentDidMount() { - document.addEventListener('scroll', this.handleScroll); + this.initEventListeners(); } componentWillUnmount() { - document.removeEventListener('scroll', this.handleScroll); this.toggleScrollScope(false); + this.removeEventListeners(); } componentWillUpdate = (nextProps) => { @@ -92,6 +92,14 @@ class Header extends Component { return { shortcuts: shortcutManager } } + initEventListeners = () => { + document.addEventListener('scroll', this.handleScroll); + } + + removeEventListeners = () => { + document.removeEventListener('scroll', this.handleScroll); + } + handleInboxOpen = (state) => { this.setState({ isInboxOpen: !!state diff --git a/src/components/header/MenuOverlay.js b/src/components/header/MenuOverlay.js index ee8a0046e..b45a96fc9 100644 --- a/src/components/header/MenuOverlay.js +++ b/src/components/header/MenuOverlay.js @@ -22,15 +22,9 @@ import { } from '../../actions/WindowActions'; class MenuOverlay extends Component { - constructor(props){ + constructor(props) { super(props); - this.state = { - queriedResults: [], - query: '', - deepSubNode: null, - path: '', - data: {} - }; + this.clearComponentState(); } componentDidMount = () => { @@ -39,7 +33,7 @@ class MenuOverlay extends Component { nodeId } = this.props; - if (nodeId == 0){ + if (nodeId == 0) { getRootBreadcrumb().then(response => { this.setState({ data: response @@ -63,6 +57,20 @@ class MenuOverlay extends Component { } + componentWillUnmount = () => { + this.clearComponentState(); + } + + clearComponentState = () => { + this.state = { + queriedResults: [], + query: '', + deepSubNode: null, + path: '', + data: {} + }; + } + handleClickOutside = (e) => this.props.onClickOutside(e); handleQuery = (e) => { diff --git a/src/components/widget/List/List.js b/src/components/widget/List/List.js index 9c2f545aa..3aac0a972 100644 --- a/src/components/widget/List/List.js +++ b/src/components/widget/List/List.js @@ -12,11 +12,7 @@ class List extends Component { constructor(props) { super(props); - this.state = { - list: null, - loading: false, - selectedItem: '' - } + this.clearComponentState(); this.previousValue = ''; } @@ -37,6 +33,18 @@ class List extends Component { } } + componentWillUnmount() { + this.clearComponentState(); + } + + clearComponentState = () => { + this.state = { + list: null, + loading: false, + selectedItem: '' + } + } + requestListData = (forceSelection = false, forceFocus = false) => { const { properties, dataId, rowId, tabId, windowType, diff --git a/src/components/widget/List/RawList.js b/src/components/widget/List/RawList.js index 5f7f174d1..7520bb869 100644 --- a/src/components/widget/List/RawList.js +++ b/src/components/widget/List/RawList.js @@ -20,6 +20,10 @@ class RawList extends Component { } } + componentWillUnmount() { + this.clearComponentState(); + } + componentDidUpdate = (prevProps, prevState) => { const { list, mandatory, defaultValue, autofocus, blur, property, @@ -123,6 +127,14 @@ class RawList extends Component { } } + clearComponentState = () => { + this.state = { + selected: 0, + dropdownList: [], + isOpen: false + } + } + focus() { if (this.dropdown) { this.dropdown.focus(); diff --git a/src/components/widget/MasterWidget.js b/src/components/widget/MasterWidget.js index f556aa565..3455ac791 100644 --- a/src/components/widget/MasterWidget.js +++ b/src/components/widget/MasterWidget.js @@ -15,11 +15,7 @@ class MasterWidget extends Component { constructor(props) { super(props); - this.state = { - updated: false, - edited: false, - data: '' - } + this.clearComponentState(); } componentDidMount() { @@ -62,6 +58,16 @@ class MasterWidget extends Component { componentWillUnmount() { clearTimeout(this.timeout); + + this.clearComponentState(); + } + + clearComponentState = () => { + this.state = { + updated: false, + edited: false, + data: '' + } } handlePatch = (property, value) => { diff --git a/src/containers/MasterWindow.js b/src/containers/MasterWindow.js index 1a70c557d..23a410c7e 100644 --- a/src/containers/MasterWindow.js +++ b/src/containers/MasterWindow.js @@ -39,7 +39,7 @@ class MasterWindow extends Component { const isDocumentNotSaved = !master.saveStatus.saved; if(isDocumentNotSaved){ - window.addEventListener('beforeunload', this.confirm); + this.initEventListeners(); } } @@ -82,10 +82,10 @@ class MasterWindow extends Component { } if(prevProps.master.saveStatus.saved && isDocumentNotSaved) { - window.addEventListener('beforeunload', this.confirm) + this.initEventListeners(); } if (!prevProps.master.saveStatus.saved && isDocumentSaved) { - window.removeEventListener('beforeunload', this.confirm) + this.removeEventListeners(); } // When closing modal, we need to update the stale tabs @@ -107,7 +107,7 @@ class MasterWindow extends Component { const isDocumentNotSaved = !master.saveStatus.saved && master.saveStatus.saved !== undefined; - window.removeEventListener('beforeunload', this.confirm); + this.removeEventListeners(); if(isDocumentNotSaved && !isDeleted){ const result = window.confirm('Do you really want to leave?'); @@ -125,6 +125,14 @@ class MasterWindow extends Component { e.returnValue = ''; } + initEventListeners = () => { + window.addEventListener('beforeunload', this.confirm); + } + + removeEventListeners = () => { + window.removeEventListener('beforeunload', this.confirm); + } + closeModalCallback = (isNew) => { if(isNew){ this.setState({