Skip to content

Commit 1bb1a51

Browse files
authored
Fix JS error when reply comment on Conversation page (#32685)
Fix #32684, regression of #32596 (side-effect of jQuery removal: jQuery could tolerate non-existing elements) And fix another regression bug from #30453 (initCompReactionSelector double-init)
1 parent 2abef97 commit 1bb1a51

File tree

2 files changed

+19
-13
lines changed

2 files changed

+19
-13
lines changed

web_src/js/features/comp/ReactionSelector.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
import {POST} from '../../modules/fetch.ts';
22
import {fomanticQuery} from '../../modules/fomantic/base.ts';
33

4-
export function initCompReactionSelector() {
5-
for (const container of document.querySelectorAll('.issue-content, .diff-file-body')) {
4+
export function initCompReactionSelector(parent: ParentNode = document) {
5+
for (const container of parent.querySelectorAll('.issue-content, .diff-file-body')) {
66
container.addEventListener('click', async (e) => {
77
// there are 2 places for the "reaction" buttons, one is the top-right reaction menu, one is the bottom of the comment
88
const target = e.target.closest('.comment-reaction-button');

web_src/js/features/repo-diff.ts

+17-11
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ function initRepoDiffFileViewToggle() {
3838
}
3939

4040
function initRepoDiffConversationForm() {
41-
addDelegatedEventListener<HTMLFormElement>(document, 'submit', '.conversation-holder form', async (form, e) => {
41+
addDelegatedEventListener<HTMLFormElement, SubmitEvent>(document, 'submit', '.conversation-holder form', async (form, e) => {
4242
e.preventDefault();
4343
const textArea = form.querySelector<HTMLTextAreaElement>('textarea');
4444
if (!validateTextareaNonEmpty(textArea)) return;
@@ -55,7 +55,9 @@ function initRepoDiffConversationForm() {
5555
formData.append(submitter.name, submitter.value);
5656
}
5757

58-
const trLineType = form.closest('tr').getAttribute('data-line-type');
58+
// on the diff page, the form is inside a "tr" and need to get the line-type ahead
59+
// but on the conversation page, there is no parent "tr"
60+
const trLineType = form.closest('tr')?.getAttribute('data-line-type');
5961
const response = await POST(form.getAttribute('action'), {data: formData});
6062
const newConversationHolder = createElementFromHTML(await response.text());
6163
const path = newConversationHolder.getAttribute('data-path');
@@ -65,14 +67,18 @@ function initRepoDiffConversationForm() {
6567
form.closest('.conversation-holder').replaceWith(newConversationHolder);
6668
form = null; // prevent further usage of the form because it should have been replaced
6769

68-
let selector;
69-
if (trLineType === 'same') {
70-
selector = `[data-path="${path}"] .add-code-comment[data-idx="${idx}"]`;
71-
} else {
72-
selector = `[data-path="${path}"] .add-code-comment[data-side="${side}"][data-idx="${idx}"]`;
73-
}
74-
for (const el of document.querySelectorAll(selector)) {
75-
el.classList.add('tw-invisible'); // TODO need to figure out why
70+
if (trLineType) {
71+
// if there is a line-type for the "tr", it means the form is on the diff page
72+
// then hide the "add-code-comment" [+] button for current code line by adding "tw-invisible" because the conversation has been added
73+
let selector;
74+
if (trLineType === 'same') {
75+
selector = `[data-path="${path}"] .add-code-comment[data-idx="${idx}"]`;
76+
} else {
77+
selector = `[data-path="${path}"] .add-code-comment[data-side="${side}"][data-idx="${idx}"]`;
78+
}
79+
for (const el of document.querySelectorAll(selector)) {
80+
el.classList.add('tw-invisible');
81+
}
7682
}
7783
fomanticQuery(newConversationHolder.querySelectorAll('.ui.dropdown')).dropdown();
7884

@@ -109,7 +115,7 @@ function initRepoDiffConversationForm() {
109115
const $conversation = $(data);
110116
$(this).closest('.conversation-holder').replaceWith($conversation);
111117
$conversation.find('.dropdown').dropdown();
112-
initCompReactionSelector($conversation);
118+
initCompReactionSelector($conversation[0]);
113119
} else {
114120
window.location.reload();
115121
}

0 commit comments

Comments
 (0)