Skip to content

Commit 698ae7a

Browse files
wxiaoguangwxiaoguang
authored
Fix dynamic content loading init problem (#33748)
1. Rewrite `dirauto.ts` to `observer.ts`. * We have been using MutationObserver for long time, it's proven that it is quite performant. * Now we extend its ability to handle more "init" works. 2. Use `observeAddedElement` to init all non-custom "dropdown". 3. Use `data-global-click` to handle click events from dynamically loaded elements. * By this new approach, the old fragile selector-based (`.comment-reaction-button`) mechanism is removed. 4. By the way, remove unused `.diff-box` selector, it was abused and never really used. A lot of FIXMEs in "repo-diff.ts" are completely fixed, newly loaded contents could work as expected.
1 parent f3ada61 commit 698ae7a

File tree

13 files changed

+171
-135
lines changed

13 files changed

+171
-135
lines changed

‎templates/repo/diff/box.tmpl‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{{$showFileTree := (and (not .DiffNotAvailable) (gt .Diff.NumFiles 1))}}
22
<div>
3-
<div class="diff-detail-box diff-box">
3+
<div class="diff-detail-box">
44
<div class="tw-flex tw-items-center tw-flex-wrap tw-gap-2 tw-ml-0.5">
55
{{if $showFileTree}}
66
<button class="diff-toggle-file-tree-button not-mobile btn interact-fg" data-show-text="{{ctx.Locale.Tr "repo.diff.show_file_tree"}}" data-hide-text="{{ctx.Locale.Tr "repo.diff.hide_file_tree"}}">
@@ -80,7 +80,7 @@
8080
{{$showFileViewToggle := or $isImage (and (not $file.IsIncomplete) $isCsv)}}
8181
{{$isExpandable := or (gt $file.Addition 0) (gt $file.Deletion 0) $file.IsBin}}
8282
{{$isReviewFile := and $.IsSigned $.PageIsPullFiles (not $.Repository.IsArchived) $.IsShowingAllCommits}}
83-
<div class="diff-file-box diff-box file-content {{TabSizeClass $.Editorconfig $file.Name}} tw-mt-0" id="diff-{{$file.NameHash}}" data-old-filename="{{$file.OldName}}" data-new-filename="{{$file.Name}}" {{if or ($file.ShouldBeHidden) (not $isExpandable)}}data-folded="true"{{end}}>
83+
<div class="diff-file-box file-content {{TabSizeClass $.Editorconfig $file.Name}} tw-mt-0" id="diff-{{$file.NameHash}}" data-old-filename="{{$file.OldName}}" data-new-filename="{{$file.Name}}" {{if or ($file.ShouldBeHidden) (not $isExpandable)}}data-folded="true"{{end}}>
8484
<h4 class="diff-file-header sticky-2nd-row ui top attached header">
8585
<div class="diff-file-name tw-flex tw-flex-1 tw-items-center tw-gap-1 tw-flex-wrap">
8686
<button class="fold-file btn interact-bg tw-p-1{{if not $isExpandable}} tw-invisible{{end}}">
@@ -209,7 +209,7 @@
209209
{{end}}
210210

211211
{{if .Diff.IsIncomplete}}
212-
<div class="diff-file-box diff-box file-content tw-mt-2" id="diff-incomplete">
212+
<div class="diff-file-box file-content tw-mt-2" id="diff-incomplete">
213213
<h4 class="ui top attached header tw-font-normal tw-flex tw-items-center tw-justify-between">
214214
{{ctx.Locale.Tr "repo.diff.too_many_files"}}
215215
<a class="ui basic tiny button" id="diff-show-more-files" data-href="?skip-to={{.Diff.End}}&file-only=true">{{ctx.Locale.Tr "repo.diff.show_more"}}</a>

‎templates/repo/issue/view_content/add_reaction.tmpl‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
<a class="muted">{{svg "octicon-smiley"}}</a>
44
<div class="menu">
55
{{range $value := AllowedReactions}}
6-
<a class="item emoji comment-reaction-button" data-tooltip-content="{{$value}}" aria-label="{{$value}}" data-reaction-content="{{$value}}">{{ReactionToEmoji $value}}</a>
6+
<a class="item emoji" data-tooltip-content="{{$value}}" aria-label="{{$value}}" data-reaction-content="{{$value}}" data-global-click="onCommentReactionButtonClick">{{ReactionToEmoji $value}}</a>
77
{{end}}
88
</div>
99
</div>

‎templates/repo/issue/view_content/conversation.tmpl‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@
4040
{{if $diff}}
4141
{{$file := (index $diff.Files 0)}}
4242
<div id="code-preview-{{$comment.ID}}" class="ui table segment{{if $resolved}} tw-hidden{{end}}">
43-
<div class="diff-file-box diff-box file-content {{TabSizeClass $.Editorconfig $file.Name}}">
43+
<div class="diff-file-box file-content {{TabSizeClass $.Editorconfig $file.Name}}">
4444
<div class="file-body file-code code-view code-diff code-diff-unified unicode-escaped">
4545
<table>
4646
<tbody>

‎templates/repo/issue/view_content/reactions.tmpl‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
<div class="bottom-reactions" data-action-url="{{$.ActionURL}}">
22
{{range $key, $value := .Reactions}}
33
{{$hasReacted := $value.HasUser ctx.RootData.SignedUserID}}
4-
<a role="button" class="ui label basic{{if $hasReacted}} primary{{end}}{{if not ctx.RootData.IsSigned}} disabled{{end}} comment-reaction-button"
5-
data-tooltip-content
6-
title="{{$value.GetFirstUsers}}{{if gt ($value.GetMoreUserCount) 0}} {{ctx.Locale.Tr "repo.reactions_more" $value.GetMoreUserCount}}{{end}}"
4+
<a role="button" class="ui label basic{{if $hasReacted}} primary{{end}}{{if not ctx.RootData.IsSigned}} disabled{{end}}"
5+
data-global-click="onCommentReactionButtonClick"
6+
data-tooltip-content title="{{$value.GetFirstUsers}}{{if gt ($value.GetMoreUserCount) 0}} {{ctx.Locale.Tr "repo.reactions_more" $value.GetMoreUserCount}}{{end}}"
77
aria-label="{{$value.GetFirstUsers}}{{if gt ($value.GetMoreUserCount) 0}} {{ctx.Locale.Tr "repo.reactions_more" $value.GetMoreUserCount}}{{end}}"
88
data-tooltip-placement="bottom-start"
99
data-reaction-content="{{$key}}" data-has-reacted="{{$hasReacted}}">

‎templates/shared/search/code/results.tmpl‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
<div class="repository search">
1212
{{range $result := .SearchResults}}
1313
{{$repo := or $.Repo (index $.RepoMaps .RepoID)}}
14-
<div class="diff-file-box diff-box file-content non-diff-file-content repo-search-result">
14+
<div class="diff-file-box file-content non-diff-file-content repo-search-result">
1515
<h4 class="ui top attached header tw-font-normal tw-flex tw-flex-wrap">
1616
{{if not $.Repo}}
1717
<span class="file tw-flex-1">

‎web_src/css/repo.css‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1085,10 +1085,6 @@ td .commit-summary {
10851085
height: 30px;
10861086
}
10871087

1088-
.repository .diff-box .resolved-placeholder .button {
1089-
padding: 8px 12px;
1090-
}
1091-
10921088
.repository .diff-file-box .header {
10931089
background-color: var(--color-box-header);
10941090
}

‎web_src/js/features/common-page.ts‎

Lines changed: 42 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import {GET} from '../modules/fetch.ts';
22
import {showGlobalErrorMessage} from '../bootstrap.ts';
33
import {fomanticQuery} from '../modules/fomantic/base.ts';
44
import {queryElems} from '../utils/dom.ts';
5+
import {observeAddedElement} from '../modules/observer.ts';
56

67
const {appUrl} = window.config;
78

@@ -28,47 +29,51 @@ export function initFootLanguageMenu() {
2829
}
2930

3031
export function initGlobalDropdown() {
31-
// Semantic UI modules.
32-
const $uiDropdowns = fomanticQuery('.ui.dropdown');
33-
3432
// do not init "custom" dropdowns, "custom" dropdowns are managed by their own code.
35-
$uiDropdowns.filter(':not(.custom)').dropdown({hideDividers: 'empty'});
33+
observeAddedElement('.ui.dropdown:not(.custom)', (el) => {
34+
const $dropdown = fomanticQuery(el);
35+
if ($dropdown.data('module-dropdown')) return; // do not re-init if other code has already initialized it.
3636

37-
// The "jump" means this dropdown is mainly used for "menu" purpose,
38-
// clicking an item will jump to somewhere else or trigger an action/function.
39-
// When a dropdown is used for non-refresh actions with tippy,
40-
// it must have this "jump" class to hide the tippy when dropdown is closed.
41-
$uiDropdowns.filter('.jump').dropdown('setting', {
42-
action: 'hide',
43-
onShow() {
44-
// hide associated tooltip while dropdown is open
45-
this._tippy?.hide();
46-
this._tippy?.disable();
47-
},
48-
onHide() {
49-
this._tippy?.enable();
50-
// eslint-disable-next-line unicorn/no-this-assignment
51-
const elDropdown = this;
37+
$dropdown.dropdown('setting', {hideDividers: 'empty'});
5238

53-
// hide all tippy elements of items after a while. eg: use Enter to click "Copy Link" in the Issue Context Menu
54-
setTimeout(() => {
55-
const $dropdown = fomanticQuery(elDropdown);
56-
if ($dropdown.dropdown('is hidden')) {
57-
queryElems(elDropdown, '.menu > .item', (el) => el._tippy?.hide());
58-
}
59-
}, 2000);
60-
},
61-
});
39+
if (el.classList.contains('jump')) {
40+
// The "jump" means this dropdown is mainly used for "menu" purpose,
41+
// clicking an item will jump to somewhere else or trigger an action/function.
42+
// When a dropdown is used for non-refresh actions with tippy,
43+
// it must have this "jump" class to hide the tippy when dropdown is closed.
44+
$dropdown.dropdown('setting', {
45+
action: 'hide',
46+
onShow() {
47+
// hide associated tooltip while dropdown is open
48+
this._tippy?.hide();
49+
this._tippy?.disable();
50+
},
51+
onHide() {
52+
this._tippy?.enable();
53+
// eslint-disable-next-line unicorn/no-this-assignment
54+
const elDropdown = this;
6255

63-
// Special popup-directions, prevent Fomantic from guessing the popup direction.
64-
// With default "direction: auto", if the viewport height is small, Fomantic would show the popup upward,
65-
// if the dropdown is at the beginning of the page, then the top part would be clipped by the window view.
66-
// eg: Issue List "Sort" dropdown
67-
// But we can not set "direction: downward" for all dropdowns, because there is a bug in dropdown menu positioning when calculating the "left" position,
68-
// which would make some dropdown popups slightly shift out of the right viewport edge in some cases.
69-
// eg: the "Create New Repo" menu on the navbar.
70-
$uiDropdowns.filter('.upward').dropdown('setting', 'direction', 'upward');
71-
$uiDropdowns.filter('.downward').dropdown('setting', 'direction', 'downward');
56+
// hide all tippy elements of items after a while. eg: use Enter to click "Copy Link" in the Issue Context Menu
57+
setTimeout(() => {
58+
const $dropdown = fomanticQuery(elDropdown);
59+
if ($dropdown.dropdown('is hidden')) {
60+
queryElems(elDropdown, '.menu > .item', (el) => el._tippy?.hide());
61+
}
62+
}, 2000);
63+
},
64+
});
65+
}
66+
67+
// Special popup-directions, prevent Fomantic from guessing the popup direction.
68+
// With default "direction: auto", if the viewport height is small, Fomantic would show the popup upward,
69+
// if the dropdown is at the beginning of the page, then the top part would be clipped by the window view.
70+
// eg: Issue List "Sort" dropdown
71+
// But we can not set "direction: downward" for all dropdowns, because there is a bug in dropdown menu positioning when calculating the "left" position,
72+
// which would make some dropdown popups slightly shift out of the right viewport edge in some cases.
73+
// eg: the "Create New Repo" menu on the navbar.
74+
if (el.classList.contains('upward')) $dropdown.dropdown('setting', 'direction', 'upward');
75+
if (el.classList.contains('downward')) $dropdown.dropdown('setting', 'direction', 'downward');
76+
});
7277
}
7378

7479
export function initGlobalTabularMenu() {

‎web_src/js/features/comp/ReactionSelector.ts‎

Lines changed: 21 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,37 +1,31 @@
11
import {POST} from '../../modules/fetch.ts';
2-
import {fomanticQuery} from '../../modules/fomantic/base.ts';
32
import type {DOMEvent} from '../../utils/dom.ts';
3+
import {registerGlobalEventFunc} from '../../modules/observer.ts';
44

5-
export function initCompReactionSelector(parent: ParentNode = document) {
6-
for (const container of parent.querySelectorAll<HTMLElement>('.issue-content, .diff-file-body')) {
7-
container.addEventListener('click', async (e: DOMEvent<MouseEvent>) => {
8-
// there are 2 places for the "reaction" buttons, one is the top-right reaction menu, one is the bottom of the comment
9-
const target = e.target.closest('.comment-reaction-button');
10-
if (!target) return;
11-
e.preventDefault();
5+
export function initCompReactionSelector() {
6+
registerGlobalEventFunc('click', 'onCommentReactionButtonClick', async (target: HTMLElement, e: DOMEvent<MouseEvent>) => {
7+
// there are 2 places for the "reaction" buttons, one is the top-right reaction menu, one is the bottom of the comment
8+
e.preventDefault();
129

13-
if (target.classList.contains('disabled')) return;
10+
if (target.classList.contains('disabled')) return;
1411

15-
const actionUrl = target.closest('[data-action-url]').getAttribute('data-action-url');
16-
const reactionContent = target.getAttribute('data-reaction-content');
12+
const actionUrl = target.closest('[data-action-url]').getAttribute('data-action-url');
13+
const reactionContent = target.getAttribute('data-reaction-content');
1714

18-
const commentContainer = target.closest('.comment-container');
15+
const commentContainer = target.closest('.comment-container');
1916

20-
const bottomReactions = commentContainer.querySelector('.bottom-reactions'); // may not exist if there is no reaction
21-
const bottomReactionBtn = bottomReactions?.querySelector(`a[data-reaction-content="${CSS.escape(reactionContent)}"]`);
22-
const hasReacted = bottomReactionBtn?.getAttribute('data-has-reacted') === 'true';
17+
const bottomReactions = commentContainer.querySelector('.bottom-reactions'); // may not exist if there is no reaction
18+
const bottomReactionBtn = bottomReactions?.querySelector(`a[data-reaction-content="${CSS.escape(reactionContent)}"]`);
19+
const hasReacted = bottomReactionBtn?.getAttribute('data-has-reacted') === 'true';
2320

24-
const res = await POST(`${actionUrl}/${hasReacted ? 'unreact' : 'react'}`, {
25-
data: new URLSearchParams({content: reactionContent}),
26-
});
27-
28-
const data = await res.json();
29-
bottomReactions?.remove();
30-
if (data.html) {
31-
commentContainer.insertAdjacentHTML('beforeend', data.html);
32-
const bottomReactionsDropdowns = commentContainer.querySelectorAll('.bottom-reactions .dropdown.select-reaction');
33-
fomanticQuery(bottomReactionsDropdowns).dropdown(); // re-init the dropdown
34-
}
21+
const res = await POST(`${actionUrl}/${hasReacted ? 'unreact' : 'react'}`, {
22+
data: new URLSearchParams({content: reactionContent}),
3523
});
36-
}
24+
25+
const data = await res.json();
26+
bottomReactions?.remove();
27+
if (data.html) {
28+
commentContainer.insertAdjacentHTML('beforeend', data.html);
29+
}
30+
});
3731
}

‎web_src/js/features/repo-diff.ts‎

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
import {initCompReactionSelector} from './comp/ReactionSelector.ts';
21
import {initRepoIssueContentHistory} from './repo-issue-content.ts';
32
import {initDiffFileTree} from './repo-diff-filetree.ts';
43
import {initDiffCommitSelect} from './repo-diff-commitselect.ts';
@@ -8,17 +7,16 @@ import {initImageDiff} from './imagediff.ts';
87
import {showErrorToast} from '../modules/toast.ts';
98
import {submitEventSubmitter, queryElemSiblings, hideElem, showElem, animateOnce, addDelegatedEventListener, createElementFromHTML, queryElems} from '../utils/dom.ts';
109
import {POST, GET} from '../modules/fetch.ts';
11-
import {fomanticQuery} from '../modules/fomantic/base.ts';
1210
import {createTippy} from '../modules/tippy.ts';
1311
import {invertFileFolding} from './file-fold.ts';
1412
import {parseDom} from '../utils.ts';
13+
import {observeAddedElement} from '../modules/observer.ts';
1514

1615
const {i18n} = window.config;
1716

18-
function initRepoDiffFileViewToggle() {
17+
function initRepoDiffFileBox(el: HTMLElement) {
1918
// switch between "rendered" and "source", for image and CSV files
20-
// FIXME: this event listener is not correctly added to "load more files"
21-
queryElems(document, '.file-view-toggle', (btn) => btn.addEventListener('click', () => {
19+
queryElems(el, '.file-view-toggle', (btn) => btn.addEventListener('click', () => {
2220
queryElemSiblings(btn, '.file-view-toggle', (el) => el.classList.remove('active'));
2321
btn.classList.add('active');
2422

@@ -75,7 +73,6 @@ function initRepoDiffConversationForm() {
7573
el.classList.add('tw-invisible');
7674
}
7775
}
78-
fomanticQuery(newConversationHolder.querySelectorAll('.ui.dropdown')).dropdown();
7976

8077
// the default behavior is to add a pending review, so if no submitter, it also means "pending_review"
8178
if (!submitter || submitter?.matches('button[name="pending_review"]')) {
@@ -110,8 +107,6 @@ function initRepoDiffConversationForm() {
110107
if (elConversationHolder) {
111108
const elNewConversation = createElementFromHTML(data);
112109
elConversationHolder.replaceWith(elNewConversation);
113-
queryElems(elConversationHolder, '.ui.dropdown:not(.custom)', (el) => fomanticQuery(el).dropdown());
114-
initCompReactionSelector(elNewConversation);
115110
} else {
116111
window.location.reload();
117112
}
@@ -149,7 +144,7 @@ function initDiffHeaderPopup() {
149144

150145
// Will be called when the show more (files) button has been pressed
151146
function onShowMoreFiles() {
152-
// FIXME: here the init calls are incomplete: at least it misses dropdown & initCompReactionSelector & initRepoDiffFileViewToggle
147+
// TODO: replace these calls with the "observer.ts" methods
153148
initRepoIssueContentHistory();
154149
initViewedCheckboxListenerFor();
155150
countAndUpdateViewedFiles();
@@ -255,11 +250,11 @@ export function initRepoDiffView() {
255250
initDiffCommitSelect();
256251
initRepoDiffShowMore();
257252
initDiffHeaderPopup();
258-
initRepoDiffFileViewToggle();
259253
initViewedCheckboxListenerFor();
260254
initExpandAndCollapseFilesButton();
261255
initRepoDiffHashChangeListener();
262256

257+
observeAddedElement('#diff-file-boxes .diff-file-box', initRepoDiffFileBox);
263258
addDelegatedEventListener(document, 'click', '.fold-file', (el) => {
264259
invertFileFolding(el.closest('.file-content'), el);
265260
});

‎web_src/js/features/repo-graph.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,8 +83,8 @@ export function initRepoGraphGit() {
8383
}
8484

8585
const flowSelectRefsDropdown = document.querySelector('#flow-select-refs-dropdown');
86-
fomanticQuery(flowSelectRefsDropdown).dropdown('set selected', dropdownSelected);
87-
fomanticQuery(flowSelectRefsDropdown).dropdown({
86+
const $dropdown = fomanticQuery(flowSelectRefsDropdown);
87+
$dropdown.dropdown({
8888
clearable: true,
8989
fullTextSeach: 'exact',
9090
onRemove(toRemove: string) {
@@ -110,6 +110,7 @@ export function initRepoGraphGit() {
110110
updateGraph();
111111
},
112112
});
113+
$dropdown.dropdown('set selected', dropdownSelected);
113114

114115
graphContainer.addEventListener('mouseenter', (e: DOMEvent<MouseEvent>) => {
115116
if (e.target.matches('#rev-list li')) {

0 commit comments

Comments
 (0)