From 7e4e050f848052f8003098160f4ec4a18ddbe4b9 Mon Sep 17 00:00:00 2001 From: Chan Ger Hean Date: Wed, 24 Feb 2021 11:47:35 +0800 Subject: [PATCH] [#1382] Avoid rerendering tabs unnecessarily (#1390) Tabs are destroyed and rerendered based on a key which can lead to mysterious bugs. Furthermore, tab information is passed indirectly from store to `main.js` before being passed to the tab itself. Let's - clean up the code such that tabs do not need to completely rerender when information changes. - pass the tab information directly. --- frontend/src/index.pug | 10 +-- frontend/src/static/js/main.js | 13 +--- frontend/src/static/js/v_authorship.js | 84 ++++++++++++++++---------- frontend/src/static/js/v_zoom.js | 51 ++++++++++------ 4 files changed, 87 insertions(+), 71 deletions(-) diff --git a/frontend/src/index.pug b/frontend/src/index.pug index efb2c5a953..f979d4d054 100644 --- a/frontend/src/index.pug +++ b/frontend/src/index.pug @@ -78,14 +78,8 @@ html #tabs-wrapper(ref="tabWrapper", slot="right") .tab-content.panel-padding .tab-pane - v-authorship#tab-authorship( - v-if="tabType === 'authorship'", - v-bind:key="generateKey(tabInfo.tabAuthorship, ['author', 'repo', 'isMergeGroup'])", - v-bind:info="tabInfo.tabAuthorship") - v-zoom#tab-zoom( - v-else-if="tabType === 'zoom'", - v-bind:key="generateKey(tabInfo.tabZoom, ['zRepo', 'zAuthor', 'zFilterGroup', 'zTimeFrame'])", - v-bind:info="tabInfo.tabZoom") + v-authorship#tab-authorship(v-if="tabType === 'authorship'") + v-zoom#tab-zoom(v-else-if="tabType === 'zoom'") #tab-empty(v-else) .title h2 Welcome to this RepoSense report! diff --git a/frontend/src/static/js/main.js b/frontend/src/static/js/main.js index 8be77f536b..fed0618e97 100644 --- a/frontend/src/static/js/main.js +++ b/frontend/src/static/js/main.js @@ -28,22 +28,18 @@ window.app = new window.Vue({ isLoadingOverlayEnabled: false, loadingOverlayOpacity: 1, - isCollapsed: false, isTabActive: true, // to force tab wrapper to load tabType: 'empty', - tabInfo: {}, creationDate: '', errorMessages: {}, }, watch: { '$store.state.tabZoomInfo': function () { - this.tabInfo.tabZoom = Object.assign({}, this.$store.state.tabZoomInfo); this.activateTab('zoom'); }, '$store.state.tabAuthorshipInfo': function () { - this.tabInfo.tabAuthorship = Object.assign({}, this.$store.state.tabAuthorshipInfo); this.activateTab('authorship'); }, '$store.state.loadingOverlayCount': function () { @@ -107,14 +103,11 @@ window.app = new window.Vue({ // handle opening of sidebar // activateTab(tabName) { - // changing isTabActive to trigger redrawing of component - this.isTabActive = false; if (this.$refs.tabWrapper) { this.$refs.tabWrapper.scrollTop = 0; } this.isTabActive = true; - this.isCollapsed = false; this.tabType = tabName; window.addHash('tabOpen', this.isTabActive); @@ -135,6 +128,7 @@ window.app = new window.Vue({ author: hash.tabAuthor, repo: hash.tabRepo, isMergeGroup: hash.authorshipIsMergeGroup === 'true', + isRefresh: true, minDate, maxDate, }; @@ -189,11 +183,6 @@ window.app = new window.Vue({ } }, - generateKey(dataObj, keysToUse) { - const picked = keysToUse.map((key) => dataObj[key]); - return JSON.stringify(picked); - }, - getRepoSenseHomeLink() { const version = window.app.repoSenseVersion; if (!version) { diff --git a/frontend/src/static/js/v_authorship.js b/frontend/src/static/js/v_authorship.js index e55d4659a9..fa7a578cae 100644 --- a/frontend/src/static/js/v_authorship.js +++ b/frontend/src/static/js/v_authorship.js @@ -6,26 +6,29 @@ const filesSortDict = { fileType: (file) => file.fileType, }; +function initialState() { + return { + isLoaded: false, + files: [], + selectedFiles: [], + filterType: 'checkboxes', + selectedFileTypes: [], + fileTypes: [], + filesLinesObj: {}, + fileTypeBlankLinesObj: {}, + filesSortType: 'lineOfCode', + toReverseSortFiles: true, + searchBarValue: '', + }; +} + const repoCache = []; const minimatch = require('minimatch'); window.vAuthorship = { - props: ['info'], template: window.$('v_authorship').innerHTML, data() { - return { - isLoaded: false, - files: [], - selectedFiles: [], - filterType: 'checkboxes', - selectedFileTypes: [], - fileTypes: [], - filesLinesObj: {}, - fileTypeBlankLinesObj: {}, - filesSortType: 'lineOfCode', - toReverseSortFiles: true, - searchBarValue: '', - }; + return initialState(); }, watch: { @@ -49,11 +52,9 @@ window.vAuthorship = { this.updateSelectedFiles(); }, - isLoaded() { - if (this.isLoaded) { - this.retrieveHashes(); - this.setInfoHash(); - } + authorshipOwnerWatchable() { + Object.assign(this.$data, initialState()); + this.initiate(); }, }, @@ -73,13 +74,12 @@ window.vAuthorship = { this.toReverseSortFiles = hash.reverseAuthorshipOrder !== 'false'; - this.selectedFileTypes = this.info.checkedFileTypes - ? this.info.checkedFileTypes.filter((fileType) => this.fileTypes.includes(fileType)) - : []; if (hash.authorshipFileTypes) { this.selectedFileTypes = hash.authorshipFileTypes .split(window.HASH_DELIMITER) .filter((fileType) => this.fileTypes.includes(fileType)); + } else { + this.resetSelectedFileTypes(); } if ('authorshipFilesGlob' in hash) { @@ -88,6 +88,12 @@ window.vAuthorship = { } }, + resetSelectedFileTypes() { + this.selectedFileTypes = this.info.checkedFileTypes + ? this.info.checkedFileTypes.filter((fileType) => this.fileTypes.includes(fileType)) + : []; + }, + setInfoHash() { const { addHash } = window; // We only set these hashes as they are propagated from summary_charts @@ -108,7 +114,7 @@ window.vAuthorship = { window.encodeHash(); }, - initiate() { + async initiate() { const repo = window.REPOS[this.info.repo]; this.getRepoProps(repo); @@ -124,12 +130,19 @@ window.vAuthorship = { } repoCache.push(this.info.repo); - if (repo.files) { - this.processFiles(repo.files); + let { files } = repo; + if (!files) { + files = await window.api.loadAuthorship(this.info.repo); + } + this.processFiles(files); + + if (this.info.isRefresh) { + this.retrieveHashes(); } else { - window.api.loadAuthorship(this.info.repo) - .then((files) => this.processFiles(files)); + this.resetSelectedFileTypes(); } + + this.setInfoHash(); }, getRepoProps(repo) { @@ -273,8 +286,7 @@ window.vAuthorship = { this.fileTypeBlankLinesObj = fileTypeBlanksInfoObj; this.files = res; - this.isLoaded = true; - this.updateSelectedFiles(); + this.updateSelectedFiles(true); }, getContributionFromAllAuthors(contributionMap) { @@ -309,7 +321,7 @@ window.vAuthorship = { window.encodeHash(); }, - updateSelectedFiles() { + updateSelectedFiles(setIsLoaded = false) { this.$store.commit('incrementLoadingOverlayCount', 1); setTimeout(() => { this.selectedFiles = this.files.filter( @@ -317,6 +329,9 @@ window.vAuthorship = { && minimatch(file.path, this.searchBarValue || '*', { matchBase: true, dot: true }), ) .sort(this.sortingFunction); + if (setIsLoaded) { + this.isLoaded = true; + } this.$store.commit('incrementLoadingOverlayCount', -1); }); }, @@ -352,6 +367,10 @@ window.vAuthorship = { }, computed: { + authorshipOwnerWatchable() { + return `${this.info.author}|${this.info.repo}|${this.info.isMergeGroup}`; + }, + sortingFunction() { return (a, b) => (this.toReverseSortFiles ? -1 : 1) * window.comparator(filesSortDict[this.filesSortType])(a, b); @@ -394,7 +413,10 @@ window.vAuthorship = { return numLinesModified; }, - ...Vuex.mapState(['fileTypeColors']), + ...Vuex.mapState({ + fileTypeColors: 'fileTypeColors', + info: 'tabAuthorshipInfo', + }), }, created() { diff --git a/frontend/src/static/js/v_zoom.js b/frontend/src/static/js/v_zoom.js index 964b1b173e..80a5864728 100644 --- a/frontend/src/static/js/v_zoom.js +++ b/frontend/src/static/js/v_zoom.js @@ -1,21 +1,28 @@ /* global Vuex */ +function initialState() { + return { + showAllCommitMessageBody: true, + expandedCommitMessagesCount: this.totalCommitMessageBodyCount, + commitsSortType: 'time', + toReverseSortedCommits: true, + isCommitsFinalized: false, + selectedFileTypes: [], + fileTypes: [], + }; +} + window.vZoom = { - props: ['info'], template: window.$('v_zoom').innerHTML, data() { - return { - showAllCommitMessageBody: true, - expandedCommitMessagesCount: this.totalCommitMessageBodyCount, - commitsSortType: 'time', - toReverseSortedCommits: true, - isCommitsFinalized: false, - selectedFileTypes: [], - fileTypes: [], - }; + return initialState(); }, computed: { + zoomOwnerWatchable() { + return `${this.info.zRepo}|${this.info.zAuthor}|${this.info.zFilterGroup}|${this.info.zTimeFrame}`; + }, + sortingFunction() { const commitSortFunction = this.commitsSortType === 'time' ? (commit) => commit.date @@ -34,7 +41,6 @@ window.vZoom = { filteredUser.commits = zUser.commits.filter( (commit) => commit[date] >= zSince && commit[date] <= zUntil, ).sort(this.sortingFunction); - this.isCommitsFinalized = true; return filteredUser; }, @@ -81,17 +87,20 @@ window.vZoom = { this.updateSelectedFileTypesHash(); }, }, - ...Vuex.mapState(['fileTypeColors']), + + ...Vuex.mapState({ + fileTypeColors: 'fileTypeColors', + info: 'tabZoomInfo', + }), }, watch: { - isCommitsFinalized() { - if (this.isCommitsFinalized) { - this.updateFileTypes(); - this.selectedFileTypes = this.fileTypes.slice(); - this.retrieveHashes(); - } + zoomOwnerWatchable() { + Object.assign(this.$data, initialState()); + this.initiate(); + this.setInfoHash(); }, + selectedFileTypes() { this.$nextTick(() => { this.updateExpandedCommitMessagesCount(); @@ -112,6 +121,9 @@ window.vZoom = { if (!this.info.zUser) { // restoring zoom tab from reloaded page this.restoreZoomTab(); } + + this.updateFileTypes(); + this.selectedFileTypes = this.fileTypes.slice(); }, openSummary() { @@ -246,8 +258,7 @@ window.vZoom = { }, created() { this.initiate(); - }, - mounted() { + this.retrieveHashes(); this.setInfoHash(); }, beforeDestroy() {