Refactor legacy line-number and scroll code (#33094)

1. remove jquery
2. rewrite the "line number selection", fix various edge cases
3. fix the scroll

web_src/css/base.css

@@ -336,8 +336,13 @@ a.label,
   border-color: var(--color-secondary);
 }
 
+.ui.dropdown .menu > .header {
+  text-transform: none; /* reset fomantic's "uppercase" */
+}
+
 .ui.dropdown .menu > .header:not(.ui) {
   color: var(--color-text);
+  font-size: 0.95em; /* reset fomantic's small font-size */
 }
 
 .ui.dropdown .menu > .item {
@@ -691,10 +696,6 @@ input:-webkit-autofill:active,
   box-shadow: 0 6px 18px var(--color-shadow) !important;
 }
 
-.ui.dropdown .menu > .header {
-  font-size: 0.8em;
-}
-
 .ui .text.left {
   text-align: left !important;
 }

web_src/js/features/repo-code.test.ts

@@ -1,17 +0,0 @@
-import {singleAnchorRegex, rangeAnchorRegex} from './repo-code.ts';
-
-test('singleAnchorRegex', () => {
-  expect(singleAnchorRegex.test('#L0')).toEqual(false);
-  expect(singleAnchorRegex.test('#L1')).toEqual(true);
-  expect(singleAnchorRegex.test('#L01')).toEqual(false);
-  expect(singleAnchorRegex.test('#n0')).toEqual(false);
-  expect(singleAnchorRegex.test('#n1')).toEqual(true);
-  expect(singleAnchorRegex.test('#n01')).toEqual(false);
-});
-
-test('rangeAnchorRegex', () => {
-  expect(rangeAnchorRegex.test('#L0-L10')).toEqual(false);
-  expect(rangeAnchorRegex.test('#L1-L10')).toEqual(true);
-  expect(rangeAnchorRegex.test('#L01-L10')).toEqual(false);
-  expect(rangeAnchorRegex.test('#L1-L01')).toEqual(false);
-});

web_src/js/features/repo-code.ts

@@ -1,12 +1,8 @@
-import $ from 'jquery';
 import {svg} from '../svg.ts';
-import {invertFileFolding} from './file-fold.ts';
 import {createTippy} from '../modules/tippy.ts';
 import {clippie} from 'clippie';
 import {toAbsoluteUrl} from '../utils.ts';
-
-export const singleAnchorRegex = /^#(L|n)([1-9][0-9]*)$/;
-export const rangeAnchorRegex = /^#(L[1-9][0-9]*)-(L[1-9][0-9]*)$/;
+import {addDelegatedEventListener} from '../utils/dom.ts';
 
 function changeHash(hash: string) {
   if (window.history.pushState) {
@@ -16,20 +12,11 @@ function changeHash(hash: string) {
   }
 }
 
-function isBlame() {
-  return Boolean(document.querySelector('div.blame'));
-}
+// it selects the code lines defined by range: `L1-L3` (3 lines) or `L2` (singe line)
+function selectRange(range: string): Element {
+  for (const el of document.querySelectorAll('.code-view tr.active')) el.classList.remove('active');
+  const elLineNums = document.querySelectorAll(`.code-view td.lines-num span[data-line-number]`);
 
-function getLineEls() {
-  return document.querySelectorAll(`.code-view td.lines-code${isBlame() ? '.blame-code' : ''}`);
-}
-
-function selectRange($linesEls, $selectionEndEl, $selectionStartEls?) {
-  for (const el of $linesEls) {
-    el.closest('tr').classList.remove('active');
-  }
-
-  // add hashchange to permalink
   const refInNewIssue = document.querySelector('a.ref-in-new-issue');
   const copyPermalink = document.querySelector('a.copy-line-permalink');
   const viewGitBlame = document.querySelector('a.view_git_blame');
@@ -59,37 +46,30 @@ function selectRange($linesEls, $selectionEndEl, $selectionStartEls?) {
     copyPermalink.setAttribute('data-url', link);
   };
 
-  if ($selectionStartEls) {
-    let a = parseInt($selectionEndEl[0].getAttribute('rel').slice(1));
-    let b = parseInt($selectionStartEls[0].getAttribute('rel').slice(1));
-    let c;
-    if (a !== b) {
-      if (a > b) {
-        c = a;
-        a = b;
-        b = c;
-      }
-      const classes = [];
-      for (let i = a; i <= b; i++) {
-        classes.push(`[rel=L${i}]`);
-      }
-      $linesEls.filter(classes.join(',')).each(function () {
-        this.closest('tr').classList.add('active');
-      });
-      changeHash(`#L${a}-L${b}`);
+  const rangeFields = range ? range.split('-') : [];
+  const start = rangeFields[0] ?? '';
+  if (!start) return null;
+  const stop = rangeFields[1] || start;
 
-      updateIssueHref(`L${a}-L${b}`);
-      updateViewGitBlameFragment(`L${a}-L${b}`);
-      updateCopyPermalinkUrl(`L${a}-L${b}`);
-      return;
+  // format is i.e. 'L14-L26'
+  let startLineNum = parseInt(start.substring(1));
+  let stopLineNum = parseInt(stop.substring(1));
+  if (startLineNum > stopLineNum) {
+    const tmp = startLineNum;
+    startLineNum = stopLineNum;
+    stopLineNum = tmp;
+    range = `${stop}-${start}`;
   }
-  }
-  $selectionEndEl[0].closest('tr').classList.add('active');
-  changeHash(`#${$selectionEndEl[0].getAttribute('rel')}`);
 
-  updateIssueHref($selectionEndEl[0].getAttribute('rel'));
-  updateViewGitBlameFragment($selectionEndEl[0].getAttribute('rel'));
-  updateCopyPermalinkUrl($selectionEndEl[0].getAttribute('rel'));
+  const first = elLineNums[startLineNum - 1] ?? null;
+  for (let i = startLineNum - 1; i <= stopLineNum - 1 && i < elLineNums.length; i++) {
+    elLineNums[i].closest('tr').classList.add('active');
+  }
+  changeHash(`#${range}`);
+  updateIssueHref(range);
+  updateViewGitBlameFragment(range);
+  updateCopyPermalinkUrl(range);
+  return first;
 }
 
 function showLineButton() {
@@ -103,6 +83,8 @@ function showLineButton() {
 
   // find active row and add button
   const tr = document.querySelector('.code-view tr.active');
+  if (!tr) return;
+
   const td = tr.querySelector('td.lines-num');
   const btn = document.createElement('button');
   btn.classList.add('code-line-button', 'ui', 'basic', 'button');
@@ -128,62 +110,36 @@ function showLineButton() {
 }
 
 export function initRepoCodeView() {
-  if ($('.code-view .lines-num').length > 0) {
-    $(document).on('click', '.lines-num span', function (e) {
-      const linesEls = getLineEls();
-      const selectedEls = Array.from(linesEls).filter((el) => {
-        return el.matches(`[rel=${this.getAttribute('id')}]`);
-      });
+  if (!document.querySelector('.code-view .lines-num')) return;
 
-      let from;
-      if (e.shiftKey) {
-        from = Array.from(linesEls).filter((el) => {
-          return el.closest('tr').classList.contains('active');
-        });
+  let selRangeStart: string;
+  addDelegatedEventListener(document, 'click', '.lines-num span', (el: HTMLElement, e: KeyboardEvent) => {
+    if (!selRangeStart || !e.shiftKey) {
+      selRangeStart = el.getAttribute('id');
+      selectRange(selRangeStart);
+    } else {
+      const selRangeStop = el.getAttribute('id');
+      selectRange(`${selRangeStart}-${selRangeStop}`);
     }
-      selectRange($(linesEls), $(selectedEls), from ? $(from) : null);
     window.getSelection().removeAllRanges();
     showLineButton();
   });
 
-    $(window).on('hashchange', () => {
-      let m = rangeAnchorRegex.exec(window.location.hash);
-      const $linesEls = $(getLineEls());
-      let $first;
-      if (m) {
-        $first = $linesEls.filter(`[rel=${m[1]}]`);
-        if ($first.length) {
-          selectRange($linesEls, $first, $linesEls.filter(`[rel=${m[2]}]`));
-
-          // show code view menu marker (don't show in blame page)
-          if (!isBlame()) {
+  const onHashChange = () => {
+    if (!window.location.hash) return;
+    const range = window.location.hash.substring(1);
+    const first = selectRange(range);
+    if (first) {
+      // set scrollRestoration to 'manual' when there is a hash in url, so that the scroll position will not be remembered after refreshing
+      if (window.history.scrollRestoration !== 'manual') window.history.scrollRestoration = 'manual';
+      first.scrollIntoView({block: 'start'});
       showLineButton();
     }
+  };
+  onHashChange();
+  window.addEventListener('hashchange', onHashChange);
 
-          $('html, body').scrollTop($first.offset().top - 200);
-          return;
-        }
-      }
-      m = singleAnchorRegex.exec(window.location.hash);
-      if (m) {
-        $first = $linesEls.filter(`[rel=L${m[2]}]`);
-        if ($first.length) {
-          selectRange($linesEls, $first);
-
-          // show code view menu marker (don't show in blame page)
-          if (!isBlame()) {
-            showLineButton();
-          }
-
-          $('html, body').scrollTop($first.offset().top - 200);
-        }
-      }
-    }).trigger('hashchange');
-  }
-  $(document).on('click', '.fold-file', ({currentTarget}) => {
-    invertFileFolding(currentTarget.closest('.file-content'), currentTarget);
-  });
-  $(document).on('click', '.copy-line-permalink', async ({currentTarget}) => {
-    await clippie(toAbsoluteUrl(currentTarget.getAttribute('data-url')));
+  addDelegatedEventListener(document, 'click', '.copy-line-permalink', (el) => {
+    clippie(toAbsoluteUrl(el.getAttribute('data-url')));
   });
 }

web_src/js/features/repo-diff.ts

@@ -19,6 +19,7 @@ import {
 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';
 
 const {pageData, i18n} = window.config;
 
@@ -244,4 +245,8 @@ export function initRepoDiffView() {
   initRepoDiffFileViewToggle();
   initViewedCheckboxListenerFor();
   initExpandAndCollapseFilesButton();
+
+  addDelegatedEventListener(document, 'click', '.fold-file', (el) => {
+    invertFileFolding(el.closest('.file-content'), el);
+  });
 }

web_src/js/features/repo-issue.ts

@@ -373,10 +373,6 @@ export async function handleReply(el) {
 
 export function initRepoPullRequestReview() {
   if (window.location.hash && window.location.hash.startsWith('#issuecomment-')) {
-    // set scrollRestoration to 'manual' when there is a hash in url, so that the scroll position will not be remembered after refreshing
-    if (window.history.scrollRestoration !== 'manual') {
-      window.history.scrollRestoration = 'manual';
-    }
     const commentDiv = document.querySelector(window.location.hash);
     if (commentDiv) {
       // get the name of the parent id
@@ -384,14 +380,6 @@ export function initRepoPullRequestReview() {
       if (groupID && groupID.startsWith('code-comments-')) {
         const id = groupID.slice(14);
         const ancestorDiffBox = commentDiv.closest('.diff-file-box');
-        // on pages like conversation, there is no diff header
-        const diffHeader = ancestorDiffBox?.querySelector('.diff-file-header');
-
-        // offset is for scrolling
-        let offset = 30;
-        if (diffHeader) {
-          offset += $('.diff-detail-box').outerHeight() + $(diffHeader).outerHeight();
-        }
 
         hideElem(`#show-outdated-${id}`);
         showElem(`#code-comments-${id}, #code-preview-${id}, #hide-outdated-${id}`);
@@ -399,12 +387,11 @@ export function initRepoPullRequestReview() {
         if (ancestorDiffBox?.getAttribute('data-folded') === 'true') {
           setFileFolding(ancestorDiffBox, ancestorDiffBox.querySelector('.fold-file'), false);
         }
-
-        window.scrollTo({
-          top: $(commentDiv).offset().top - offset,
-          behavior: 'instant',
-        });
       }
+      // set scrollRestoration to 'manual' when there is a hash in url, so that the scroll position will not be remembered after refreshing
+      if (window.history.scrollRestoration !== 'manual') window.history.scrollRestoration = 'manual';
+      // wait for a while because some elements (eg: image, editor, etc.) may change the viewport's height.
+      setTimeout(() => commentDiv.scrollIntoView({block: 'start'}), 100);
     }
   }