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.

Sign up for GitHub

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

Refactor diffFileInfo / DiffTreeStore #24998

Merged
merged 4 commits into from
May 30, 2023
Merged

Refactor diffFileInfo / DiffTreeStore #24998

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
021d411
fix
wxiaoguang May 30, 2023
f1f7247
fine tune
wxiaoguang May 30, 2023
46b294a
Merge branch 'main' into refactor-diff-view
GiteaBot May 30, 2023
6ecd3bc
Merge branch 'main' into refactor-diff-view
GiteaBot May 30, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 25 additions & 27 deletions templates/repo/diff/box.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,33 +45,31 @@
{{end}}
</div>
</div>
<script id="diff-data-script">
(() => {
const diffData = {
files: [{{range $i, $file := .Diff.Files}}{Name:"{{$file.Name}}",NameHash:"{{$file.NameHash}}",Type:{{$file.Type}},IsBin:{{$file.IsBin}},Addition:{{$file.Addition}},Deletion:{{$file.Deletion}}},{{end}}],
isIncomplete: {{.Diff.IsIncomplete}},
tooManyFilesMessage: "{{$.locale.Tr "repo.diff.too_many_files"}}",
binaryFileMessage: "{{$.locale.Tr "repo.diff.bin"}}",
showMoreMessage: "{{.locale.Tr "repo.diff.show_more"}}",
statisticsMessage: "{{.locale.Tr "repo.diff.stats_desc_file"}}",
fileTreeIsVisible: false,
fileListIsVisible: false,
isLoadingNewData: false,
diffEnd: {{.Diff.End}},
link: "{{$.Link}}"
};
if(window.config.pageData.diffFileInfo) {
// Page is already loaded - add the data to our existing data
window.config.pageData.diffFileInfo.files.push(...diffData.files);
window.config.pageData.diffFileInfo.isIncomplete = diffData.isIncomplete;
window.config.pageData.diffFileInfo.diffEnd = diffData.diffEnd;
window.config.pageData.diffFileInfo.link = diffData.link;
} else {
// new load of page - populate initial data
window.config.pageData.diffFileInfo = diffData;
}
})();
</script>
<script id="diff-data-script" type="module">
const diffDataFiles = [{{range $i, $file := .Diff.Files}}{Name:"{{$file.Name}}",NameHash:"{{$file.NameHash}}",Type:{{$file.Type}},IsBin:{{$file.IsBin}},Addition:{{$file.Addition}},Deletion:{{$file.Deletion}}},{{end}}];
const diffData = {
isIncomplete: {{.Diff.IsIncomplete}},
tooManyFilesMessage: "{{$.locale.Tr "repo.diff.too_many_files"}}",
binaryFileMessage: "{{$.locale.Tr "repo.diff.bin"}}",
showMoreMessage: "{{.locale.Tr "repo.diff.show_more"}}",
statisticsMessage: "{{.locale.Tr "repo.diff.stats_desc_file"}}",
linkLoadMore: "{{$.Link}}?skip-to={{.Diff.End}}&file-only=true",
};

// for first time loading, the diffFileInfo is a plain object
// after the Vue component is mounted, the diffFileInfo is a reactive object
// keep in mind that this script block would be executed many times when loading more files, by "loadMoreFiles"
let diffFileInfo = window.config.pageData.diffFileInfo || {
files:[],
fileTreeIsVisible: false,
fileListIsVisible: false,
isLoadingNewData: false,
selectedItem: '',
};
diffFileInfo = Object.assign(diffFileInfo, diffData);
diffFileInfo.files.push(...diffDataFiles);
window.config.pageData.diffFileInfo = diffFileInfo;
</script>
<div id="diff-file-list"></div>
<div id="diff-container">
<div id="diff-file-tree" class="gt-hidden"></div>
Expand Down
28 changes: 12 additions & 16 deletions web_src/js/components/DiffFileList.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,33 +1,32 @@
<template>
<ol class="diff-detail-box diff-stats gt-m-0" ref="root" v-if="fileListIsVisible">
<li v-for="file in files" :key="file.NameHash">
<ol class="diff-detail-box diff-stats gt-m-0" ref="root" v-if="store.fileListIsVisible">
<li v-for="file in store.files" :key="file.NameHash">
<div class="gt-font-semibold gt-df gt-ac pull-right">
<span v-if="file.IsBin" class="gt-ml-1 gt-mr-3">{{ binaryFileMessage }}</span>
<span v-if="file.IsBin" class="gt-ml-1 gt-mr-3">{{ store.binaryFileMessage }}</span>
{{ file.IsBin ? '' : file.Addition + file.Deletion }}
<span v-if="!file.IsBin" class="diff-stats-bar gt-mx-3" :data-tooltip-content="statisticsMessage.replace('%d', (file.Addition + file.Deletion)).replace('%d', file.Addition).replace('%d', file.Deletion)">
<span v-if="!file.IsBin" class="diff-stats-bar gt-mx-3" :data-tooltip-content="store.statisticsMessage.replace('%d', (file.Addition + file.Deletion)).replace('%d', file.Addition).replace('%d', file.Deletion)">
<div class="diff-stats-add-bar" :style="{ 'width': diffStatsWidth(file.Addition, file.Deletion) }"/>
</span>
</div>
<!-- todo finish all file status, now modify, add, delete and rename -->
<span :class="['status', diffTypeToString(file.Type)]" :data-tooltip-content="diffTypeToString(file.Type)">&nbsp;</span>
<a class="file gt-mono" :href="'#diff-' + file.NameHash">{{ file.Name }}</a>
</li>
<li v-if="isIncomplete" id="diff-too-many-files-stats" class="gt-pt-2">
<span class="file gt-df gt-ac gt-sb">{{ tooManyFilesMessage }}
<a :class="['ui', 'basic', 'tiny', 'button', isLoadingNewData === true ? 'disabled' : '']" id="diff-show-more-files-stats" @click.stop="loadMoreData">{{ showMoreMessage }}</a>
<li v-if="store.isIncomplete" class="gt-pt-2">
<span class="file gt-df gt-ac gt-sb">{{ store.tooManyFilesMessage }}
<a :class="['ui', 'basic', 'tiny', 'button', store.isLoadingNewData ? 'disabled' : '']" @click.stop="loadMoreData">{{ store.showMoreMessage }}</a>
</span>
</li>
</ol>
</template>

<script>
import {doLoadMoreFiles} from '../features/repo-diff.js';

const {pageData} = window.config;
import {loadMoreFiles} from '../features/repo-diff.js';
import {diffTreeStore} from '../modules/stores.js';

export default {
data: () => {
return pageData.diffFileInfo;
return {store: diffTreeStore()};
},
mounted() {
document.getElementById('show-file-list-btn').addEventListener('click', this.toggleFileList);
Expand All @@ -37,7 +36,7 @@ export default {
},
methods: {
toggleFileList() {
this.fileListIsVisible = !this.fileListIsVisible;
this.store.fileListIsVisible = !this.store.fileListIsVisible;
},
diffTypeToString(pType) {
const diffTypes = {
Expand All @@ -53,10 +52,7 @@ export default {
return `${adds / (adds + dels) * 100}%`;
},
loadMoreData() {
this.isLoadingNewData = true;
doLoadMoreFiles(this.link, this.diffEnd, () => {
this.isLoadingNewData = false;
});
loadMoreFiles(this.store.linkLoadMore);
}
},
};
Expand Down
42 changes: 13 additions & 29 deletions web_src/js/components/DiffFileTree.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,42 +1,33 @@
<template>
<div
v-if="fileTreeIsVisible"
class="gt-mr-3 gt-mt-3 diff-detail-box"
>
<div v-if="store.fileTreeIsVisible" class="gt-mr-3 gt-mt-3 diff-detail-box">
<!-- only render the tree if we're visible. in many cases this is something that doesn't change very often -->
<div class="ui list">
<DiffFileTreeItem v-for="item in fileTree" :key="item.name" :item="item"/>
</div>
<div v-if="isIncomplete" id="diff-too-many-files-stats" class="gt-pt-2">
<span class="gt-mr-2">{{ tooManyFilesMessage }}</span><a :class="['ui', 'basic', 'tiny', 'button', isLoadingNewData === true ? 'disabled' : '']" id="diff-show-more-files-stats" @click.stop="loadMoreData">{{ showMoreMessage }}</a>
<div v-if="store.isIncomplete" class="gt-pt-2">
<a :class="['ui', 'basic', 'tiny', 'button', store.isLoadingNewData ? 'disabled' : '']" @click.stop="loadMoreData">{{ store.showMoreMessage }}</a>
</div>
</div>
</template>

<script>
import DiffFileTreeItem from './DiffFileTreeItem.vue';
import {doLoadMoreFiles} from '../features/repo-diff.js';
import {loadMoreFiles} from '../features/repo-diff.js';
import {toggleElem} from '../utils/dom.js';
import {DiffTreeStore} from '../modules/stores.js';
import {diffTreeStore} from '../modules/stores.js';
import {setFileFolding} from '../features/file-fold.js';

const {pageData} = window.config;
const LOCAL_STORAGE_KEY = 'diff_file_tree_visible';

export default {
components: {DiffFileTreeItem},
data: () => {
const fileTreeIsVisible = localStorage.getItem(LOCAL_STORAGE_KEY) === 'true';
pageData.diffFileInfo.fileTreeIsVisible = fileTreeIsVisible;
return {
...pageData.diffFileInfo,
store: DiffTreeStore,
};
return {store: diffTreeStore()};
},
computed: {
fileTree() {
const result = [];
for (const file of this.files) {
for (const file of this.store.files) {
// Split file into directories
const splits = file.Name.split('/');
let index = 0;
Expand Down Expand Up @@ -98,9 +89,7 @@ export default {
}
},
mounted() {
// replace the pageData.diffFileInfo.files with our watched data so we get updates
pageData.diffFileInfo.files = this.files;

this.store.fileTreeIsVisible = localStorage.getItem(LOCAL_STORAGE_KEY) === 'true';
document.querySelector('.diff-toggle-file-tree-button').addEventListener('click', this.toggleVisibility);

this.hashChangeListener = () => {
Expand All @@ -124,12 +113,12 @@ export default {
}
},
toggleVisibility() {
this.updateVisibility(!this.fileTreeIsVisible);
this.updateVisibility(!this.store.fileTreeIsVisible);
},
updateVisibility(visible) {
this.fileTreeIsVisible = visible;
localStorage.setItem(LOCAL_STORAGE_KEY, this.fileTreeIsVisible);
this.updateState(this.fileTreeIsVisible);
this.store.fileTreeIsVisible = visible;
localStorage.setItem(LOCAL_STORAGE_KEY, this.store.fileTreeIsVisible);
this.updateState(this.store.fileTreeIsVisible);
},
updateState(visible) {
const btn = document.querySelector('.diff-toggle-file-tree-button');
Expand All @@ -142,12 +131,7 @@ export default {
toggleElem(toHide, visible);
},
loadMoreData() {
this.isLoadingNewData = true;
doLoadMoreFiles(this.link, this.diffEnd, () => {
this.isLoadingNewData = false;
const {pageData} = window.config;
this.diffEnd = pageData.diffFileInfo.diffEnd;
});
loadMoreFiles(this.store.linkLoadMore);
},
},
};
Expand Down
4 changes: 2 additions & 2 deletions web_src/js/components/DiffFileTreeItem.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@

<script>
import {SvgIcon} from '../svg.js';
import {DiffTreeStore} from '../modules/stores.js';
import {diffTreeStore} from '../modules/stores.js';

export default {
components: {SvgIcon},
Expand All @@ -56,7 +56,7 @@ export default {
},
},
data: () => ({
store: DiffTreeStore,
store: diffTreeStore(),
collapsed: false,
}),
methods: {
Expand Down
39 changes: 16 additions & 23 deletions web_src/js/features/repo-diff.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import {initDiffFileTree} from './repo-diff-filetree.js';
import {validateTextareaNonEmpty} from './comp/ComboMarkdownEditor.js';
import {initViewedCheckboxListenerFor, countAndUpdateViewedFiles, initExpandAndCollapseFilesButton} from './pull-view-file.js';

const {csrfToken} = window.config;
const {csrfToken, pageData} = window.config;

function initRepoDiffReviewButton() {
const $reviewBox = $('#review-box');
Expand Down Expand Up @@ -119,37 +119,29 @@ function onShowMoreFiles() {
countAndUpdateViewedFiles();
}

export function doLoadMoreFiles(link, diffEnd, callback) {
const url = `${link}?skip-to=${diffEnd}&file-only=true`;
loadMoreFiles(url, callback);
}

function loadMoreFiles(url, callback) {
export function loadMoreFiles(url) {
const $target = $('a#diff-show-more-files');
if ($target.hasClass('disabled')) {
callback();
if ($target.hasClass('disabled') || pageData.diffFileInfo.isLoadingNewData) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why we need to check both isLoadingNewData and disabled class here? A bit confused if a single check for isLoadingNewData works.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No particular reason. But disabled means traditional MVC while isXxx is for reactive MVVM. It's also right to do so And remind readers that this logic is shared by MVC and MVVM

return;
}

pageData.diffFileInfo.isLoadingNewData = true;
$target.addClass('disabled');
$.ajax({
type: 'GET',
url,
}).done((resp) => {
if (!resp) {
$target.removeClass('disabled');
callback(resp);
return;
}
$('#diff-incomplete').replaceWith($(resp).find('#diff-file-boxes').children());
// By simply rerunning the script we add the new data to our existing
// pagedata object. this triggers vue and the filetree and filelist will
// render the new elements.
$('body').append($(resp).find('script#diff-data-script'));
const $resp = $(resp);
// the response is a full HTML page, we need to extract the relevant contents:
// 1. append the newly loaded file list items to the existing list
$('#diff-incomplete').replaceWith($resp.find('#diff-file-boxes').children());
// 2. re-execute the script to append the newly loaded items to the JS variables to refresh the DiffFileTree
$('body').append($resp.find('script#diff-data-script'));

onShowMoreFiles();
callback(resp);
}).fail(() => {
}).always(() => {
$target.removeClass('disabled');
callback();
pageData.diffFileInfo.isLoadingNewData = false;
});
}

Expand All @@ -158,7 +150,8 @@ function initRepoDiffShowMore() {
e.preventDefault();

const $target = $(e.target);
loadMoreFiles($target.data('href'), () => {});
const linkLoadMore = $target.attr('data-href');
loadMoreFiles(linkLoadMore);
});

$(document).on('click', 'a.diff-load-button', (e) => {
Expand Down
11 changes: 8 additions & 3 deletions web_src/js/modules/stores.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
import {reactive} from 'vue';

export const DiffTreeStore = reactive({
selectedItem: '',
});
let diffTreeStoreReactive;
export function diffTreeStore() {
if (!diffTreeStoreReactive) {
diffTreeStoreReactive = reactive(window.config.pageData.diffFileInfo);
window.config.pageData.diffFileInfo = diffTreeStoreReactive;
}
return diffTreeStoreReactive;
}