Fix MarkdownEditor suggestions filtering bug and allow lazy-loading…

… suggestions (#2424)

* Fix test workaround

* Allow lazy-loading Markdown suggestions

* Update tests to demonstrate fuzzy filtering bug

* Fix type error

* Fix bug where short items incorrectly appear first in suggestions lists

* Create nine-apes-add.md

Co-authored-by: Siddharth Kshetrapal <siddharthkp@github.com>

.changeset/nine-apes-add.md

@@ -0,0 +1,5 @@
+---
+"@primer/react": patch
+---
+
+Fix `MarkdownEditor` suggestions filtering bug and allow lazy-loading suggestions

src/drafts/MarkdownEditor/MarkdownEditor.stories.tsx

@@ -1,6 +1,6 @@
 import {DiffIcon} from '@primer/octicons-react'
 import React, {Meta} from '@storybook/react'
-import {useState} from 'react'
+import {useRef, useState} from 'react'
 import BaseStyles from '../../BaseStyles'
 import Box from '../../Box'
 import MarkdownEditor, {Emoji, Mentionable, Reference, SavedReply} from '.'
@@ -301,3 +301,67 @@ export const CustomButtons = ({
     </>
   )
 }
+
+function useLazySuggestions<T>(suggestions: T[]) {
+  const promiseRef = useRef<Promise<T[]> | null>(null)
+
+  return () => {
+    // This simulates waiting to make an API  request until the first time the suggestions are needed
+    // Then, once we have made the API request we keep returning the same Promise which will already
+    // be resolved with the cached data
+    if (!promiseRef.current) {
+      promiseRef.current = new Promise(resolve => {
+        setTimeout(() => resolve(suggestions), 500)
+      })
+    }
+
+    return promiseRef.current
+  }
+}
+
+export const LazyLoadedSuggestions = ({
+  disabled,
+  fullHeight,
+  monospace,
+  minHeightLines,
+  maxHeightLines,
+  hideLabel,
+  required,
+  fileUploadsEnabled,
+  onSubmit,
+  savedRepliesEnabled,
+  pasteUrlsAsPlainText
+}: ArgProps) => {
+  const [value, setValue] = useState('')
+
+  const emojiSuggestions = useLazySuggestions(emojis)
+  const mentionSuggestions = useLazySuggestions(mentionables)
+  const referenceSuggestions = useLazySuggestions(references)
+
+  return (
+    <>
+      <MarkdownEditor
+        value={value}
+        onChange={setValue}
+        onPrimaryAction={onSubmit}
+        disabled={disabled}
+        fullHeight={fullHeight}
+        monospace={monospace}
+        minHeightLines={minHeightLines}
+        maxHeightLines={maxHeightLines}
+        placeholder="Enter some Markdown..."
+        onRenderPreview={renderPreview}
+        onUploadFile={fileUploadsEnabled ? onUploadFile : undefined}
+        emojiSuggestions={emojiSuggestions}
+        mentionSuggestions={mentionSuggestions}
+        referenceSuggestions={referenceSuggestions}
+        savedReplies={savedRepliesEnabled ? savedReplies : undefined}
+        required={required}
+        pasteUrlsAsPlainText={pasteUrlsAsPlainText}
+      >
+        <MarkdownEditor.Label visuallyHidden={hideLabel}>Markdown Editor Example</MarkdownEditor.Label>
+      </MarkdownEditor>
+      <p>Note: for demo purposes, files starting with &quot;A&quot; will be rejected.</p>
+    </>
+  )
+}

src/drafts/MarkdownEditor/MarkdownEditor.test.tsx

@@ -2,7 +2,7 @@ import {DiffAddedIcon} from '@primer/octicons-react'
 import {fireEvent, render as _render, waitFor, within} from '@testing-library/react'
 import userEvent from '@testing-library/user-event'
 import {UserEvent} from '@testing-library/user-event/dist/types/setup'
-import React, {forwardRef, useLayoutEffect, useRef, useState} from 'react'
+import React, {forwardRef, useRef, useState} from 'react'
 import MarkdownEditor, {Emoji, MarkdownEditorHandle, MarkdownEditorProps, Mentionable, Reference, SavedReply} from '.'
 import ThemeProvider from '../../ThemeProvider'
 
@@ -21,17 +21,6 @@ const UncontrolledEditor = forwardRef<MarkdownEditorHandle, UncontrolledEditorPr
 
   const onRenderPreview = async () => 'Preview'
 
-  useLayoutEffect(() => {
-    // combobox-nav attempts to filter out 'hidden' options by checking if the option has an
-    // offsetHeight or width > 0. In JSDom, all elements have offsetHeight = offsetWidth = 0,
-    // so we need to override at least one to make the class recognize that any options exist.
-    for (const option of document.querySelectorAll('[role=option]'))
-      Object.defineProperty(option, 'offsetHeight', {
-        value: 1,
-        writable: true
-      })
-  })
-
   return (
     <ThemeProvider>
       <MarkdownEditor
@@ -117,6 +106,20 @@ const render = async (ui: React.ReactElement) => {
 }
 
 describe('MarkdownEditor', () => {
+  // combobox-nav attempts to filter out 'hidden' options by checking if the option has an
+  // offsetHeight or width > 0. In JSDom, all elements have offsetHeight = offsetWidth = 0,
+  // so we need to override at least one to make the class recognize that any options exist.
+  const originalOffsetHeight = Object.getOwnPropertyDescriptor(HTMLElement.prototype, 'offsetHeight')
+  beforeAll(() => {
+    Object.defineProperty(HTMLElement.prototype, 'offsetHeight', {
+      configurable: true,
+      value: 10
+    })
+  })
+  afterAll(() => {
+    if (originalOffsetHeight) Object.defineProperty(HTMLElement.prototype, 'offsetHeight', originalOffsetHeight)
+  })
+
   beforeEach(() => {
     jest.mock('@primer/behaviors/utils', () => ({
       // for all tests, default to Non-Mac (Ctrl) keybindings
@@ -832,8 +835,6 @@ describe('MarkdownEditor', () => {
   })
 
   describe('suggestions', () => {
-    // we don't test filtering logic here because that's up to the consumer
-
     const emojis: Emoji[] = [
       {name: '+1', character: '👍'},
       {name: '-1', character: '👎'},
@@ -847,7 +848,10 @@ describe('MarkdownEditor', () => {
       {identifier: 'github', description: 'GitHub'},
       {identifier: 'primer', description: 'Primer'},
       {identifier: 'actions', description: 'Actions'},
-      {identifier: 'primer-css', description: ''}
+      {identifier: 'primer-css', description: ''},
+      {identifier: 'mnl', description: ''},
+      {identifier: 'gth', description: ''},
+      {identifier: 'mla', description: ''}
     ]
 
     const references: Reference[] = [
@@ -982,6 +986,20 @@ describe('MarkdownEditor', () => {
       })
     })
 
+    it('applies suggestion and hides list on %s-press', async () => {
+      const {queryForSuggestionsList, getAllSuggestions, getInput, user} = await render(<EditorWithSuggestions />)
+
+      const input = getInput()
+      await user.type(input, `hello :`)
+      expect(queryForSuggestionsList()).toBeInTheDocument()
+
+      await waitFor(() => expect(getAllSuggestions()[0]).toHaveAttribute('data-combobox-option-default'))
+
+      await user.keyboard(`{Enter}`)
+      expect(input.value).toBe(`hello 👍 `) // suggestions are inserted with a following space
+      expect(queryForSuggestionsList()).not.toBeInTheDocument()
+    })
+
     it('filters mention suggestions using fuzzy match against name', async () => {
       const {getInput, getAllSuggestions, user} = await render(<EditorWithSuggestions />)
       await user.type(getInput(), '@octct')
@@ -991,9 +1009,9 @@ describe('MarkdownEditor', () => {
 
     it('filters mention suggestions using fuzzy match against ID', async () => {
       const {getInput, getAllSuggestions, user} = await render(<EditorWithSuggestions />)
-      await user.type(getInput(), '@prmrcss')
+      await user.type(getInput(), '@git')
       expect(getAllSuggestions()).toHaveLength(1)
-      expect(getAllSuggestions()[0]).toHaveTextContent('primer-css')
+      expect(getAllSuggestions()[0]).toHaveTextContent('github')
     })
 
     it('filters reference suggestions using fuzzy match against name', async () => {

src/drafts/MarkdownEditor/MarkdownEditor.tsx

@@ -24,6 +24,7 @@ import {Emoji} from './suggestions/_useEmojiSuggestions'
 import {Mentionable} from './suggestions/_useMentionSuggestions'
 import {Reference} from './suggestions/_useReferenceSuggestions'
 import {isModifierKey} from './utils'
+import {SuggestionOptions} from './suggestions'
 
 export type MarkdownEditorProps = SxProp & {
   /** Current value of the editor as a multiline markdown string. */
@@ -69,12 +70,21 @@ export type MarkdownEditorProps = SxProp & {
    * @default 35
    */
   maxHeightLines?: number
-  /** Array of all possible emojis to suggest. Leave `undefined` to disable emoji autocomplete. */
-  emojiSuggestions?: Array<Emoji>
-  /** Array of all possible mention suggestions. Leave `undefined` to disable `@`-mention autocomplete. */
-  mentionSuggestions?: Array<Mentionable>
-  /** Array of all possible references to suggest. Leave `undefined` to disable `#`-reference autocomplete. */
-  referenceSuggestions?: Array<Reference>
+  /**
+   * Array of all possible emojis to suggest. Leave `undefined` to disable emoji autocomplete.
+   * For lazy-loading suggestions, an async function can be provided instead.
+   */
+  emojiSuggestions?: SuggestionOptions<Emoji>
+  /**
+   * Array of all possible mention suggestions. Leave `undefined` to disable `@`-mention autocomplete.
+   * For lazy-loading suggestions, an async function can be provided instead.
+   */
+  mentionSuggestions?: SuggestionOptions<Mentionable>
+  /**
+   * Array of all possible references to suggest. Leave `undefined` to disable `#`-reference autocomplete.
+   * For lazy-loading suggestions, an async function can be provided instead.
+   */
+  referenceSuggestions?: SuggestionOptions<Reference>
   /**
    * Uploads a file to a hosting service and returns the URL. If not provided, file uploads
    * will be disabled.

src/drafts/MarkdownEditor/_MarkdownInput.tsx

@@ -7,6 +7,7 @@ import {Emoji, useEmojiSuggestions} from './suggestions/_useEmojiSuggestions'
 import {Mentionable, useMentionSuggestions} from './suggestions/_useMentionSuggestions'
 import {Reference, useReferenceSuggestions} from './suggestions/_useReferenceSuggestions'
 import {useRefObjectAsForwardedRef} from '../../hooks'
+import {SuggestionOptions} from './suggestions'
 
 interface MarkdownInputProps extends Omit<TextareaProps, 'onChange'> {
   value: string
@@ -18,9 +19,9 @@ interface MarkdownInputProps extends Omit<TextareaProps, 'onChange'> {
   maxLength?: number
   fullHeight?: boolean
   isDraggedOver: boolean
-  emojiSuggestions?: Array<Emoji>
-  mentionSuggestions?: Array<Mentionable>
-  referenceSuggestions?: Array<Reference>
+  emojiSuggestions?: SuggestionOptions<Emoji>
+  mentionSuggestions?: SuggestionOptions<Mentionable>
+  referenceSuggestions?: SuggestionOptions<Reference>
   minHeightLines: number
   maxHeightLines: number
   monospace: boolean
@@ -70,13 +71,14 @@ export const MarkdownInput = forwardRef<HTMLTextAreaElement, MarkdownInputProps>
       [mentionsTrigger, referencesTrigger, emojiTrigger]
     )
 
-    const onShowSuggestions = (event: ShowSuggestionsEvent) => {
+    const onShowSuggestions = async (event: ShowSuggestionsEvent) => {
+      setSuggestions('loading')
       if (event.trigger.triggerChar === emojiTrigger.triggerChar) {
-        setSuggestions(calculateEmojiSuggestions(event.query))
+        setSuggestions(await calculateEmojiSuggestions(event.query))
       } else if (event.trigger.triggerChar === mentionsTrigger.triggerChar) {
-        setSuggestions(calculateMentionSuggestions(event.query))
+        setSuggestions(await calculateMentionSuggestions(event.query))
       } else if (event.trigger.triggerChar === referencesTrigger.triggerChar) {
-        setSuggestions(calculateReferenceSuggestions(event.query))
+        setSuggestions(await calculateReferenceSuggestions(event.query))
       }
     }
 
@@ -97,7 +99,7 @@ export const MarkdownInput = forwardRef<HTMLTextAreaElement, MarkdownInputProps>
       <InlineAutocomplete
         triggers={triggers}
         suggestions={suggestions}
-        onShowSuggestions={onShowSuggestions}
+        onShowSuggestions={e => onShowSuggestions(e)}
         onHideSuggestions={() => setSuggestions(null)}
         sx={{flex: 'auto'}}
         tabInsertsSuggestions

src/drafts/MarkdownEditor/suggestions/_useMentionSuggestions.tsx

@@ -26,8 +26,16 @@ const mentionableToSuggestion = (mentionable: Mentionable): Suggestion => ({
   )
 })
 
-const scoreSuggestion = (query: string, mentionable: Mentionable): number =>
-  score(query, `${mentionable.identifier} ${mentionable.description}`.trim().toLowerCase())
+const scoreSuggestion = (query: string, mentionable: Mentionable): number => {
+  const fzyScore = score(query, `${mentionable.identifier} ${mentionable.description}`.trim().toLowerCase())
+
+  // fzy unintuitively returns Infinity if the length of the item is less than or equal to the length of the query
+  // All users have an identifier but some have empty descriptions; in those cases the query might equal the identifier
+  // and we'd still want to show the suggestion in that case.
+  if (fzyScore === Infinity && query.toLowerCase() !== mentionable.identifier.toLowerCase()) return -Infinity
+
+  return fzyScore
+}
 
 export const useMentionSuggestions: UseSuggestionsHook<Mentionable> = mentionables => ({
   calculateSuggestions: suggestionsCalculator(mentionables, scoreSuggestion, mentionableToSuggestion),

src/drafts/MarkdownEditor/suggestions/_useReferenceSuggestions.tsx

@@ -43,11 +43,16 @@ const referenceToSuggestion = (reference: Reference): Suggestion => ({
   )
 })
 
-const scoreSuggestion = (query: string, reference: Reference): number =>
-  score(query, `${reference.id} ${reference.titleText}`)
+const scoreSuggestion = (query: string, reference: Reference): number => {
+  // fzy unituitively returns Infinity if the length of the item is less than or equal to the length of the query
+  const fzyScore = score(query, `${reference.id} ${reference.titleText}`)
+  // Here, unlike for mentionables, we don't need to check for equality because the user's query
+  // can never equal the search string (we don't do filtering if the query is in "#123 some text" form)
+  return fzyScore === Infinity ? -Infinity : fzyScore
+}
 
 export const useReferenceSuggestions: UseSuggestionsHook<Reference> = references => ({
-  calculateSuggestions: (query: string) => {
+  calculateSuggestions: async (query: string) => {
     if (/^\d+\s/.test(query)) return [] // don't return anything if the query is in the form #123 ..., assuming they already have the number they want
     return suggestionsCalculator(references, scoreSuggestion, referenceToSuggestion)(query)
   },

src/drafts/MarkdownEditor/suggestions/index.ts

@@ -2,23 +2,31 @@ import {Suggestion, Trigger} from '../../InlineAutocomplete'
 
 const MAX_SUGGESTIONS = 5
 
-export type UseSuggestionsHook<T> = (options: T[]) => {
+export type SuggestionOptions<T> = T[] | (() => Promise<T[]>)
+
+export type UseSuggestionsHook<T> = (options: SuggestionOptions<T>) => {
   trigger: Trigger
-  calculateSuggestions: (query: string) => Suggestion[]
+  calculateSuggestions: (query: string) => Promise<Suggestion[]>
 }
 
 export const suggestionsCalculator =
-  <T>(options: T[], score: (query: string, option: T) => number, toSuggestion: (option: T) => Suggestion) =>
-  (query: string) => {
+  <T>(
+    options: SuggestionOptions<T>,
+    score: (query: string, option: T) => number,
+    toSuggestion: (option: T) => Suggestion
+  ) =>
+  async (query: string) => {
+    const optionsArray = Array.isArray(options) ? options : await options()
+
     // If the query is empty, scores will be -INFINITY
     const scoredAndSorted = query
-      ? options
+      ? optionsArray
           .map(o => [score(query, o), o] as const)
           .filter(([s]) => s > 0)
           .sort(([a], [b]) => b - a)
           .slice(0, MAX_SUGGESTIONS)
           .map(([, o]) => o)
-      : options.slice(0, MAX_SUGGESTIONS)
+      : optionsArray.slice(0, MAX_SUGGESTIONS)
 
     return scoredAndSorted.map(toSuggestion)
   }