Skip to content

Commit c30f4f4

Browse files
authored
Fix issue suggestion bug (#33389) (#33391)
Backport #33389
1 parent 4578288 commit c30f4f4

File tree

1 file changed

+53
-9
lines changed

1 file changed

+53
-9
lines changed

web_src/js/features/comp/TextExpander.ts

Lines changed: 53 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,19 +5,33 @@ import {parseIssueHref, parseRepoOwnerPathInfo} from '../../utils.ts';
55
import {createElementFromAttrs, createElementFromHTML} from '../../utils/dom.ts';
66
import {getIssueColor, getIssueIcon} from '../issue.ts';
77
import {debounce} from 'perfect-debounce';
8+
import type TextExpanderElement from '@github/text-expander-element';
89

9-
const debouncedSuggestIssues = debounce((key: string, text: string) => new Promise<{matched:boolean; fragment?: HTMLElement}>(async (resolve) => {
10+
type TextExpanderProvideResult = {
11+
matched: boolean,
12+
fragment?: HTMLElement,
13+
}
14+
15+
type TextExpanderChangeEvent = Event & {
16+
detail?: {
17+
key: string,
18+
text: string,
19+
provide: (result: TextExpanderProvideResult | Promise<TextExpanderProvideResult>) => void,
20+
}
21+
}
22+
23+
async function fetchIssueSuggestions(key: string, text: string): Promise<TextExpanderProvideResult> {
1024
const issuePathInfo = parseIssueHref(window.location.href);
1125
if (!issuePathInfo.ownerName) {
1226
const repoOwnerPathInfo = parseRepoOwnerPathInfo(window.location.pathname);
1327
issuePathInfo.ownerName = repoOwnerPathInfo.ownerName;
1428
issuePathInfo.repoName = repoOwnerPathInfo.repoName;
1529
// then no issuePathInfo.indexString here, it is only used to exclude the current issue when "matchIssue"
1630
}
17-
if (!issuePathInfo.ownerName) return resolve({matched: false});
31+
if (!issuePathInfo.ownerName) return {matched: false};
1832

1933
const matches = await matchIssue(issuePathInfo.ownerName, issuePathInfo.repoName, issuePathInfo.indexString, text);
20-
if (!matches.length) return resolve({matched: false});
34+
if (!matches.length) return {matched: false};
2135

2236
const ul = createElementFromAttrs('ul', {class: 'suggestions'});
2337
for (const issue of matches) {
@@ -29,11 +43,40 @@ const debouncedSuggestIssues = debounce((key: string, text: string) => new Promi
2943
);
3044
ul.append(li);
3145
}
32-
resolve({matched: true, fragment: ul});
33-
}), 100);
46+
return {matched: true, fragment: ul};
47+
}
3448

35-
export function initTextExpander(expander) {
36-
expander?.addEventListener('text-expander-change', ({detail: {key, provide, text}}) => {
49+
export function initTextExpander(expander: TextExpanderElement) {
50+
if (!expander) return;
51+
52+
const textarea = expander.querySelector<HTMLTextAreaElement>('textarea');
53+
54+
// help to fix the text-expander "multiword+promise" bug: do not show the popup when there is no "#" before current line
55+
const shouldShowIssueSuggestions = () => {
56+
const posVal = textarea.value.substring(0, textarea.selectionStart);
57+
const lineStart = posVal.lastIndexOf('\n');
58+
const keyStart = posVal.lastIndexOf('#');
59+
return keyStart > lineStart;
60+
};
61+
62+
const debouncedIssueSuggestions = debounce(async (key: string, text: string): Promise<TextExpanderProvideResult> => {
63+
// https://github.com/github/text-expander-element/issues/71
64+
// Upstream bug: when using "multiword+promise", TextExpander will get wrong "key" position.
65+
// To reproduce, comment out the "shouldShowIssueSuggestions" check, use the "await sleep" below,
66+
// then use content "close #20\nclose #20\nclose #20" (3 lines), keep changing the last line `#20` part from the end (including removing the `#`)
67+
// There will be a JS error: Uncaught (in promise) IndexSizeError: Failed to execute 'setStart' on 'Range': The offset 28 is larger than the node's length (27).
68+
69+
// check the input before the request, to avoid emitting empty query to backend (still related to the upstream bug)
70+
if (!shouldShowIssueSuggestions()) return {matched: false};
71+
// await sleep(Math.random() * 1000); // help to reproduce the text-expander bug
72+
const ret = await fetchIssueSuggestions(key, text);
73+
// check the input again to avoid text-expander using incorrect position (upstream bug)
74+
if (!shouldShowIssueSuggestions()) return {matched: false};
75+
return ret;
76+
}, 300); // to match onInputDebounce delay
77+
78+
expander.addEventListener('text-expander-change', (e: TextExpanderChangeEvent) => {
79+
const {key, text, provide} = e.detail;
3780
if (key === ':') {
3881
const matches = matchEmoji(text);
3982
if (!matches.length) return provide({matched: false});
@@ -81,10 +124,11 @@ export function initTextExpander(expander) {
81124

82125
provide({matched: true, fragment: ul});
83126
} else if (key === '#') {
84-
provide(debouncedSuggestIssues(key, text));
127+
provide(debouncedIssueSuggestions(key, text));
85128
}
86129
});
87-
expander?.addEventListener('text-expander-value', ({detail}) => {
130+
131+
expander.addEventListener('text-expander-value', ({detail}: Record<string, any>) => {
88132
if (detail?.item) {
89133
// add a space after @mentions and #issue as it's likely the user wants one
90134
const suffix = ['@', '#'].includes(detail.key) ? ' ' : '';

0 commit comments

Comments
 (0)