refactor(diff): drive diff DOM init from the global selector observer (#38740)

Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
This commit is contained in:
Lunny Xiao
2026-08-02 13:19:57 +00:00
committed by GitHub
co-authored by wxiaoguang
parent b87fdd5da4
commit 4e4ea75fb9
6 changed files with 97 additions and 136 deletions
+7 -7
View File
@@ -81,7 +81,7 @@
<div class="diff-file-header sticky-2nd-row ui top attached header">
<div class="diff-file-name tw-flex tw-flex-1 tw-items-center tw-gap-1 tw-flex-wrap">
<div class="flex-text-block">
<button class="fold-file btn interact-bg tw-flex-shrink-0 tw-p-1{{if not $isExpandable}} tw-invisible{{end}}">
<button class="fold-file btn interact-bg tw-flex-shrink-0 tw-p-1 {{if not $isExpandable}}tw-invisible{{end}}" data-global-click="diffFileViewFold">
{{if $file.ShouldBeHidden}}
{{svg "octicon-chevron-right" 18}}
{{else}}
@@ -115,7 +115,7 @@
{{end}}
{{if $showFileViewToggle}}
<div class="ui compact icon buttons">
<div class="ui compact icon buttons" data-global-init="initDiffFileViewToggle">
<button class="ui tiny basic button file-view-toggle" data-toggle-selector="#diff-source-{{$file.NameHash}}" data-tooltip-content="{{ctx.Locale.Tr "repo.file_view_source"}}">{{svg "octicon-code"}}</button>
<button class="ui tiny basic button file-view-toggle active" data-toggle-selector="#diff-rendered-{{$file.NameHash}}" data-tooltip-content="{{ctx.Locale.Tr "repo.file_view_rendered"}}">{{svg "octicon-file"}}</button>
</div>
@@ -127,12 +127,12 @@
<span class="changed-since-last-review unselectable not-mobile">{{ctx.Locale.Tr "repo.pulls.has_changed_since_last_review"}}</span>
{{end}}
{{if $isReviewFile}}
<label data-link="{{$.Issue.Link}}/viewed-files" data-headcommit="{{$.AfterCommitID}}" class="viewed-file-form unselectable{{if $file.IsViewed}} viewed-file-checked-form{{end}}">
<input type="checkbox" name="{{$file.GetDiffFileName}}" autocomplete="off"{{if $file.IsViewed}} checked{{end}}> {{ctx.Locale.Tr "repo.pulls.has_viewed_file"}}
<label data-link="{{$.Issue.Link}}/viewed-files" data-headcommit="{{$.AfterCommitID}}" class="viewed-file-form unselectable {{if $file.IsViewed}}viewed-file-checked-form{{end}}" data-global-init="initDiffFileViewedForm">
<input type="checkbox" name="{{$file.GetDiffFileName}}" autocomplete="off" {{if $file.IsViewed}}checked{{end}}> {{ctx.Locale.Tr "repo.pulls.has_viewed_file"}}
</label>
{{end}}
{{if not $file.IsSubmodule}}
<button class="btn diff-header-popup-btn tw-p-1">{{svg "octicon-kebab-horizontal" 18}}</button>
<button class="btn tw-p-1" data-global-init="initDiffHeaderPopupMenu">{{svg "octicon-kebab-horizontal" 18}}</button>
<div class="tippy-target">
{{if not (or $file.IsIncomplete $file.IsBin)}}
<button class="unescape-button item" data-unicode-content-selector="#diff-{{$file.NameHash}}">{{ctx.Locale.Tr "repo.unescape_control_characters"}}</button>
@@ -162,7 +162,7 @@
{{ctx.Locale.Tr "repo.diff.file_suppressed_line_too_long"}}
{{else}}
{{ctx.Locale.Tr "repo.diff.file_suppressed"}}
<a class="ui basic tiny button diff-load-button" data-href="?file-only=true&files={{$file.Name}}&files={{$file.OldName}}">{{ctx.Locale.Tr "repo.diff.load"}}</a>
<a class="ui basic tiny button" data-global-click="diffLoadFileBody" data-href="?file-only=true&files={{$file.Name}}&files={{$file.OldName}}">{{ctx.Locale.Tr "repo.diff.load"}}</a>
{{end}}
{{else}}
{{ctx.Locale.Tr "repo.diff.bin_not_shown"}}
@@ -209,7 +209,7 @@
<div class="diff-file-box file-content tw-mt-2" id="diff-incomplete">
<h4 class="ui top attached header tw-font-normal flex-left-right">
{{ctx.Locale.Tr "repo.diff.too_many_files"}}
<a class="ui basic tiny button" id="diff-show-more-files" data-href="?skip-to={{.Diff.End}}&file-only=true">{{ctx.Locale.Tr "repo.diff.show_more"}}</a>
<a class="ui basic tiny button" id="diff-show-more-files" data-global-click="diffLoadMoreFiles" data-href="?skip-to={{.Diff.End}}&file-only=true">{{ctx.Locale.Tr "repo.diff.show_more"}}</a>
</h4>
</div>
{{end}}
+1 -1
View File
@@ -5,7 +5,7 @@
{{if or $hasLeft $hasRight}}
<tr>
<td colspan="2">
<div class="image-diff"
<div class="image-diff" data-global-init="initDiffFileImageDiff"
data-path-before="{{$root.BeforeRawPath}}/{{PathEscapeSegments $file.OldName}}"
data-path-after="{{$root.RawPath}}/{{PathEscapeSegments $file.Name}}"
data-mime-before="{{$file.LeftBlobMimeType}}"
+3 -6
View File
@@ -1,5 +1,5 @@
import {GET} from '../modules/fetch.ts';
import {hideElem, loadElem, queryElemChildren, queryElems} from '../utils/dom.ts';
import {hideElem, loadElem, queryElemChildren} from '../utils/dom.ts';
import {parseDom} from '../utils.ts';
type ImageContext = {
@@ -98,7 +98,6 @@ class ImageDiff {
async init(containerEl: HTMLElement) {
this.containerEl = containerEl;
containerEl.setAttribute('data-image-diff-loaded', 'true');
// the container may be hidden by "viewed" checkbox, so use the parent's width for reference
this.diffContainerWidth = Math.max(containerEl.closest('.diff-file-box')!.clientWidth - 300, 100);
@@ -300,8 +299,6 @@ class ImageDiff {
}
}
export function initImageDiff() {
for (const el of queryElems<HTMLImageElement>(document, '.image-diff:not([data-image-diff-loaded])')) {
(new ImageDiff()).init(el); // it is async, but we don't need to await for it
}
export async function initImageDiff(el: HTMLElement) {
await new ImageDiff().init(el);
}
+34 -42
View File
@@ -6,7 +6,6 @@ const {pageData} = window.config;
// it is undefined on most pages, fortunately, when it is accessed by the related functions, it exists
const prReview = pageData.prReview!;
const viewedStyleClass = 'viewed-file-checked-form';
const viewedCheckboxSelector = '.viewed-file-form'; // Selector under which all "Viewed" checkbox forms can be found
const expandFilesBtnSelector = '#expand-files-btn';
const collapseFilesBtnSelector = '#collapse-files-btn';
@@ -21,52 +20,45 @@ function refreshViewedFilesSummary() {
.replace('%[2]d', String(prReview.numberOfFiles));
}
// Initializes a listener for all children of the given html element
// (for example 'document' in the most basic case)
// to watch for changes of viewed-file checkboxes
export function initViewedCheckboxListenerFor() {
for (const form of document.querySelectorAll(`${viewedCheckboxSelector}:not([data-has-viewed-checkbox-listener="true"])`)) {
// To prevent double addition of listeners
form.setAttribute('data-has-viewed-checkbox-listener', String(true));
// Initializes a listener for viewed-file checkboxes
export function initDiffFileViewedForm(el: Element) {
// The checkbox consists of a div containing the real checkbox with its label,
// hence the actual checkbox first has to be found
const checkbox = el.querySelector<HTMLInputElement>('input[type=checkbox]')!;
checkbox.addEventListener('input', function() {
// Mark the file as viewed visually - will especially change the background
if (this.checked) {
el.classList.add(viewedStyleClass);
checkbox.setAttribute('checked', '');
prReview.numberOfViewedFiles++;
} else {
el.classList.remove(viewedStyleClass);
checkbox.removeAttribute('checked');
prReview.numberOfViewedFiles--;
}
// The checkbox consists of a div containing the real checkbox with its label,
// hence the actual checkbox first has to be found
const checkbox = form.querySelector<HTMLInputElement>('input[type=checkbox]')!;
checkbox.addEventListener('input', function() {
// Mark the file as viewed visually - will especially change the background
if (this.checked) {
form.classList.add(viewedStyleClass);
checkbox.setAttribute('checked', '');
prReview.numberOfViewedFiles++;
} else {
form.classList.remove(viewedStyleClass);
checkbox.removeAttribute('checked');
prReview.numberOfViewedFiles--;
}
// Update viewed-files summary and remove "has changed" label if present
refreshViewedFilesSummary();
const hasChangedLabel = el.parentNode!.querySelector('.changed-since-last-review');
hasChangedLabel?.remove();
// Update viewed-files summary and remove "has changed" label if present
refreshViewedFilesSummary();
const hasChangedLabel = form.parentNode!.querySelector('.changed-since-last-review');
hasChangedLabel?.remove();
const fileName = checkbox.getAttribute('name')!;
const fileName = checkbox.getAttribute('name')!;
// check if the file is in our diffTreeStore and if we find it -> change the IsViewed status
diffTreeStoreSetViewed(diffTreeStore(), fileName, this.checked);
// check if the file is in our diffTreeStore and if we find it -> change the IsViewed status
diffTreeStoreSetViewed(diffTreeStore(), fileName, this.checked);
// Unfortunately, actual forms cause too many problems, hence another approach is needed
const files: Record<string, boolean> = {};
files[fileName] = this.checked;
const data: Record<string, any> = {files};
const headCommitSHA = el.getAttribute('data-headcommit');
if (headCommitSHA) data.headCommitSHA = headCommitSHA;
POST(el.getAttribute('data-link')!, {data});
// Unfortunately, actual forms cause too many problems, hence another approach is needed
const files: Record<string, boolean> = {};
files[fileName] = this.checked;
const data: Record<string, any> = {files};
const headCommitSHA = form.getAttribute('data-headcommit');
if (headCommitSHA) data.headCommitSHA = headCommitSHA;
POST(form.getAttribute('data-link')!, {data});
// Fold the file accordingly
const parentBox = form.closest('.diff-file-header')!;
setFileFolding(parentBox.closest('.file-content')!, parentBox.querySelector('.fold-file')!, this.checked);
});
}
// Fold the file accordingly
const parentBox = el.closest('.diff-file-header')!;
setFileFolding(parentBox.closest('.file-content')!, parentBox.querySelector('.fold-file')!, this.checked);
});
}
export function initExpandAndCollapseFilesButton() {
+52 -78
View File
@@ -2,8 +2,7 @@ import {initRepoIssueContentHistory} from './repo-issue-content.ts';
import {initDiffFileTree} from './repo-diff-filetree.ts';
import {initDiffCommitSelect} from './repo-diff-commitselect.ts';
import {validateTextareaNonEmpty} from './comp/ComboMarkdownEditor.ts';
import {initViewedCheckboxListenerFor, initExpandAndCollapseFilesButton} from './pull-view-file.ts';
import {initImageDiff} from './imagediff.ts';
import {initExpandAndCollapseFilesButton, initDiffFileViewedForm} from './pull-view-file.ts';
import {showErrorToast} from '../modules/toast.ts';
import {queryElemSiblings, hideElem, showElem, animateOnce, addDelegatedEventListener, createElementFromHTML, queryElems} from '../utils/dom.ts';
import {errorMessage} from '../modules/errors.ts';
@@ -11,10 +10,11 @@ import {POST, GET} from '../modules/fetch.ts';
import {createTippy} from '../modules/tippy.ts';
import {invertFileFolding} from './file-fold.ts';
import {parseDom} from '../utils.ts';
import {registerGlobalSelectorFunc} from '../modules/observer.ts';
import {registerGlobalEventFunc, registerGlobalInitFunc} from '../modules/observer.ts';
import {performFetchActionTrigger} from './common-fetch-action.ts';
import {initImageDiff} from './imagediff.ts';
function initRepoDiffFileBox(el: HTMLElement) {
function initDiffFileViewToggle(el: HTMLElement) {
// switch between "rendered" and "source", for image and CSV files
queryElems(el, '.file-view-toggle', (btn) => btn.addEventListener('click', () => {
queryElemSiblings(btn, '.file-view-toggle', (el) => el.classList.remove('active'));
@@ -133,48 +133,38 @@ function initRepoDiffConversationNav() {
});
}
function initDiffHeaderPopup() {
for (const btn of document.querySelectorAll('.diff-header-popup-btn:not([data-header-popup-initialized])')) {
btn.setAttribute('data-header-popup-initialized', '');
const popup = btn.nextElementSibling;
if (!popup?.matches('.tippy-target')) throw new Error('Popup element not found');
createTippy(btn, {
content: popup,
theme: 'menu',
placement: 'bottom-end',
trigger: 'click',
interactive: true,
hideOnClick: true,
});
}
function initDiffHeaderPopupMenu(btn: HTMLElement) {
const popup = btn.nextElementSibling;
if (!popup?.matches('.tippy-target')) throw new Error('Popup element not found');
createTippy(btn, {
content: popup,
theme: 'menu',
placement: 'bottom-end',
trigger: 'click',
interactive: true,
hideOnClick: true,
});
}
// Will be called when the show more (files) button has been pressed
function onShowMoreFiles() {
// TODO: replace these calls with the "observer.ts" methods
initRepoIssueContentHistory();
initViewedCheckboxListenerFor();
initImageDiff();
initDiffHeaderPopup();
function onDiffFileBodyChange() {
initRepoIssueContentHistory(); // it scans the whole page via a fetch, so it doesn't fit the per-element observer pattern
}
async function loadMoreFiles(btn: Element): Promise<boolean> {
if (btn.classList.contains('disabled')) {
return false;
}
async function diffLoadMoreFiles(btn: Element): Promise<boolean> {
if (btn.classList.contains('disabled')) return false;
btn.classList.add('disabled');
const url = btn.getAttribute('data-href')!;
try {
const response = await GET(url);
const resp = await response.text();
const respDoc = parseDom(resp, 'text/html');
const resp = await GET(url);
if (!resp.ok) return false;
const respText = await resp.text();
const respDoc = parseDom(respText, 'text/html');
const respFileBoxes = respDoc.querySelector('#diff-file-boxes')!;
// the response is a full HTML page, we need to extract the relevant contents:
// * append the newly loaded file list items to the existing list
const respFileBoxesChildren = Array.from(respFileBoxes.children); // "children:HTMLCollection" will be empty after replaceWith
document.querySelector('#diff-incomplete')!.replaceWith(...respFileBoxesChildren);
onShowMoreFiles();
onDiffFileBodyChange();
return true;
} catch (error) {
console.error('Error:', error);
@@ -185,36 +175,24 @@ async function loadMoreFiles(btn: Element): Promise<boolean> {
return false;
}
function initRepoDiffShowMore() {
addDelegatedEventListener(document, 'click', 'a#diff-show-more-files', (el, e) => {
e.preventDefault();
loadMoreFiles(el);
});
addDelegatedEventListener(document, 'click', 'a.diff-load-button', async (el, e) => {
e.preventDefault();
if (el.classList.contains('disabled')) return;
el.classList.add('disabled');
const url = el.getAttribute('data-href')!;
try {
const response = await GET(url);
const resp = await response.text();
const respDoc = parseDom(resp, 'text/html');
const respFileBody = respDoc.querySelector('#diff-file-boxes .diff-file-body .file-body')!;
const respFileBodyChildren = Array.from(respFileBody.children); // "children:HTMLCollection" will be empty after replaceWith
el.parentElement!.replaceWith(...respFileBodyChildren);
// FIXME: calling onShowMoreFiles is not quite right here.
// But since onShowMoreFiles mixes "init diff box" and "init diff body" together,
// so it still needs to call it to make the "ImageDiff" and something similar work.
onShowMoreFiles();
} catch (error) {
console.error('Error:', error);
} finally {
el.classList.remove('disabled');
}
});
async function diffLoadFileBody(el: Element) {
if (el.classList.contains('disabled')) return;
el.classList.add('disabled');
const url = el.getAttribute('data-href')!;
try {
const resp = await GET(url);
if (!resp.ok) return;
const respText = await resp.text();
const respDoc = parseDom(respText, 'text/html');
const respFileBody = respDoc.querySelector('#diff-file-boxes .diff-file-body .file-body')!;
const respFileBodyChildren = Array.from(respFileBody.children); // "children:HTMLCollection" will be empty after replaceWith
el.parentElement!.replaceWith(...respFileBodyChildren);
onDiffFileBodyChange();
} catch (error) {
console.error('Error:', error);
} finally {
el.classList.remove('disabled');
}
}
async function onLocationHashChange() {
@@ -264,31 +242,27 @@ async function onLocationHashChange() {
}
// Load more files, await ensures we don't block progress
const ok = await loadMoreFiles(showMoreButton);
const ok = await diffLoadMoreFiles(showMoreButton);
if (!ok) return; // failed to load more files
}
}
function initRepoDiffHashChangeListener() {
window.addEventListener('hashchange', onLocationHashChange);
onLocationHashChange();
}
export function initRepoDiffView() {
initRepoDiffConversationForm(); // such form appears on the "conversation" page and "diff" page
registerGlobalEventFunc('click', 'diffLoadMoreFiles', (el) => { diffLoadMoreFiles(el) });
registerGlobalEventFunc('click', 'diffLoadFileBody', diffLoadFileBody);
registerGlobalEventFunc('click', 'diffFileViewFold', (el) => invertFileFolding(el.closest('.file-content')!, el));
registerGlobalInitFunc('initDiffHeaderPopupMenu', initDiffHeaderPopupMenu);
registerGlobalInitFunc('initDiffFileViewedForm', initDiffFileViewedForm);
registerGlobalInitFunc('initDiffFileImageDiff', initImageDiff);
registerGlobalInitFunc('initDiffFileViewToggle', initDiffFileViewToggle);
if (!document.querySelector('#diff-file-boxes')) return;
initRepoDiffConversationNav(); // "previous" and "next" buttons only appear on "diff" page
initDiffFileTree();
initDiffCommitSelect();
initRepoDiffShowMore();
initDiffHeaderPopup();
initViewedCheckboxListenerFor();
initExpandAndCollapseFilesButton();
initRepoDiffHashChangeListener();
registerGlobalSelectorFunc('#diff-file-boxes .diff-file-box', initRepoDiffFileBox);
addDelegatedEventListener(document, 'click', '.fold-file', (el) => {
invertFileFolding(el.closest('.file-content')!, el);
});
window.addEventListener('hashchange', onLocationHashChange);
onLocationHashChange();
}
-2
View File
@@ -6,7 +6,6 @@ import {initGlobalCopyToClipboardListener} from './modules/clipboard.ts';
import {initCopyContent} from './features/copycontent.ts';
import {initRepoGraphGit} from './features/repo-graph.ts';
import {initHeatmap} from './features/heatmap.ts';
import {initImageDiff} from './features/imagediff.ts';
import {initRepoMigration} from './features/repo-migration.ts';
import {initRepoProjectsView} from './features/repo-projects.ts';
import {initTableSort} from './features/tablesort.ts';
@@ -95,7 +94,6 @@ const initPerformanceTracer = callInitFunctions([
initCommmPageComponents,
initHeatmap,
initImageDiff,
initMarkupAnchors,
initMarkupContent,
initRefIssueContextPopup,