From 8db66ad00b591af46aa51e520d216bd2b415f0f4 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Thu, 21 Nov 2024 20:24:06 +0800 Subject: [PATCH 1/6] fix --- web_src/js/features/repo-diff.ts | 78 +++++++++++++++---------------- web_src/js/features/repo-issue.ts | 48 ++++++++----------- web_src/js/utils/dom.test.ts | 1 + web_src/js/utils/dom.ts | 17 ++++++- 4 files changed, 76 insertions(+), 68 deletions(-) diff --git a/web_src/js/features/repo-diff.ts b/web_src/js/features/repo-diff.ts index a0bd9955fe468..3f0df6433fee9 100644 --- a/web_src/js/features/repo-diff.ts +++ b/web_src/js/features/repo-diff.ts @@ -7,30 +7,19 @@ 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} from '../utils/dom.ts'; +import { + submitEventSubmitter, + queryElemSiblings, + hideElem, + showElem, + animateOnce, + addElemsEventListener, createElementFromHTML, +} from '../utils/dom.ts'; import {POST, GET} from '../modules/fetch.ts'; +import {fomanticQuery} from '../modules/fomantic/base.ts'; const {pageData, i18n} = window.config; -function initRepoDiffReviewButton() { - const reviewBox = document.querySelector('#review-box'); - if (!reviewBox) return; - - const counter = reviewBox.querySelector('.review-comments-counter'); - if (!counter) return; - - $(document).on('click', 'button[name="pending_review"]', (e) => { - const $form = $(e.target).closest('form'); - // Watch for the form's submit event. - $form.on('submit', () => { - const num = parseInt(counter.getAttribute('data-pending-comment-number')) + 1 || 1; - counter.setAttribute('data-pending-comment-number', num); - counter.textContent = num; - animateOnce(reviewBox, 'pulse-1p5-200'); - }); - }); -} - function initRepoDiffFileViewToggle() { $('.file-view-toggle').on('click', function () { for (const el of queryElemSiblings(this)) { @@ -47,19 +36,15 @@ function initRepoDiffFileViewToggle() { } function initRepoDiffConversationForm() { - $(document).on('submit', '.conversation-holder form', async (e) => { + addElemsEventListener(document, 'submit', '.conversation-holder form', async (form, e) => { e.preventDefault(); + const textArea = form.querySelector('textarea'); + if (!validateTextareaNonEmpty(textArea)) return; + if (form.classList.contains('is-loading')) return; - const $form = $(e.target); - const textArea = e.target.querySelector('textarea'); - if (!validateTextareaNonEmpty(textArea)) { - return; - } - - if (e.target.classList.contains('is-loading')) return; try { - e.target.classList.add('is-loading'); - const formData = new FormData($form[0]); + form.classList.add('is-loading'); + const formData = new FormData(form); // if the form is submitted by a button, append the button's name and value to the form data const submitter = submitEventSubmitter(e); @@ -68,26 +53,42 @@ function initRepoDiffConversationForm() { formData.append(submitter.name, submitter.value); } - const response = await POST(e.target.getAttribute('action'), {data: formData}); - const $newConversationHolder = $(await response.text()); - const {path, side, idx} = $newConversationHolder.data(); + const trLineType = form.closest('tr').getAttribute('data-line-type'); + const response = await POST(form.getAttribute('action'), {data: formData}); + const newConversationHolder = createElementFromHTML(await response.text()); + const path = newConversationHolder.getAttribute('data-path'); + const side = newConversationHolder.getAttribute('data-side'); + const idx = newConversationHolder.getAttribute('data-idx'); + + form.closest('.conversation-holder').replaceWith(newConversationHolder); + form = null; // prevent further usage of the form because it should have been replaced - $form.closest('.conversation-holder').replaceWith($newConversationHolder); let selector; - if ($form.closest('tr').data('line-type') === 'same') { + if (trLineType === 'same') { selector = `[data-path="${path}"] .add-code-comment[data-idx="${idx}"]`; } else { selector = `[data-path="${path}"] .add-code-comment[data-side="${side}"][data-idx="${idx}"]`; } for (const el of document.querySelectorAll(selector)) { - el.classList.add('tw-invisible'); + el.classList.add('tw-invisible'); // TODO need to figure out why + } + fomanticQuery(newConversationHolder.querySelectorAll('.ui.dropdown')).dropdown(); + + // the default behavior is to add a pending review, so if no submitter, it also means "pending_review" + if (!submitter || submitter?.matches('button[name="pending_review"]')) { + const reviewBox = document.querySelector('#review-box'); + const counter = reviewBox?.querySelector('.review-comments-counter'); + if (!counter) return; + const num = parseInt(counter.getAttribute('data-pending-comment-number')) + 1 || 1; + counter.setAttribute('data-pending-comment-number', String(num)); + counter.textContent = String(num); + animateOnce(reviewBox, 'pulse-1p5-200'); } - $newConversationHolder.find('.dropdown').dropdown(); } catch (error) { console.error('Error:', error); showErrorToast(i18n.network_error); } finally { - e.target.classList.remove('is-loading'); + form?.classList.remove('is-loading'); } }); @@ -219,7 +220,6 @@ export function initRepoDiffView() { initDiffFileList(); initDiffCommitSelect(); initRepoDiffShowMore(); - initRepoDiffReviewButton(); initRepoDiffFileViewToggle(); initViewedCheckboxListenerFor(); initExpandAndCollapseFilesButton(); diff --git a/web_src/js/features/repo-issue.ts b/web_src/js/features/repo-issue.ts index 7457531ece47d..ca87add32a19b 100644 --- a/web_src/js/features/repo-issue.ts +++ b/web_src/js/features/repo-issue.ts @@ -1,7 +1,7 @@ import $ from 'jquery'; import {htmlEscape} from 'escape-goat'; import {createTippy, showTemporaryTooltip} from '../modules/tippy.ts'; -import {hideElem, showElem, toggleElem} from '../utils/dom.ts'; +import {addElemsEventListener, createElementFromHTML, hideElem, showElem, toggleElem} from '../utils/dom.ts'; import {setFileFolding} from './file-fold.ts'; import {ComboMarkdownEditor, getComboMarkdownEditor, initComboMarkdownEditor} from './comp/ComboMarkdownEditor.ts'; import {parseIssuePageInfo, toAbsoluteUrl} from '../utils.ts'; @@ -443,21 +443,19 @@ export function initRepoPullRequestReview() { }); } - $(document).on('click', '.add-code-comment', async function (e) { - if (e.target.classList.contains('btn-add-single')) return; // https://github.com/go-gitea/gitea/issues/4745 + addElemsEventListener(document, 'click', '.add-code-comment', async (el, e) => { e.preventDefault(); - const isSplit = this.closest('.code-diff')?.classList.contains('code-diff-split'); - const side = this.getAttribute('data-side'); - const idx = this.getAttribute('data-idx'); - const path = this.closest('[data-path]')?.getAttribute('data-path'); - const tr = this.closest('tr'); + const isSplit = el.closest('.code-diff')?.classList.contains('code-diff-split'); + const side = el.getAttribute('data-side'); + const idx = el.getAttribute('data-idx'); + const path = el.closest('[data-path]')?.getAttribute('data-path'); + const tr = el.closest('tr'); const lineType = tr.getAttribute('data-line-type'); - const ntr = tr.nextElementSibling; - let $ntr = $(ntr); + let ntr = tr.nextElementSibling; if (!ntr?.classList.contains('add-comment')) { - $ntr = $(` + ntr = createElementFromHTML(` ${isSplit ? ` @@ -466,24 +464,18 @@ export function initRepoPullRequestReview() { `} `); - $(tr).after($ntr); + tr.after(ntr); } - - const $td = $ntr.find(`.add-comment-${side}`); - const $commentCloud = $td.find('.comment-code-cloud'); - if (!$commentCloud.length && !$ntr.find('button[name="pending_review"]').length) { - try { - const response = await GET(this.closest('[data-new-comment-url]')?.getAttribute('data-new-comment-url')); - const html = await response.text(); - $td.html(html); - $td.find("input[name='line']").val(idx); - $td.find("input[name='side']").val(side === 'left' ? 'previous' : 'proposed'); - $td.find("input[name='path']").val(path); - const editor = await initComboMarkdownEditor($td[0].querySelector('.combo-markdown-editor')); - editor.focus(); - } catch (error) { - console.error(error); - } + const td = ntr.querySelector(`.add-comment-${side}`); + const commentCloud = td.querySelector('.comment-code-cloud'); + if (!commentCloud && !ntr.querySelector('button[name="pending_review"]')) { + const response = await GET(el.closest('[data-new-comment-url]')?.getAttribute('data-new-comment-url')); + td.innerHTML = await response.text(); + td.querySelector("input[name='line']").value = idx; + td.querySelector("input[name='side']").value = (side === 'left' ? 'previous' : 'proposed'); + td.querySelector("input[name='path']").value = path; + const editor = await initComboMarkdownEditor(td.querySelector('.combo-markdown-editor')); + editor.focus(); } }); } diff --git a/web_src/js/utils/dom.test.ts b/web_src/js/utils/dom.test.ts index d7e3a4939e23b..cb99a85511b06 100644 --- a/web_src/js/utils/dom.test.ts +++ b/web_src/js/utils/dom.test.ts @@ -2,6 +2,7 @@ import {createElementFromAttrs, createElementFromHTML, querySingleVisibleElem} f test('createElementFromHTML', () => { expect(createElementFromHTML('foobar').outerHTML).toEqual('foobar'); + expect(createElementFromHTML('foo').outerHTML).toEqual('foo'); }); test('createElementFromAttrs', () => { diff --git a/web_src/js/utils/dom.ts b/web_src/js/utils/dom.ts index 29b34dd1e3f42..51fd50234c525 100644 --- a/web_src/js/utils/dom.ts +++ b/web_src/js/utils/dom.ts @@ -302,8 +302,15 @@ export function replaceTextareaSelection(textarea: HTMLTextAreaElement, text: st // Warning: Do not enter any unsanitized variables here export function createElementFromHTML(htmlString: string): HTMLElement { + htmlString = htmlString.trim(); + // some tags like "tr" are special, it must use a correct parent container to create + if (htmlString.startsWith('(parent: Element, s if (candidates.length > 1) throw new Error(`Expected exactly one visible element matching selector "${selector}", but found ${candidates.length}`); return candidates.length ? candidates[0] as T : null; } + +export function addElemsEventListener(parent: Node, type: string, selector: string, listener: (elem: T, e: Event) => void | Promise, options?: boolean | AddEventListenerOptions) { + parent.addEventListener(type, (e: Event) => { + const elem = (e.target as HTMLElement).closest(selector); + if (!elem) return; + listener(elem as T, e); + }, options); +} From ae9687d75f4286c021d0aa0f3e5b593452008c76 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Thu, 21 Nov 2024 20:43:25 +0800 Subject: [PATCH 2/6] Update web_src/js/features/repo-diff.ts Co-authored-by: silverwind --- web_src/js/features/repo-diff.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/web_src/js/features/repo-diff.ts b/web_src/js/features/repo-diff.ts index 3f0df6433fee9..74bc784c3bbff 100644 --- a/web_src/js/features/repo-diff.ts +++ b/web_src/js/features/repo-diff.ts @@ -13,7 +13,8 @@ import { hideElem, showElem, animateOnce, - addElemsEventListener, createElementFromHTML, + addElemsEventListener, + createElementFromHTML, } from '../utils/dom.ts'; import {POST, GET} from '../modules/fetch.ts'; import {fomanticQuery} from '../modules/fomantic/base.ts'; From b674b975c93ba245150b76cbe7f81ef4d92f0c87 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Thu, 21 Nov 2024 20:45:13 +0800 Subject: [PATCH 3/6] Update web_src/js/features/repo-diff.ts Co-authored-by: silverwind --- web_src/js/features/repo-diff.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web_src/js/features/repo-diff.ts b/web_src/js/features/repo-diff.ts index 74bc784c3bbff..1b8d023c0f367 100644 --- a/web_src/js/features/repo-diff.ts +++ b/web_src/js/features/repo-diff.ts @@ -39,7 +39,7 @@ function initRepoDiffFileViewToggle() { function initRepoDiffConversationForm() { addElemsEventListener(document, 'submit', '.conversation-holder form', async (form, e) => { e.preventDefault(); - const textArea = form.querySelector('textarea'); + const textArea = form.querySelector('textarea'); if (!validateTextareaNonEmpty(textArea)) return; if (form.classList.contains('is-loading')) return; From 3b4b3915d7c7c462b99f2f04d994a158503a49ba Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Thu, 21 Nov 2024 20:47:13 +0800 Subject: [PATCH 4/6] Update web_src/js/utils/dom.ts Co-authored-by: silverwind --- web_src/js/utils/dom.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web_src/js/utils/dom.ts b/web_src/js/utils/dom.ts index 51fd50234c525..674c497596bc2 100644 --- a/web_src/js/utils/dom.ts +++ b/web_src/js/utils/dom.ts @@ -301,7 +301,7 @@ export function replaceTextareaSelection(textarea: HTMLTextAreaElement, text: st } // Warning: Do not enter any unsanitized variables here -export function createElementFromHTML(htmlString: string): HTMLElement { +export function createElementFromHTML(htmlString: string): T { htmlString = htmlString.trim(); // some tags like "tr" are special, it must use a correct parent container to create if (htmlString.startsWith(' Date: Thu, 21 Nov 2024 20:49:46 +0800 Subject: [PATCH 5/6] addDelegatedEventListener --- web_src/js/features/repo-diff.ts | 4 ++-- web_src/js/features/repo-issue.ts | 4 ++-- web_src/js/utils/dom.ts | 6 +++--- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/web_src/js/features/repo-diff.ts b/web_src/js/features/repo-diff.ts index 1b8d023c0f367..0d489665a2307 100644 --- a/web_src/js/features/repo-diff.ts +++ b/web_src/js/features/repo-diff.ts @@ -13,7 +13,7 @@ import { hideElem, showElem, animateOnce, - addElemsEventListener, + addDelegatedEventListener, createElementFromHTML, } from '../utils/dom.ts'; import {POST, GET} from '../modules/fetch.ts'; @@ -37,7 +37,7 @@ function initRepoDiffFileViewToggle() { } function initRepoDiffConversationForm() { - addElemsEventListener(document, 'submit', '.conversation-holder form', async (form, e) => { + addDelegatedEventListener(document, 'submit', '.conversation-holder form', async (form, e) => { e.preventDefault(); const textArea = form.querySelector('textarea'); if (!validateTextareaNonEmpty(textArea)) return; diff --git a/web_src/js/features/repo-issue.ts b/web_src/js/features/repo-issue.ts index ca87add32a19b..9cc478712bcee 100644 --- a/web_src/js/features/repo-issue.ts +++ b/web_src/js/features/repo-issue.ts @@ -1,7 +1,7 @@ import $ from 'jquery'; import {htmlEscape} from 'escape-goat'; import {createTippy, showTemporaryTooltip} from '../modules/tippy.ts'; -import {addElemsEventListener, createElementFromHTML, hideElem, showElem, toggleElem} from '../utils/dom.ts'; +import {addDelegatedEventListener, createElementFromHTML, hideElem, showElem, toggleElem} from '../utils/dom.ts'; import {setFileFolding} from './file-fold.ts'; import {ComboMarkdownEditor, getComboMarkdownEditor, initComboMarkdownEditor} from './comp/ComboMarkdownEditor.ts'; import {parseIssuePageInfo, toAbsoluteUrl} from '../utils.ts'; @@ -443,7 +443,7 @@ export function initRepoPullRequestReview() { }); } - addElemsEventListener(document, 'click', '.add-code-comment', async (el, e) => { + addDelegatedEventListener(document, 'click', '.add-code-comment', async (el, e) => { e.preventDefault(); const isSplit = el.closest('.code-diff')?.classList.contains('code-diff-split'); diff --git a/web_src/js/utils/dom.ts b/web_src/js/utils/dom.ts index 674c497596bc2..8bb9b8defcb30 100644 --- a/web_src/js/utils/dom.ts +++ b/web_src/js/utils/dom.ts @@ -307,11 +307,11 @@ export function createElementFromHTML(htmlString: string): T { if (htmlString.startsWith(', ...children: (Node|string)[]): HTMLElement { @@ -348,7 +348,7 @@ export function querySingleVisibleElem(parent: Element, s return candidates.length ? candidates[0] as T : null; } -export function addElemsEventListener(parent: Node, type: string, selector: string, listener: (elem: T, e: Event) => void | Promise, options?: boolean | AddEventListenerOptions) { +export function addDelegatedEventListener(parent: Node, type: string, selector: string, listener: (elem: T, e: Event) => void | Promise, options?: boolean | AddEventListenerOptions) { parent.addEventListener(type, (e: Event) => { const elem = (e.target as HTMLElement).closest(selector); if (!elem) return; From 4eaf28d5ccac8e1083e632c7f736f4910af01d4d Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Thu, 21 Nov 2024 20:51:06 +0800 Subject: [PATCH 6/6] createElementFromHTML --- web_src/js/utils/dom.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/web_src/js/utils/dom.ts b/web_src/js/utils/dom.ts index 8bb9b8defcb30..4bbb0c414aca0 100644 --- a/web_src/js/utils/dom.ts +++ b/web_src/js/utils/dom.ts @@ -301,13 +301,13 @@ export function replaceTextareaSelection(textarea: HTMLTextAreaElement, text: st } // Warning: Do not enter any unsanitized variables here -export function createElementFromHTML(htmlString: string): T { +export function createElementFromHTML(htmlString: string): T { htmlString = htmlString.trim(); // some tags like "tr" are special, it must use a correct parent container to create if (htmlString.startsWith('('tr'); } const div = document.createElement('div'); div.innerHTML = htmlString;