Skip to content

Small improvements to MarkdownEditor component #2236

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Aug 23, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/improve-markdown-editor.md
Original file line number Diff line number Diff line change
@@ -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`.
6 changes: 3 additions & 3 deletions docs/content/drafts/MarkdownViewer.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -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.'}} />
)
}

Expand All @@ -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)}
/>
)
Expand Down Expand Up @@ -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}
Expand Down
38 changes: 37 additions & 1 deletion src/drafts/MarkdownEditor/MarkdownEditor.test.tsx
Original file line number Diff line number Diff line change
@@ -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'

Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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>
)
}
})
})
64 changes: 57 additions & 7 deletions src/drafts/MarkdownEditor/MarkdownEditor.tsx
Original file line number Diff line number Diff line change
@@ -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'
Expand All @@ -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. */
Expand Down Expand Up @@ -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
Expand All @@ -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.
*/
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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()
Expand All @@ -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()
Expand All @@ -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}),
Expand Down Expand Up @@ -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
Expand All @@ -380,6 +431,5 @@ const MarkdownEditor = forwardRef<MarkdownEditorHandle, MarkdownEditorProps>(
)
}
)
MarkdownEditor.displayName = 'MarkdownEditor'

export default MarkdownEditor
5 changes: 5 additions & 0 deletions src/drafts/MarkdownEditor/utils.ts
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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