From 4e4ea75fb95c316fffc0ae00c0f4d3037e727902 Mon Sep 17 00:00:00 2001
From: Lunny Xiao <xiaolunwen@gmail.com>
Date: Sun, 2 Aug 2026 06:19:57 -0700
Subject: [PATCH] refactor(diff): drive diff DOM init from the global selector
 observer (#38740)

Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
---
 templates/repo/diff/box.tmpl          |  14 +--
 templates/repo/diff/image_diff.tmpl   |   2 +-
 web_src/js/features/imagediff.ts      |   9 +-
 web_src/js/features/pull-view-file.ts |  76 +++++++--------
 web_src/js/features/repo-diff.ts      | 130 +++++++++++---------------
 web_src/js/index.ts                   |   2 -
 6 files changed, 97 insertions(+), 136 deletions(-)

diff --git a/templates/repo/diff/box.tmpl b/templates/repo/diff/box.tmpl
index 7f3090561c..4ed7fe9ab7 100644
--- a/templates/repo/diff/box.tmpl
+++ b/templates/repo/diff/box.tmpl
@@ -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}}
diff --git a/templates/repo/diff/image_diff.tmpl b/templates/repo/diff/image_diff.tmpl
index affaeeb3e3..2c28e0eb93 100644
--- a/templates/repo/diff/image_diff.tmpl
+++ b/templates/repo/diff/image_diff.tmpl
@@ -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}}"
diff --git a/web_src/js/features/imagediff.ts b/web_src/js/features/imagediff.ts
index 462a1a3cee..d38dbde53c 100644
--- a/web_src/js/features/imagediff.ts
+++ b/web_src/js/features/imagediff.ts
@@ -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);
 }
diff --git a/web_src/js/features/pull-view-file.ts b/web_src/js/features/pull-view-file.ts
index d93d52549f..e955e3354e 100644
--- a/web_src/js/features/pull-view-file.ts
+++ b/web_src/js/features/pull-view-file.ts
@@ -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() {
diff --git a/web_src/js/features/repo-diff.ts b/web_src/js/features/repo-diff.ts
index 89af812b67..c42b57ff09 100644
--- a/web_src/js/features/repo-diff.ts
+++ b/web_src/js/features/repo-diff.ts
@@ -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();
 }
diff --git a/web_src/js/index.ts b/web_src/js/index.ts
index 3289238b0d..ef586d2201 100644
--- a/web_src/js/index.ts
+++ b/web_src/js/index.ts
@@ -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,