Small improvements to MarkdownEditor component (#2236)

* Limit the type of the `ref` forwarded to `MarkdownEditor`

* Add cmd/ctrl+shift+P keyboard shortcut for toggling views

* Fix prop name typo in MarkdownViewer docs

* Create improve-markdown-editor.md

* Update src/drafts/MarkdownEditor/MarkdownEditor.tsx

* Fix syntax error

.changeset/improve-markdown-editor.md

@@ -0,0 +1,5 @@
+---
+"@primer/react": patch
+---
+
+Fix `MarkdownViewer` doc examples, add <kbd>Cmd/Ctrl+Shift+P</kbd> shortcut for toggling `MarkdownEditor` view mode, and strictly limit the type of the `ref` passed to `MarkdownEditor`.

docs/content/drafts/MarkdownViewer.mdx

@@ -19,7 +19,7 @@ The `MarkdownViewer` displays rendered Markdown with appropriate styling and han
 const MarkdownViewerExample = () => {
   return (
     // eslint-disable-next-line github/unescaped-html-literal
-    <MarkdownViewer dangerousRenderedHtml={{__html: '<strong>Lorem ipsum</strong> dolor sit amet.'}} />
+    <MarkdownViewer dangerousRenderedHTML={{__html: '<strong>Lorem ipsum</strong> dolor sit amet.'}} />
   )
 }
 
@@ -33,7 +33,7 @@ const MarkdownViewerExample = () => {
   return (
     <MarkdownViewer
       // eslint-disable-next-line github/unescaped-html-literal
-      dangerousRenderedHtml={{__html: "<a href='https://example.com'>Example link</a>"}}
+      dangerousRenderedHTML={{__html: "<a href='https://example.com'>Example link</a>"}}
       onLinkClick={ev => console.log(ev)}
     />
   )
@@ -64,7 +64,7 @@ const renderedHtml = `
 const MarkdownViewerExample = () => {
   return (
     <MarkdownViewer
-      dangerousRenderedHtml={{__html: renderedHtml}}
+      dangerousRenderedHTML={{__html: renderedHtml}}
       markdownValue={markdownSource}
       onChange={value => console.log(value) /* save the value to the server */}
       disabled={false}

src/drafts/MarkdownEditor/MarkdownEditor.test.tsx

@@ -1,7 +1,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 React, {forwardRef, useLayoutEffect, useState} from 'react'
+import React, {forwardRef, useLayoutEffect, useRef, useState} from 'react'
 import MarkdownEditor, {Emoji, MarkdownEditorHandle, MarkdownEditorProps, Mentionable, Reference, SavedReply} from '.'
 import ThemeProvider from '../../ThemeProvider'
 
@@ -231,6 +231,22 @@ describe('MarkdownEditor', () => {
     expect(getInput()).toHaveAttribute('name', 'Name')
   })
 
+  describe('toggles between view modes on ctrl/cmd+shift+P', () => {
+    const shortcut = '{Control>}{Shift>}{P}{/Control}{/Shift}'
+
+    it('enters preview mode when editing', async () => {
+      const {getInput, user} = await render(<UncontrolledEditor />)
+      await user.type(getInput(), shortcut)
+    })
+
+    it('enters edit mode when previewing', async () => {
+      const {getInput, user, getViewSwitch} = await render(<UncontrolledEditor />)
+      await user.click(getViewSwitch())
+      await user.keyboard(shortcut)
+      expect(getInput()).toHaveFocus()
+    })
+  })
+
   describe('action buttons', () => {
     it('renders custom action buttons', async () => {
       const {getActionButton} = await render(
@@ -1104,4 +1120,24 @@ describe('MarkdownEditor', () => {
       expect(getInput()).toHaveFocus()
     })
   })
+
+  it('uses types to prevent assigning HTMLTextAreaElement ref to MarkdownEditor', () => {
+    // eslint-disable-next-line @typescript-eslint/no-unused-vars
+    const Element = () => {
+      const inputRef = useRef<HTMLTextAreaElement>(null)
+      return (
+        <MarkdownEditor
+          // @ts-expect-error Ref<HTMLTextAreaElement> should not be assignable to Ref<MarkdownEditorHandle>
+          ref={inputRef}
+          value=""
+          onChange={() => {
+            /*noop*/
+          }}
+          onRenderPreview={async () => 'preview'}
+        >
+          <MarkdownEditor.Label>Test</MarkdownEditor.Label>
+        </MarkdownEditor>
+      )
+    }
+  })
 })

src/drafts/MarkdownEditor/MarkdownEditor.tsx

@@ -1,4 +1,3 @@
-import {isMacOS} from '@primer/behaviors/utils'
 import {useSSRSafeId} from '@react-aria/ssr'
 import React, {forwardRef, useCallback, useEffect, useImperativeHandle, useMemo, useRef, useState} from 'react'
 import Box from '../../Box'
@@ -24,6 +23,7 @@ import {SavedRepliesContext, SavedRepliesHandle, SavedReply} from './_SavedRepli
 import {Emoji} from './suggestions/_useEmojiSuggestions'
 import {Mentionable} from './suggestions/_useMentionSuggestions'
 import {Reference} from './suggestions/_useReferenceSuggestions'
+import {isModifierKey} from './utils'
 
 export type MarkdownEditorProps = SxProp & {
   /** Current value of the editor as a multiline markdown string. */
@@ -97,11 +97,19 @@ export type MarkdownEditorProps = SxProp & {
   savedReplies?: SavedReply[]
 }
 
+const handleBrand = Symbol()
+
 export interface MarkdownEditorHandle {
   /** Focus on the markdown textarea (has no effect in preview mode). */
   focus: (options?: FocusOptions) => void
   /** Scroll to the editor. */
   scrollIntoView: (options?: ScrollIntoViewOptions) => void
+  /**
+   * This 'fake' member prevents other types from being assigned to this, thus
+   * disallowing broader ref types like `HTMLTextAreaElement`.
+   * @private
+   */
+  [handleBrand]: undefined
 }
 
 const a11yOnlyStyle = {clipPath: 'Circle(0)', position: 'absolute'} as const
@@ -111,6 +119,15 @@ const CONDENSED_WIDTH_THRESHOLD = 675
 const {Slot, Slots} = createSlots(['Toolbar', 'Actions', 'Label'])
 export const MarkdownEditorSlot = Slot
 
+/**
+ * We want to switch editors from preview mode on cmd/ctrl+shift+P. But in preview mode,
+ * there's no input to focus so we have to bind the event to the document. If there are
+ * multiple editors, we want the most recent one to switch to preview mode to be the one
+ * that we switch back to edit mode, so we maintain a LIFO stack of IDs of editors in
+ * preview mode.
+ */
+let editorsInPreviewMode: string[] = []
+
 /**
  * Markdown textarea with controls & keyboard shortcuts.
  */
@@ -171,10 +188,14 @@ const MarkdownEditor = forwardRef<MarkdownEditorHandle, MarkdownEditorProps>(
     })
 
     const inputRef = useRef<HTMLTextAreaElement>(null)
-    useImperativeHandle(ref, () => ({
-      focus: opts => inputRef.current?.focus(opts),
-      scrollIntoView: opts => containerRef.current?.scrollIntoView(opts)
-    }))
+    useImperativeHandle(
+      ref,
+      () =>
+        ({
+          focus: opts => inputRef.current?.focus(opts),
+          scrollIntoView: opts => containerRef.current?.scrollIntoView(opts)
+        } as MarkdownEditorHandle)
+    )
 
     const inputHeight = useRef(0)
     if (inputRef.current && inputRef.current.offsetHeight) inputHeight.current = inputRef.current.offsetHeight
@@ -233,7 +254,7 @@ const MarkdownEditor = forwardRef<MarkdownEditorHandle, MarkdownEditorProps>(
           savedRepliesRef.current?.openMenu()
           e.preventDefault()
           e.stopPropagation()
-        } else if (isMacOS() ? e.metaKey : e.ctrlKey) {
+        } else if (isModifierKey(e)) {
           if (e.key === 'Enter') onPrimaryAction?.()
           else if (e.key === 'b') format?.bold()
           else if (e.key === 'i') format?.italic()
@@ -243,6 +264,7 @@ const MarkdownEditor = forwardRef<MarkdownEditorHandle, MarkdownEditorProps>(
           else if (e.key === '8') format?.unorderedList()
           else if (e.shiftKey && e.key === '7') format?.orderedList()
           else if (e.shiftKey && e.key === 'l') format?.taskList()
+          else if (e.shiftKey && e.key === 'p') setView?.('preview')
           else return
 
           e.preventDefault()
@@ -254,6 +276,34 @@ const MarkdownEditor = forwardRef<MarkdownEditorHandle, MarkdownEditorProps>(
       }
     )
 
+    useEffect(() => {
+      if (view === 'preview') {
+        editorsInPreviewMode.push(id)
+
+        const handler = (e: KeyboardEvent) => {
+          if (
+            !e.defaultPrevented &&
+            editorsInPreviewMode.at(-1) === id &&
+            isModifierKey(e) &&
+            e.shiftKey &&
+            e.key === 'p'
+          ) {
+            setView?.('edit')
+            setTimeout(() => inputRef.current?.focus())
+            e.preventDefault()
+          }
+        }
+        document.addEventListener('keydown', handler)
+
+        return () => {
+          document.removeEventListener('keydown', handler)
+          // Performing the filtering in the cleanup callback allows it to happen also when
+          // the user clicks the toggle button, not just on keyboard shortcut
+          editorsInPreviewMode = editorsInPreviewMode.filter(id_ => id_ !== id)
+        }
+      }
+    }, [view, setView, id])
+
     // If we don't memoize the context object, every child will rerender on every render even if memoized
     const context = useMemo(
       () => ({disabled, formattingToolsRef, condensed, required}),
@@ -354,6 +404,7 @@ const MarkdownEditor = forwardRef<MarkdownEditorHandle, MarkdownEditorProps>(
                       boxSizing: 'border-box'
                     }}
                     aria-live="polite"
+                    tabIndex={-1}
                   >
                     <h2 style={a11yOnlyStyle}>Rendered Markdown Preview</h2>
                     <MarkdownViewer
@@ -380,6 +431,5 @@ const MarkdownEditor = forwardRef<MarkdownEditorHandle, MarkdownEditorProps>(
     )
   }
 )
-MarkdownEditor.displayName = 'MarkdownEditor'
 
 export default MarkdownEditor

src/drafts/MarkdownEditor/utils.ts

@@ -1,3 +1,5 @@
+import {isMacOS} from '@primer/behaviors/utils'
+
 export const getSelectedLineRange = (textarea: HTMLTextAreaElement): [number, number] => {
   // Subtract one from the caret position so the newline found is not the one _at_ the caret position
   // then add one because we don't want to include the found newline. Also changes -1 (not found) result to 0
@@ -16,3 +18,6 @@ export const markdownLink = (text: string, url: string) =>
   `[${text.replaceAll('[', '\\[').replaceAll(']', '\\]')}](${url.replaceAll('(', '\\(').replaceAll(')', '\\)')})`
 
 export const markdownImage = (altText: string, url: string) => `!${markdownLink(altText, url)}`
+
+export const isModifierKey = (event: KeyboardEvent | React.KeyboardEvent<unknown>) =>
+  isMacOS() ? event.metaKey : event.ctrlKey