Refactor issue sidebar and fix various problems (#37045)

Fix various legacy problems, including: * Don't create default column when viewing an empty project * Fix layouts for Windows * Fix (partially) #15509 * Fix (partially) #17705 The sidebar refactoring: it is a clear partial-reloading approach, brings better user experiences, and it makes "Multiple projects" / "Project column on issue sidebar" feature easy to be added. --------- Signed-off-by: wxiaoguang <wxiaoguang@gmail.com>
2026-03-31 10:03:52 +08:00
parent daf581fa89
commit 6ca5573718
21 changed files with 317 additions and 179 deletions
@@ -0,0 +1,61 @@
+import {syncIssueMainContentTimelineItems} from './repo-issue-sidebar-combolist.ts';
+import {createElementFromHTML} from '../utils/dom.ts';
+
+describe('syncIssueMainContentTimelineItems', () => {
+  test('InsertNew', () => {
+    const oldContent = createElementFromHTML(`
+    <div>
+        <div class="timeline-item">First</div>
+        <div class="timeline-item" id="timeline-comments-end"></div>
+    </div>
+  `);
+    const newContent = createElementFromHTML(`
+    <div>
+        <div class="timeline-item" id="a">New</div>
+    </div>
+  `);
+    syncIssueMainContentTimelineItems(oldContent, newContent);
+    expect(oldContent.innerHTML.replace(/>\s+</g, '><').trim()).toBe(
+      `<div class="timeline-item">First</div>` +
+      `<div class="timeline-item" id="a">New</div>` +
+      `<div class="timeline-item" id="timeline-comments-end"></div>`,
+    );
+  });
+
+  test('Sync', () => {
+    const oldContent = createElementFromHTML(`
+    <div>
+      <div class="timeline-item">First</div>
+      <div class="timeline-item" id="it-1">Item 1</div>
+      <div class="timeline-item event" id="it-2">Item 2</div>
+      <div class="timeline-item" id="it-3">Item 3</div>
+      <div class="timeline-item event" id="it-4">Item 4</div>
+      <div class="timeline-item" id="timeline-comments-end"></div>
+      <div class="timeline-item">Other</div>
+    </div>
+  `);
+    const newContent = createElementFromHTML(`
+    <div>
+      <div class="timeline-item" id="it-1">New 1</div>
+      <div class="timeline-item event" id="it-2">New 2</div>
+      <div class="timeline-item" id="it-x">New X</div>
+    </div>
+  `);
+    syncIssueMainContentTimelineItems(oldContent, newContent);
+
+    // Item 1 won't be replaced because it's not an event
+    // Item 2 will be replaced with New 2
+    // Item 3 will be kept because it's not in new content
+    // Item 4 will be removed because it's not in new content, and it's an event
+    // New X will be inserted at the end of timeline items (before timeline-comments-end)
+    expect(oldContent.innerHTML.replace(/>\s+</g, '><').trim()).toBe(
+      `<div class="timeline-item">First</div>` +
+      `<div class="timeline-item" id="it-1">Item 1</div>` +
+      `<div class="timeline-item event" id="it-2">New 2</div>` +
+      `<div class="timeline-item" id="it-3">Item 3</div>` +
+      `<div class="timeline-item" id="it-x">New X</div>` +
+      `<div class="timeline-item" id="timeline-comments-end"></div>` +
+      `<div class="timeline-item">Other</div>`,
+    );
+  });
+});
@@ -1,25 +1,40 @@
 import {fomanticQuery} from '../modules/fomantic/base.ts';
-import {POST} from '../modules/fetch.ts';
+import {GET, POST} from '../modules/fetch.ts';
+import {showErrorToast} from '../modules/toast.ts';
 import {addDelegatedEventListener, queryElemChildren, queryElems, toggleElem} from '../utils/dom.ts';
+import {parseDom} from '../utils.ts';

-// if there are draft comments, confirm before reloading, to avoid losing comments
-function issueSidebarReloadConfirmDraftComment() {
-  const commentTextareas = [
-    document.querySelector<HTMLTextAreaElement>('.edit-content-zone:not(.tw-hidden) textarea'),
-    document.querySelector<HTMLTextAreaElement>('#comment-form textarea'),
-  ];
-  for (const textarea of commentTextareas) {
-    // Most users won't feel too sad if they lose a comment with 10 chars, they can re-type these in seconds.
-    // But if they have typed more (like 50) chars and the comment is lost, they will be very unhappy.
-    if (textarea && textarea.value.trim().length > 10) {
-      textarea.parentElement!.scrollIntoView();
-      if (!window.confirm('Page will be reloaded, but there are draft comments. Continuing to reload will discard the comments. Continue?')) {
-        return;
-      }
-      break;
+export function syncIssueMainContentTimelineItems(oldMainContent: Element, newMainContent: Element) {
+  // find the end of comments timeline by "id=timeline-comments-end" in current main content, and insert new items before it
+  const timelineEnd = oldMainContent.querySelector('.timeline-item[id="timeline-comments-end"]');
+  if (!timelineEnd) return;
+
+  const oldTimelineItems = oldMainContent.querySelectorAll(`.timeline-item[id]`);
+  for (const oldItem of oldTimelineItems) {
+    const oldItemId = oldItem.getAttribute('id')!;
+    const newItem = newMainContent.querySelector(`.timeline-item[id="${CSS.escape(oldItemId)}"]`);
+    if (oldItem.classList.contains('event') && !newItem) {
+      // if the item is not in new content, we want to remove it from old content only if it's an event item, otherwise we keep it
+      oldItem.remove();
    }
  }
-  window.location.reload();
+
+  const newTimelineItems = newMainContent.querySelectorAll(`.timeline-item[id]`);
+  for (const newItem of newTimelineItems) {
+    const newItemId = newItem.getAttribute('id')!;
+    const oldItem = oldMainContent.querySelector(`.timeline-item[id="${CSS.escape(newItemId)}"]`);
+    if (oldItem) {
+      if (oldItem.classList.contains('event')) {
+        // for event item (e.g.: "add & remove labels"), we want to replace the existing one if exists
+        // because the label operations can be merged into one event item, so the new item might be different from the old one
+        oldItem.replaceWith(newItem);
+        window.htmx.process(newItem);
+      }
+      continue;
+    }
+    timelineEnd.insertAdjacentElement('beforebegin', newItem);
+    window.htmx.process(newItem);
+  }
 }

 export class IssueSidebarComboList {
@@ -27,11 +42,14 @@ export class IssueSidebarComboList {
  updateAlgo: string;
  selectionMode: string;
  elDropdown: HTMLElement;
-  elList: HTMLElement;
+  elList: HTMLElement | null;
  elComboValue: HTMLInputElement;
  initialValues: string[];
  container: HTMLElement;

+  elIssueMainContent: HTMLElement;
+  elIssueSidebar: HTMLElement;
+
  constructor(container: HTMLElement) {
    this.container = container;
    this.updateUrl = container.getAttribute('data-update-url')!;
@@ -40,8 +58,11 @@ export class IssueSidebarComboList {
    if (!['single', 'multiple'].includes(this.selectionMode)) throw new Error(`Invalid data-update-on: ${this.selectionMode}`);
    if (!['diff', 'all'].includes(this.updateAlgo)) throw new Error(`Invalid data-update-algo: ${this.updateAlgo}`);
    this.elDropdown = container.querySelector<HTMLElement>(':scope > .ui.dropdown')!;
-    this.elList = container.querySelector<HTMLElement>(':scope > .ui.list')!;
+    this.elList = container.querySelector<HTMLElement>(':scope > .ui.list');
    this.elComboValue = container.querySelector<HTMLInputElement>(':scope > .combo-value')!;
+
+    this.elIssueMainContent = document.querySelector('.issue-content-left')!;
+    this.elIssueSidebar = document.querySelector('.issue-content-right')!;
  }

  collectCheckedValues() {
@@ -49,6 +70,7 @@ export class IssueSidebarComboList {
  }

  updateUiList(changedValues: Array<string>) {
+    if (!this.elList) return;
    const elEmptyTip = this.elList.querySelector('.item.empty-list')!;
    queryElemChildren(this.elList, '.item:not(.empty-list)', (el) => el.remove());
    for (const value of changedValues) {
@@ -62,22 +84,58 @@ export class IssueSidebarComboList {
    toggleElem(elEmptyTip, !hasItems);
  }

-  async updateToBackend(changedValues: Array<string>) {
+  async reloadPagePartially() {
+    const resp = await GET(window.location.href);
+    if (!resp.ok) throw new Error(`Failed to reload page: ${resp.statusText}`);
+    const doc = parseDom(await resp.text(), 'text/html');
+
+    // we can safely replace the whole right part (sidebar) because there are only some dropdowns and lists
+    const newSidebar = doc.querySelector('.issue-content-right')!;
+    this.elIssueSidebar.replaceWith(newSidebar);
+    window.htmx.process(newSidebar);
+
+    // for the main content (left side), at the moment we only support handling known timeline items
+    const newMainContent = doc.querySelector('.issue-content-left')!;
+    syncIssueMainContentTimelineItems(this.elIssueMainContent, newMainContent);
+  }
+
+  async sendRequestToBackend(changedValues: Array<string>): Promise<Response | null> {
+    let lastResp: Response | null = null;
    if (this.updateAlgo === 'diff') {
      for (const value of this.initialValues) {
        if (!changedValues.includes(value)) {
-          await POST(this.updateUrl, {data: new URLSearchParams({action: 'detach', id: value})});
+          lastResp = await POST(this.updateUrl, {data: new URLSearchParams({action: 'detach', id: value})});
+          if (!lastResp.ok) return lastResp;
        }
      }
      for (const value of changedValues) {
        if (!this.initialValues.includes(value)) {
-          await POST(this.updateUrl, {data: new URLSearchParams({action: 'attach', id: value})});
+          lastResp = await POST(this.updateUrl, {data: new URLSearchParams({action: 'attach', id: value})});
+          if (!lastResp.ok) return lastResp;
        }
      }
    } else {
-      await POST(this.updateUrl, {data: new URLSearchParams({id: changedValues.join(',')})});
+      lastResp = await POST(this.updateUrl, {data: new URLSearchParams({id: changedValues.join(',')})});
+    }
+    return lastResp;
+  }
+
+  async updateToBackend(changedValues: Array<string>) {
+    this.elIssueSidebar.classList.add('is-loading');
+    try {
+      const resp = await this.sendRequestToBackend(changedValues);
+      if (!resp) return; // no request sent, no need to reload
+      if (!resp.ok) {
+        showErrorToast(`Failed to update to backend: ${resp.statusText}`);
+        return;
+      }
+      await this.reloadPagePartially();
+    } catch (e) {
+      console.error('Failed to update to backend', e);
+      showErrorToast(`Failed to update to backend: ${e}`);
+    } finally {
+      this.elIssueSidebar.classList.remove('is-loading');
    }
-    issueSidebarReloadConfirmDraftComment();
  }

  async doUpdate() {
@@ -25,12 +25,15 @@ If there is `data-update-url`, it also calls backend to attach/detach the change
 Also, the changed items will be synchronized to the `ui list` items.
 The menu items must have correct `href`, otherwise the links of synchronized (cloned) items would be wrong.

+The `ui list` is optional, so a single dropdown can also work, to select items and update them to backend.
+
 Synchronization logic:
 * On page load:
  * If the dropdown menu contains checked items, there will be no synchronization.
    In this case, it's assumed that the dropdown menu is already in sync with the list.
  * If the dropdown menu doesn't contain checked items, it will use dropdown's value to mark the selected items as checked.
    And the selected (checked) items will be synchronized to the list.
+    Dropdown's value should be empty if the there is no dropdown item but a pre-defined list item need to be displayed.
 * On dropdown selection change:
  * The selected items will be synchronized to the list after the dropdown is hidden

@@ -1,12 +1,18 @@
 import {POST} from '../modules/fetch.ts';
 import {queryElems, toggleElem} from '../utils/dom.ts';
 import {IssueSidebarComboList} from './repo-issue-sidebar-combolist.ts';
+import {registerGlobalInitFunc} from '../modules/observer.ts';
+import {parseIssuePageInfo} from '../utils.ts';
+import {html} from '../utils/html.ts';
+import {fomanticQuery} from '../modules/fomantic/base.ts';
+import {showTemporaryTooltip} from '../modules/tippy.ts';

-function initBranchSelector() {
+const {appSubUrl} = window.config;
+
+function initRepoIssueBranchSelector(elSidebar: HTMLElement) {
  // TODO: RemoveIssueRef: see "repo/issue/branch_selector_field.tmpl"
-  const elSelectBranch = document.querySelector('.ui.dropdown.select-branch.branch-selector-dropdown');
+  const elSelectBranch = elSidebar.querySelector('.ui.dropdown.select-branch.branch-selector-dropdown');
  if (!elSelectBranch) return;
-
  const urlUpdateIssueRef = elSelectBranch.getAttribute('data-url-update-issueref');
  const elBranchMenu = elSelectBranch.querySelector('.reference-list-menu')!;
  queryElems(elBranchMenu, '.item:not(.no-select)', (el) => el.addEventListener('click', async function (e) {
@@ -30,23 +36,84 @@ function initBranchSelector() {
  }));
 }

-function initRepoIssueDue() {
-  const form = document.querySelector<HTMLFormElement>('.issue-due-form');
+function initRepoIssueDue(elSidebar: HTMLElement) {
+  const form = elSidebar.querySelector<HTMLFormElement>('.issue-due-form');
  if (!form) return;
  const deadline = form.querySelector<HTMLInputElement>('input[name=deadline]')!;
-  document.querySelector('.issue-due-edit')?.addEventListener('click', () => {
+  elSidebar.querySelector('.issue-due-edit')?.addEventListener('click', () => {
    toggleElem(form);
  });
-  document.querySelector('.issue-due-remove')?.addEventListener('click', () => {
+  elSidebar.querySelector('.issue-due-remove')?.addEventListener('click', () => {
    deadline.value = '';
    form.dispatchEvent(new Event('submit', {cancelable: true, bubbles: true}));
  });
 }

-export function initRepoIssueSidebar() {
-  initBranchSelector();
-  initRepoIssueDue();
+export function initRepoIssueSidebarDependency(elSidebar: HTMLElement) {
+  const elDropdown = elSidebar.querySelector('#new-dependency-drop-list');
+  if (!elDropdown) return;

-  // init the combo list: a dropdown for selecting items, and a list for showing selected items and related actions
-  queryElems<HTMLElement>(document, '.issue-sidebar-combo', (el) => new IssueSidebarComboList(el).init());
+  const issuePageInfo = parseIssuePageInfo();
+  const crossRepoSearch = elDropdown.getAttribute('data-issue-cross-repo-search');
+  let issueSearchUrl = `${issuePageInfo.repoLink}/issues/search?q={query}&type=${issuePageInfo.issueDependencySearchType}`;
+  if (crossRepoSearch === 'true') {
+    issueSearchUrl = `${appSubUrl}/issues/search?q={query}&priority_repo_id=${issuePageInfo.repoId}&type=${issuePageInfo.issueDependencySearchType}`;
+  }
+  fomanticQuery(elDropdown).dropdown({
+    fullTextSearch: true,
+    apiSettings: {
+      cache: false,
+      rawResponse: true,
+      url: issueSearchUrl,
+      onResponse(response: any) {
+        const filteredResponse = {success: true, results: [] as Array<Record<string, any>>};
+        const currIssueId = elDropdown.getAttribute('data-issue-id');
+        // Parse the response from the api to work with our dropdown
+        for (const issue of response) {
+          // Don't list current issue in the dependency list.
+          if (String(issue.id) === currIssueId) continue;
+          filteredResponse.results.push({
+            value: issue.id,
+            name: html`<div class="gt-ellipsis">#${issue.number} ${issue.title}</div><div class="text small tw-break-anywhere">${issue.repository.full_name}</div>`,
+          });
+        }
+        return filteredResponse;
+      },
+    },
+  });
+}
+
+export function initRepoPullRequestAllowMaintainerEdit(elSidebar: HTMLElement) {
+  const wrapper = elSidebar.querySelector('#allow-edits-from-maintainers')!;
+  if (!wrapper) return;
+  const checkbox = wrapper.querySelector<HTMLInputElement>('input[type="checkbox"]')!;
+  checkbox.addEventListener('input', async () => {
+    const url = `${wrapper.getAttribute('data-url')}/set_allow_maintainer_edit`;
+    wrapper.classList.add('is-loading');
+    try {
+      const resp = await POST(url, {data: new URLSearchParams({allow_maintainer_edit: String(checkbox.checked)})});
+      if (!resp.ok) {
+        throw new Error('Failed to update maintainer edit permission');
+      }
+      const data = await resp.json();
+      checkbox.checked = data.allow_maintainer_edit;
+    } catch (error) {
+      checkbox.checked = !checkbox.checked;
+      console.error(error);
+      showTemporaryTooltip(wrapper, wrapper.getAttribute('data-prompt-error')!);
+    } finally {
+      wrapper.classList.remove('is-loading');
+    }
+  });
+}
+
+export function initRepoIssueSidebar() {
+  registerGlobalInitFunc('initRepoIssueSidebar', (elSidebar) => {
+    initRepoIssueBranchSelector(elSidebar);
+    initRepoIssueDue(elSidebar);
+    initRepoIssueSidebarDependency(elSidebar);
+    initRepoPullRequestAllowMaintainerEdit(elSidebar);
+    // init the combo list: a dropdown for selecting items, and a list for showing selected items and related actions
+    queryElems(elSidebar, '.issue-sidebar-combo', (el) => new IssueSidebarComboList(el).init());
+  });
 }
@@ -1,5 +1,5 @@
-import {html, htmlEscape} from '../utils/html.ts';
-import {createTippy, showTemporaryTooltip} from '../modules/tippy.ts';
+import {htmlEscape} from '../utils/html.ts';
+import {createTippy} from '../modules/tippy.ts';
 import {
  addDelegatedEventListener,
  createElementFromHTML,
@@ -10,7 +10,7 @@ import {
 } from '../utils/dom.ts';
 import {setFileFolding} from './file-fold.ts';
 import {ComboMarkdownEditor, getComboMarkdownEditor, initComboMarkdownEditor} from './comp/ComboMarkdownEditor.ts';
-import {parseIssuePageInfo, toAbsoluteUrl} from '../utils.ts';
+import {toAbsoluteUrl} from '../utils.ts';
 import {GET, POST} from '../modules/fetch.ts';
 import {showErrorToast} from '../modules/toast.ts';
 import {initRepoIssueSidebar} from './repo-issue-sidebar.ts';
@@ -20,40 +20,6 @@ import {registerGlobalInitFunc} from '../modules/observer.ts';

 const {appSubUrl} = window.config;

-export function initRepoIssueSidebarDependency() {
-  const elDropdown = document.querySelector('#new-dependency-drop-list');
-  if (!elDropdown) return;
-
-  const issuePageInfo = parseIssuePageInfo();
-  const crossRepoSearch = elDropdown.getAttribute('data-issue-cross-repo-search');
-  let issueSearchUrl = `${issuePageInfo.repoLink}/issues/search?q={query}&type=${issuePageInfo.issueDependencySearchType}`;
-  if (crossRepoSearch === 'true') {
-    issueSearchUrl = `${appSubUrl}/issues/search?q={query}&priority_repo_id=${issuePageInfo.repoId}&type=${issuePageInfo.issueDependencySearchType}`;
-  }
-  fomanticQuery(elDropdown).dropdown({
-    fullTextSearch: true,
-    apiSettings: {
-      cache: false,
-      rawResponse: true,
-      url: issueSearchUrl,
-      onResponse(response: any) {
-        const filteredResponse = {success: true, results: [] as Array<Record<string, any>>};
-        const currIssueId = elDropdown.getAttribute('data-issue-id');
-        // Parse the response from the api to work with our dropdown
-        for (const issue of response) {
-          // Don't list current issue in the dependency list.
-          if (String(issue.id) === currIssueId) continue;
-          filteredResponse.results.push({
-            value: issue.id,
-            name: html`<div class="gt-ellipsis">#${issue.number} ${issue.title}</div><div class="text small tw-break-anywhere">${issue.repository.full_name}</div>`,
-          });
-        }
-        return filteredResponse;
-      },
-    },
-  });
-}
-
 function initRepoIssueLabelFilter(elDropdown: HTMLElement) {
  const url = new URL(window.location.href);
  const showArchivedLabels = url.searchParams.get('archived_labels') === 'true';
@@ -197,32 +163,6 @@ export function initRepoIssueCodeCommentCancel() {
  });
 }

-export function initRepoPullRequestAllowMaintainerEdit() {
-  const wrapper = document.querySelector('#allow-edits-from-maintainers')!;
-  if (!wrapper) return;
-  const checkbox = wrapper.querySelector<HTMLInputElement>('input[type="checkbox"]')!;
-  checkbox.addEventListener('input', async () => {
-    const url = `${wrapper.getAttribute('data-url')}/set_allow_maintainer_edit`;
-    wrapper.classList.add('is-loading');
-    try {
-      const resp = await POST(url, {data: new URLSearchParams({
-        allow_maintainer_edit: String(checkbox.checked),
-      })});
-      if (!resp.ok) {
-        throw new Error('Failed to update maintainer edit permission');
-      }
-      const data = await resp.json();
-      checkbox.checked = data.allow_maintainer_edit;
-    } catch (error) {
-      checkbox.checked = !checkbox.checked;
-      console.error(error);
-      showTemporaryTooltip(wrapper, wrapper.getAttribute('data-prompt-error')!);
-    } finally {
-      wrapper.classList.remove('is-loading');
-    }
-  });
-}
-
 export function initRepoIssueComments() {
  if (!document.querySelector('.repository.view.issue .timeline')) return;

@@ -566,6 +506,8 @@ function initIssueTemplateCommentEditors(commentForm: HTMLFormElement) {
 }

 export function initRepoCommentFormAndSidebar() {
+  initRepoIssueSidebar();
+
  const commentForm = document.querySelector<HTMLFormElement>('.comment.form');
  if (!commentForm) return;

@@ -576,6 +518,4 @@ export function initRepoCommentFormAndSidebar() {
    // it's quite unclear about the "comment form" elements, sometimes it's for issue comment, sometimes it's for file editor/uploader message
    initSingleCommentEditor(commentForm);
  }
-
-  initRepoIssueSidebar();
 }