From 23ac435633acb19fd05a3edc925863269aa63aaa Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sat, 25 Jan 2025 18:41:16 +0800 Subject: [PATCH 1/7] fix --- web_src/js/features/comp/TextExpander.ts | 54 ++++++++++++++++++++---- 1 file changed, 46 insertions(+), 8 deletions(-) diff --git a/web_src/js/features/comp/TextExpander.ts b/web_src/js/features/comp/TextExpander.ts index dc08d1739d362..d9395f3be1e1e 100644 --- a/web_src/js/features/comp/TextExpander.ts +++ b/web_src/js/features/comp/TextExpander.ts @@ -7,7 +7,20 @@ import {getIssueColor, getIssueIcon} from '../issue.ts'; import {debounce} from 'perfect-debounce'; import type TextExpanderElement from '@github/text-expander-element'; -const debouncedSuggestIssues = debounce((key: string, text: string) => new Promise<{matched:boolean; fragment?: HTMLElement}>(async (resolve) => { +type TextExpanderProvideResult = { + matched: boolean, + fragment?: HTMLElement, +} + +type TextExpanderChangeEvent = Event & { + detail?: { + key: string, + text: string, + provide: (result: TextExpanderProvideResult | Promise) => void, + } +} + +async function fetchIssueSuggestions(key: string, text: string): Promise { const issuePathInfo = parseIssueHref(window.location.href); if (!issuePathInfo.ownerName) { const repoOwnerPathInfo = parseRepoOwnerPathInfo(window.location.pathname); @@ -15,10 +28,10 @@ const debouncedSuggestIssues = debounce((key: string, text: string) => new Promi issuePathInfo.repoName = repoOwnerPathInfo.repoName; // then no issuePathInfo.indexString here, it is only used to exclude the current issue when "matchIssue" } - if (!issuePathInfo.ownerName) return resolve({matched: false}); + if (!issuePathInfo.ownerName) return {matched: false}; const matches = await matchIssue(issuePathInfo.ownerName, issuePathInfo.repoName, issuePathInfo.indexString, text); - if (!matches.length) return resolve({matched: false}); + if (!matches.length) return {matched: false}; const ul = createElementFromAttrs('ul', {class: 'suggestions'}); for (const issue of matches) { @@ -30,11 +43,35 @@ const debouncedSuggestIssues = debounce((key: string, text: string) => new Promi ); ul.append(li); } - resolve({matched: true, fragment: ul}); -}), 100); + return {matched: true, fragment: ul}; +} export function initTextExpander(expander: TextExpanderElement) { - expander?.addEventListener('text-expander-change', ({detail: {key, provide, text}}: Record) => { + if (!expander) return; + + const textarea = expander.querySelector('textarea'); + + const shouldShowIssueSuggestions = () => { + const posVal = textarea.value.substring(0, textarea.selectionStart); + const lineStart = posVal.lastIndexOf('\n'); + const keyStart = posVal.lastIndexOf('#'); + return keyStart > lineStart; + }; + + const debouncedIssueSuggestions = debounce(async (key: string, text: string): Promise => { + // Upstream bug: when using "multiword", TextExpander will get wrong "key" position. + // To reproduce, use the "await sleep" below, + // then use content "close #20\nclose #20\close #20", keep changing the last line `#20` part from the end (including removing the `#`) + // 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). + if (!shouldShowIssueSuggestions()) return {matched: false}; + // await sleep(Math.random() * 1000); // help to reproduce the text-expander bug + const ret = await fetchIssueSuggestions(key, text); + if (!shouldShowIssueSuggestions()) return {matched: false}; + return ret; + }, 300); // to match onInputDebounce delay + + expander.addEventListener('text-expander-change', (e: TextExpanderChangeEvent) => { + const {key, text, provide} = e.detail; if (key === ':') { const matches = matchEmoji(text); if (!matches.length) return provide({matched: false}); @@ -82,10 +119,11 @@ export function initTextExpander(expander: TextExpanderElement) { provide({matched: true, fragment: ul}); } else if (key === '#') { - provide(debouncedSuggestIssues(key, text)); + provide(debouncedIssueSuggestions(key, text)); } }); - expander?.addEventListener('text-expander-value', ({detail}: Record) => { + + expander.addEventListener('text-expander-value', ({detail}: Record) => { if (detail?.item) { // add a space after @mentions and #issue as it's likely the user wants one const suffix = ['@', '#'].includes(detail.key) ? ' ' : ''; From 0dc10e80483f70edbbe22386b729df7f5b7f19c2 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sat, 25 Jan 2025 22:10:13 +0800 Subject: [PATCH 2/7] improve comment --- web_src/js/features/comp/TextExpander.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/web_src/js/features/comp/TextExpander.ts b/web_src/js/features/comp/TextExpander.ts index d9395f3be1e1e..36925c2e386ff 100644 --- a/web_src/js/features/comp/TextExpander.ts +++ b/web_src/js/features/comp/TextExpander.ts @@ -51,6 +51,7 @@ export function initTextExpander(expander: TextExpanderElement) { const textarea = expander.querySelector('textarea'); + // help to fix the text-expander "multiword+promise" bug: do not show the popup when there is no "#" before current line const shouldShowIssueSuggestions = () => { const posVal = textarea.value.substring(0, textarea.selectionStart); const lineStart = posVal.lastIndexOf('\n'); @@ -59,8 +60,8 @@ export function initTextExpander(expander: TextExpanderElement) { }; const debouncedIssueSuggestions = debounce(async (key: string, text: string): Promise => { - // Upstream bug: when using "multiword", TextExpander will get wrong "key" position. - // To reproduce, use the "await sleep" below, + // Upstream bug: when using "multiword+promise", TextExpander will get wrong "key" position. + // To reproduce, comment out the "shouldShowIssueSuggestions" check, use the "await sleep" below, // then use content "close #20\nclose #20\close #20", keep changing the last line `#20` part from the end (including removing the `#`) // 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). if (!shouldShowIssueSuggestions()) return {matched: false}; From 8b459a2aa8d03fb4cfd0392073db9b6d1e1e6e1b Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sat, 25 Jan 2025 22:38:03 +0800 Subject: [PATCH 3/7] improve comment --- web_src/js/features/comp/TextExpander.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/web_src/js/features/comp/TextExpander.ts b/web_src/js/features/comp/TextExpander.ts index 36925c2e386ff..a683ad6dd8db0 100644 --- a/web_src/js/features/comp/TextExpander.ts +++ b/web_src/js/features/comp/TextExpander.ts @@ -60,6 +60,7 @@ export function initTextExpander(expander: TextExpanderElement) { }; const debouncedIssueSuggestions = debounce(async (key: string, text: string): Promise => { + // https://github.com/github/text-expander-element/issues/71 // Upstream bug: when using "multiword+promise", TextExpander will get wrong "key" position. // To reproduce, comment out the "shouldShowIssueSuggestions" check, use the "await sleep" below, // then use content "close #20\nclose #20\close #20", keep changing the last line `#20` part from the end (including removing the `#`) From 5f3d83f01ddbfb6550df9d250d46ced0ec398d59 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sun, 26 Jan 2025 20:49:32 +0800 Subject: [PATCH 4/7] Update web_src/js/features/comp/TextExpander.ts --- web_src/js/features/comp/TextExpander.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/web_src/js/features/comp/TextExpander.ts b/web_src/js/features/comp/TextExpander.ts index a683ad6dd8db0..1397f3a510c23 100644 --- a/web_src/js/features/comp/TextExpander.ts +++ b/web_src/js/features/comp/TextExpander.ts @@ -65,8 +65,10 @@ export function initTextExpander(expander: TextExpanderElement) { // To reproduce, comment out the "shouldShowIssueSuggestions" check, use the "await sleep" below, // then use content "close #20\nclose #20\close #20", keep changing the last line `#20` part from the end (including removing the `#`) // 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). + // check the input before the request, to avoid emitting empty query to backend (still related to the upstream bug) if (!shouldShowIssueSuggestions()) return {matched: false}; // await sleep(Math.random() * 1000); // help to reproduce the text-expander bug + // check the input again to avoid text-expander use incorrect position (upstream bug) const ret = await fetchIssueSuggestions(key, text); if (!shouldShowIssueSuggestions()) return {matched: false}; return ret; From 6765b06eb444c565061fdc01593eb9133e469231 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sun, 26 Jan 2025 20:50:10 +0800 Subject: [PATCH 5/7] Update web_src/js/features/comp/TextExpander.ts --- web_src/js/features/comp/TextExpander.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/web_src/js/features/comp/TextExpander.ts b/web_src/js/features/comp/TextExpander.ts index 1397f3a510c23..c6f812790e8a2 100644 --- a/web_src/js/features/comp/TextExpander.ts +++ b/web_src/js/features/comp/TextExpander.ts @@ -65,6 +65,7 @@ export function initTextExpander(expander: TextExpanderElement) { // To reproduce, comment out the "shouldShowIssueSuggestions" check, use the "await sleep" below, // then use content "close #20\nclose #20\close #20", keep changing the last line `#20` part from the end (including removing the `#`) // 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). + // check the input before the request, to avoid emitting empty query to backend (still related to the upstream bug) if (!shouldShowIssueSuggestions()) return {matched: false}; // await sleep(Math.random() * 1000); // help to reproduce the text-expander bug From 95c610cdbd0b9bc45c9b8f7127a3daab0cf943ab Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sun, 26 Jan 2025 21:06:03 +0800 Subject: [PATCH 6/7] Update web_src/js/features/comp/TextExpander.ts --- web_src/js/features/comp/TextExpander.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web_src/js/features/comp/TextExpander.ts b/web_src/js/features/comp/TextExpander.ts index c6f812790e8a2..08823d1d01e9b 100644 --- a/web_src/js/features/comp/TextExpander.ts +++ b/web_src/js/features/comp/TextExpander.ts @@ -69,8 +69,8 @@ export function initTextExpander(expander: TextExpanderElement) { // check the input before the request, to avoid emitting empty query to backend (still related to the upstream bug) if (!shouldShowIssueSuggestions()) return {matched: false}; // await sleep(Math.random() * 1000); // help to reproduce the text-expander bug - // check the input again to avoid text-expander use incorrect position (upstream bug) const ret = await fetchIssueSuggestions(key, text); + // check the input again to avoid text-expander use incorrect position (upstream bug) if (!shouldShowIssueSuggestions()) return {matched: false}; return ret; }, 300); // to match onInputDebounce delay From f19385426dac3611dc26d41eca376cb392989559 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sun, 26 Jan 2025 21:07:28 +0800 Subject: [PATCH 7/7] improve comment --- web_src/js/features/comp/TextExpander.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/web_src/js/features/comp/TextExpander.ts b/web_src/js/features/comp/TextExpander.ts index 08823d1d01e9b..1e6d46f977b70 100644 --- a/web_src/js/features/comp/TextExpander.ts +++ b/web_src/js/features/comp/TextExpander.ts @@ -63,14 +63,14 @@ export function initTextExpander(expander: TextExpanderElement) { // https://github.com/github/text-expander-element/issues/71 // Upstream bug: when using "multiword+promise", TextExpander will get wrong "key" position. // To reproduce, comment out the "shouldShowIssueSuggestions" check, use the "await sleep" below, - // then use content "close #20\nclose #20\close #20", keep changing the last line `#20` part from the end (including removing the `#`) + // then use content "close #20\nclose #20\nclose #20" (3 lines), keep changing the last line `#20` part from the end (including removing the `#`) // 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). // check the input before the request, to avoid emitting empty query to backend (still related to the upstream bug) if (!shouldShowIssueSuggestions()) return {matched: false}; // await sleep(Math.random() * 1000); // help to reproduce the text-expander bug const ret = await fetchIssueSuggestions(key, text); - // check the input again to avoid text-expander use incorrect position (upstream bug) + // check the input again to avoid text-expander using incorrect position (upstream bug) if (!shouldShowIssueSuggestions()) return {matched: false}; return ret; }, 300); // to match onInputDebounce delay