Refactor repo-diff.ts (#33746)

Remove jQuery

web_src/js/features/repo-diff.ts

@@ -1,4 +1,3 @@
-import $ from 'jquery';
 import {initCompReactionSelector} from './comp/ReactionSelector.ts';
 import {initRepoIssueContentHistory} from './repo-issue-content.ts';
 import {initDiffFileTree} from './repo-diff-filetree.ts';
@@ -7,35 +6,28 @@ import {validateTextareaNonEmpty} from './comp/ComboMarkdownEditor.ts';
 import {initViewedCheckboxListenerFor, countAndUpdateViewedFiles, initExpandAndCollapseFilesButton} from './pull-view-file.ts';
 import {initImageDiff} from './imagediff.ts';
 import {showErrorToast} from '../modules/toast.ts';
-import {
-  submitEventSubmitter,
-  queryElemSiblings,
-  hideElem,
-  showElem,
-  animateOnce,
-  addDelegatedEventListener,
-  createElementFromHTML,
-} from '../utils/dom.ts';
+import {submitEventSubmitter, queryElemSiblings, hideElem, showElem, animateOnce, addDelegatedEventListener, createElementFromHTML, queryElems} from '../utils/dom.ts';
 import {POST, GET} from '../modules/fetch.ts';
 import {fomanticQuery} from '../modules/fomantic/base.ts';
 import {createTippy} from '../modules/tippy.ts';
 import {invertFileFolding} from './file-fold.ts';
+import {parseDom} from '../utils.ts';
 
 const {i18n} = window.config;
 
 function initRepoDiffFileViewToggle() {
-  $('.file-view-toggle').on('click', function () {
-    for (const el of queryElemSiblings(this)) {
-      el.classList.remove('active');
-    }
-    this.classList.add('active');
+  // switch between "rendered" and "source", for image and CSV files
+  // FIXME: this event listener is not correctly added to "load more files"
+  queryElems(document, '.file-view-toggle', (btn) => btn.addEventListener('click', () => {
+    queryElemSiblings(btn, '.file-view-toggle', (el) => el.classList.remove('active'));
+    btn.classList.add('active');
 
-    const target = document.querySelector(this.getAttribute('data-toggle-selector'));
-    if (!target) return;
+    const target = document.querySelector(btn.getAttribute('data-toggle-selector'));
+    if (!target) throw new Error('Target element not found');
 
     hideElem(queryElemSiblings(target));
     showElem(target);
-  });
+  }));
 }
 
 function initRepoDiffConversationForm() {
@@ -103,22 +95,23 @@ function initRepoDiffConversationForm() {
     }
   });
 
-  $(document).on('click', '.resolve-conversation', async function (e) {
+  addDelegatedEventListener(document, 'click', '.resolve-conversation', async (el, e) => {
     e.preventDefault();
-    const comment_id = $(this).data('comment-id');
-    const origin = $(this).data('origin');
-    const action = $(this).data('action');
-    const url = $(this).data('update-url');
+    const comment_id = el.getAttribute('data-comment-id');
+    const origin = el.getAttribute('data-origin');
+    const action = el.getAttribute('data-action');
+    const url = el.getAttribute('data-update-url');
 
     try {
       const response = await POST(url, {data: new URLSearchParams({origin, action, comment_id})});
       const data = await response.text();
 
-      if ($(this).closest('.conversation-holder').length) {
-        const $conversation = $(data);
-        $(this).closest('.conversation-holder').replaceWith($conversation);
-        $conversation.find('.dropdown').dropdown();
-        initCompReactionSelector($conversation[0]);
+      const elConversationHolder = el.closest('.conversation-holder');
+      if (elConversationHolder) {
+        const elNewConversation = createElementFromHTML(data);
+        elConversationHolder.replaceWith(elNewConversation);
+        queryElems(elConversationHolder, '.ui.dropdown:not(.custom)', (el) => fomanticQuery(el).dropdown());
+        initCompReactionSelector(elNewConversation);
       } else {
         window.location.reload();
       }
@@ -128,24 +121,19 @@ function initRepoDiffConversationForm() {
   });
 }
 
-export function initRepoDiffConversationNav() {
+function initRepoDiffConversationNav() {
   // Previous/Next code review conversation
-  $(document).on('click', '.previous-conversation', (e) => {
-    const $conversation = $(e.currentTarget).closest('.comment-code-cloud');
-    const $conversations = $('.comment-code-cloud:not(.tw-hidden)');
-    const index = $conversations.index($conversation);
-    const previousIndex = index > 0 ? index - 1 : $conversations.length - 1;
-    const $previousConversation = $conversations.eq(previousIndex);
-    const anchor = $previousConversation.find('.comment').first()[0].getAttribute('id');
-    window.location.href = `#${anchor}`;
-  });
-  $(document).on('click', '.next-conversation', (e) => {
-    const $conversation = $(e.currentTarget).closest('.comment-code-cloud');
-    const $conversations = $('.comment-code-cloud:not(.tw-hidden)');
-    const index = $conversations.index($conversation);
-    const nextIndex = index < $conversations.length - 1 ? index + 1 : 0;
-    const $nextConversation = $conversations.eq(nextIndex);
-    const anchor = $nextConversation.find('.comment').first()[0].getAttribute('id');
+  addDelegatedEventListener(document, 'click', '.previous-conversation, .next-conversation', (el, e) => {
+    e.preventDefault();
+    const isPrevious = el.matches('.previous-conversation');
+    const elCurConversation = el.closest('.comment-code-cloud');
+    const elAllConversations = document.querySelectorAll('.comment-code-cloud:not(.tw-hidden)');
+    const index = Array.from(elAllConversations).indexOf(elCurConversation);
+    const previousIndex = index > 0 ? index - 1 : elAllConversations.length - 1;
+    const nextIndex = index < elAllConversations.length - 1 ? index + 1 : 0;
+    const navIndex = isPrevious ? previousIndex : nextIndex;
+    const elNavConversation = elAllConversations[navIndex];
+    const anchor = elNavConversation.querySelector('.comment').id;
     window.location.href = `#${anchor}`;
   });
 }
@@ -161,7 +149,7 @@ function initDiffHeaderPopup() {
 
 // Will be called when the show more (files) button has been pressed
 function onShowMoreFiles() {
-  // FIXME: here the init calls are incomplete: at least it misses dropdown & initCompReactionSelector
+  // FIXME: here the init calls are incomplete: at least it misses dropdown & initCompReactionSelector & initRepoDiffFileViewToggle
   initRepoIssueContentHistory();
   initViewedCheckboxListenerFor();
   countAndUpdateViewedFiles();
@@ -179,10 +167,11 @@ async function loadMoreFiles(btn: Element): Promise<boolean> {
   try {
     const response = await GET(url);
     const resp = await response.text();
-    const $resp = $(resp);
+    const respDoc = parseDom(resp, '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
-    $('#diff-incomplete').replaceWith($resp.find('#diff-file-boxes').children());
+    document.querySelector('#diff-incomplete').replaceWith(...Array.from(respFileBoxes.children));
     onShowMoreFiles();
     return true;
   } catch (error) {
@@ -200,31 +189,27 @@ function initRepoDiffShowMore() {
     loadMoreFiles(el);
   });
 
-  $(document).on('click', 'a.diff-load-button', async (e) => {
+  addDelegatedEventListener(document, 'click', 'a.diff-load-button', async (el, e) => {
     e.preventDefault();
-    const $target = $(e.target);
-
-    if (e.target.classList.contains('disabled')) {
-      return;
-    }
-
-    e.target.classList.add('disabled');
+    if (el.classList.contains('disabled')) return;
 
-    const url = $target.data('href');
+    el.classList.add('disabled');
+    const url = el.getAttribute('data-href');
 
     try {
       const response = await GET(url);
       const resp = await response.text();
-
-      if (!resp) {
-        return;
-      }
-      $target.parent().replaceWith($(resp).find('#diff-file-boxes .diff-file-body .file-body').children());
+      const respDoc = parseDom(resp, 'text/html');
+      const respFileBody = respDoc.querySelector('#diff-file-boxes .diff-file-body .file-body');
+      el.parentElement.replaceWith(...Array.from(respFileBody.children));
+      // 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 {
-      e.target.classList.remove('disabled');
+      el.classList.remove('disabled');
     }
   });
 }
@@ -262,8 +247,10 @@ function initRepoDiffHashChangeListener() {
 }
 
 export function initRepoDiffView() {
-  initRepoDiffConversationForm();
-  if (!$('#diff-file-boxes').length) return;
+  initRepoDiffConversationForm(); // such form appears on the "conversation" page and "diff" page
+
+  if (!document.querySelector('#diff-file-boxes')) return;
+  initRepoDiffConversationNav(); // "previous" and "next" buttons only appear on "diff" page
   initDiffFileTree();
   initDiffCommitSelect();
   initRepoDiffShowMore();

web_src/js/features/repo-legacy.ts

@@ -9,7 +9,6 @@ import {initUnicodeEscapeButton} from './repo-unicode-escape.ts';
 import {initRepoCloneButtons} from './repo-common.ts';
 import {initCitationFileCopyContent} from './citation.ts';
 import {initCompLabelEdit} from './comp/LabelEdit.ts';
-import {initRepoDiffConversationNav} from './repo-diff.ts';
 import {initCompReactionSelector} from './comp/ReactionSelector.ts';
 import {initRepoSettings} from './repo-settings.ts';
 import {initRepoPullRequestMergeForm} from './repo-issue-pr-form.ts';
@@ -64,7 +63,6 @@ export function initRepository() {
     initRepoIssueWipToggle();
     initRepoIssueComments();
 
-    initRepoDiffConversationNav();
     initRepoIssueReferenceIssue();
 
     initRepoIssueCommentDelete();