Make issue suggestion work for all editors (#33340)

And do not handle special keys when the text-expander popup exists

Author: wxiaoguang

web_src/js/features/comp/EditorMarkdown.ts

@@ -184,8 +184,13 @@ function handleNewline(textarea: HTMLTextAreaElement, e: Event) {
   triggerEditorContentChanged(textarea);
 }
 
+function isTextExpanderShown(textarea: HTMLElement): boolean {
+  return Boolean(textarea.closest('text-expander')?.querySelector('.suggestions'));
+}
+
 export function initTextareaMarkdown(textarea) {
   textarea.addEventListener('keydown', (e) => {
+    if (isTextExpanderShown(textarea)) return;
     if (e.key === 'Tab' && !e.ctrlKey && !e.metaKey && !e.altKey) {
       // use Tab/Shift-Tab to indent/unindent the selected lines
       handleIndentSelection(textarea, e);

web_src/js/features/comp/TextExpander.ts

@@ -1,14 +1,19 @@
 import {matchEmoji, matchMention, matchIssue} from '../../utils/match.ts';
 import {emojiString} from '../emoji.ts';
 import {svg} from '../../svg.ts';
-import {parseIssueHref, parseIssueNewHref} from '../../utils.ts';
+import {parseIssueHref, parseRepoOwnerPathInfo} from '../../utils.ts';
 import {createElementFromAttrs, createElementFromHTML} from '../../utils/dom.ts';
 import {getIssueColor, getIssueIcon} from '../issue.ts';
 import {debounce} from 'perfect-debounce';
 
 const debouncedSuggestIssues = debounce((key: string, text: string) => new Promise<{matched:boolean; fragment?: HTMLElement}>(async (resolve) => {
-  let issuePathInfo = parseIssueHref(window.location.href);
-  if (!issuePathInfo.ownerName) issuePathInfo = parseIssueNewHref(window.location.href);
+  const issuePathInfo = parseIssueHref(window.location.href);
+  if (!issuePathInfo.ownerName) {
+    const repoOwnerPathInfo = parseRepoOwnerPathInfo(window.location.pathname);
+    issuePathInfo.ownerName = repoOwnerPathInfo.ownerName;
+    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});
 
   const matches = await matchIssue(issuePathInfo.ownerName, issuePathInfo.repoName, issuePathInfo.indexString, text);

web_src/js/types.ts

@@ -30,6 +30,11 @@ export type RequestOpts = {
   data?: RequestData,
 } & RequestInit;
 
+export type RepoOwnerPathInfo = {
+  ownerName: string,
+  repoName: string,
+}
+
 export type IssuePathInfo = {
   ownerName: string,
   repoName: string,

web_src/js/utils.test.ts

@@ -1,7 +1,7 @@
 import {
   basename, extname, isObject, stripTags, parseIssueHref,
   parseUrl, translateMonth, translateDay, blobToDataURI,
-  toAbsoluteUrl, encodeURLEncodedBase64, decodeURLEncodedBase64, isImageFile, isVideoFile, parseIssueNewHref,
+  toAbsoluteUrl, encodeURLEncodedBase64, decodeURLEncodedBase64, isImageFile, isVideoFile, parseRepoOwnerPathInfo,
 } from './utils.ts';
 
 test('basename', () => {
@@ -45,12 +45,14 @@ test('parseIssueHref', () => {
   expect(parseIssueHref('')).toEqual({ownerName: undefined, repoName: undefined, type: undefined, index: undefined});
 });
 
-test('parseIssueNewHref', () => {
-  expect(parseIssueNewHref('/owner/repo/issues/new')).toEqual({ownerName: 'owner', repoName: 'repo', pathType: 'issues'});
-  expect(parseIssueNewHref('/owner/repo/issues/new?query')).toEqual({ownerName: 'owner', repoName: 'repo', pathType: 'issues'});
-  expect(parseIssueNewHref('/sub/owner/repo/issues/new#hash')).toEqual({ownerName: 'owner', repoName: 'repo', pathType: 'issues'});
-  expect(parseIssueNewHref('/sub/owner/repo/compare/feature/branch-1...fix/branch-2')).toEqual({ownerName: 'owner', repoName: 'repo', pathType: 'pulls'});
-  expect(parseIssueNewHref('/other')).toEqual({});
+test('parseRepoOwnerPathInfo', () => {
+  expect(parseRepoOwnerPathInfo('/owner/repo/issues/new')).toEqual({ownerName: 'owner', repoName: 'repo'});
+  expect(parseRepoOwnerPathInfo('/owner/repo/releases')).toEqual({ownerName: 'owner', repoName: 'repo'});
+  expect(parseRepoOwnerPathInfo('/other')).toEqual({});
+  window.config.appSubUrl = '/sub';
+  expect(parseRepoOwnerPathInfo('/sub/owner/repo/issues/new')).toEqual({ownerName: 'owner', repoName: 'repo'});
+  expect(parseRepoOwnerPathInfo('/sub/owner/repo/compare/feature/branch-1...fix/branch-2')).toEqual({ownerName: 'owner', repoName: 'repo'});
+  window.config.appSubUrl = '';
 });
 
 test('parseUrl', () => {

web_src/js/utils.ts

@@ -1,5 +1,5 @@
 import {decode, encode} from 'uint8-to-base64';
-import type {IssuePageInfo, IssuePathInfo} from './types.ts';
+import type {IssuePageInfo, IssuePathInfo, RepoOwnerPathInfo} from './types.ts';
 
 // transform /path/to/file.ext to file.ext
 export function basename(path: string): string {
@@ -32,16 +32,17 @@ export function stripTags(text: string): string {
 }
 
 export function parseIssueHref(href: string): IssuePathInfo {
+  // FIXME: it should use pathname and trim the appSubUrl ahead
   const path = (href || '').replace(/[#?].*$/, '');
   const [_, ownerName, repoName, pathType, indexString] = /([^/]+)\/([^/]+)\/(issues|pulls)\/([0-9]+)/.exec(path) || [];
   return {ownerName, repoName, pathType, indexString};
 }
 
-export function parseIssueNewHref(href: string): IssuePathInfo {
-  const path = (href || '').replace(/[#?].*$/, '');
-  const [_, ownerName, repoName, pathTypeField] = /([^/]+)\/([^/]+)\/(issues\/new|compare\/.+\.\.\.)/.exec(path) || [];
-  const pathType = pathTypeField ? (pathTypeField.startsWith('issues/new') ? 'issues' : 'pulls') : undefined;
-  return {ownerName, repoName, pathType};
+export function parseRepoOwnerPathInfo(pathname: string): RepoOwnerPathInfo {
+  const appSubUrl = window.config.appSubUrl;
+  if (appSubUrl && pathname.startsWith(appSubUrl)) pathname = pathname.substring(appSubUrl.length);
+  const [_, ownerName, repoName] = /([^/]+)\/([^/]+)/.exec(pathname) || [];
+  return {ownerName, repoName};
 }
 
 export function parseIssuePageInfo(): IssuePageInfo {