diff --git a/.changeset/chilly-dragons-crash.md b/.changeset/chilly-dragons-crash.md
new file mode 100644
index 00000000000..4dbbeace691
--- /dev/null
+++ b/.changeset/chilly-dragons-crash.md
@@ -0,0 +1,5 @@
+---
+"@primer/react": patch
+---
+
+`MarkdownViewer` is now SSR-compatible
diff --git a/src/drafts/MarkdownViewer/MarkdownViewer.test.tsx b/src/drafts/MarkdownViewer/MarkdownViewer.test.tsx
index af975617fdc..14d4e393a79 100644
--- a/src/drafts/MarkdownViewer/MarkdownViewer.test.tsx
+++ b/src/drafts/MarkdownViewer/MarkdownViewer.test.tsx
@@ -1,6 +1,6 @@
-import React from 'react'
-import {render, waitFor, fireEvent} from '@testing-library/react'
+import {fireEvent, render, waitFor} from '@testing-library/react'
import userEvent from '@testing-library/user-event'
+import React from 'react'
import MarkdownViewer from '../MarkdownViewer'
@@ -89,6 +89,42 @@ text after list`
fireEvent.change(items[0])
await waitFor(() => expect(onChangeMock).toHaveBeenCalledWith(noItemsCheckedMarkdown))
})
+
+ it('attaches event handlers to new tasks', async () => {
+ const onChangeMock = jest.fn()
+ const TestComponent = () => {
+ const [html, setHtml] = React.useState(taskListHtml)
+ const [markdown, setMarkdown] = React.useState(noItemsCheckedMarkdown)
+ return (
+ <>
+
+
+ >
+ )
+ }
+ const {getByRole, getAllByRole} = render()
+
+ // Change markdown and html content
+ const button = getByRole('button', {name: 'Add markdown'})
+ fireEvent.click(button)
+
+ const items = getAllByRole('checkbox')
+ fireEvent.change(items[2])
+ await waitFor(() => expect(onChangeMock).toHaveBeenCalledWith(`${noItemsCheckedMarkdown}\n- [x] item 3\n`))
+ })
})
describe('link interception', () => {
diff --git a/src/drafts/MarkdownViewer/MarkdownViewer.tsx b/src/drafts/MarkdownViewer/MarkdownViewer.tsx
index fe1636a2ae0..f34f08e25dc 100644
--- a/src/drafts/MarkdownViewer/MarkdownViewer.tsx
+++ b/src/drafts/MarkdownViewer/MarkdownViewer.tsx
@@ -1,5 +1,5 @@
-import React, {DOMAttributes, useCallback, useEffect, useRef, useState} from 'react'
-import {Spinner, Box} from '../..'
+import React, {DOMAttributes, useCallback} from 'react'
+import {Box, Spinner} from '../..'
import {useLinkInterception} from './_useLinkInterception'
import {useListInteraction} from './_useListInteraction'
@@ -55,12 +55,6 @@ type NoninteractiveMarkdownViewerProps = CoreMarkdownViewerProps & {
export type MarkdownViewerProps = NoninteractiveMarkdownViewerProps | InteractiveMarkdownViewerProps
-const createRenderedContainer = (html: string): HTMLDivElement => {
- const div = document.createElement('div')
- div.innerHTML = html
- return div
-}
-
const MarkdownViewer = ({
dangerousRenderedHTML,
loading = false,
@@ -70,25 +64,25 @@ const MarkdownViewer = ({
onLinkClick,
openLinksInNewTab = false,
}: MarkdownViewerProps) => {
- const outputContainerRef = useRef(null)
-
- // Render the HTML into an internal container element so we can modify it before it becomes visible.
- // Using `unsafeInnerHTML` would require an effect to run after rendering which would cause flicker
- const [htmlContainer, setHtmlContainer] = useState(() => createRenderedContainer(dangerousRenderedHTML.__html))
- useEffect(
- () => setHtmlContainer(createRenderedContainer(dangerousRenderedHTML.__html)),
- [dangerousRenderedHTML.__html],
- )
+ // We're using state to store the HTML container because we want the value
+ // to re-run effects when it changes
+ const [htmlContainer, setHtmlContainer] = React.useState()
+ const htmlContainerRef = React.useCallback((node: HTMLElement | null) => {
+ if (!node) return
+ setHtmlContainer(node)
+ }, [])
const onChange = useCallback(
async (value: string) => {
try {
await externalOnChange?.(value)
} catch (error) {
- setHtmlContainer(createRenderedContainer(dangerousRenderedHTML.__html))
+ if (htmlContainer) {
+ htmlContainer.innerHTML = dangerousRenderedHTML.__html
+ }
}
},
- [externalOnChange, dangerousRenderedHTML.__html],
+ [externalOnChange, htmlContainer, dangerousRenderedHTML],
)
useListInteraction({
@@ -96,6 +90,7 @@ const MarkdownViewer = ({
disabled: disabled || !externalOnChange,
htmlContainer,
markdownValue,
+ dependencies: [dangerousRenderedHTML],
})
useLinkInterception({
@@ -104,20 +99,16 @@ const MarkdownViewer = ({
openLinksInNewTab,
})
- // If we were to inject the `...htmlContainer.children` instead of the container element itself,
- // those children elements would be moved from the `htmlContainer` to the `outputContainer`. Then if
- // other effects use `htmlContainer.querySelectorAll`, they wouldn't find any elements to affect
- useEffect(() => outputContainerRef.current?.replaceChildren(htmlContainer), [htmlContainer])
-
return loading ? (
) : (
div > :last-child': {mb: 0}}}
+ dangerouslySetInnerHTML={dangerousRenderedHTML}
/>
)
}
diff --git a/src/drafts/MarkdownViewer/_useLinkInterception.ts b/src/drafts/MarkdownViewer/_useLinkInterception.ts
index 0c2e8a5f205..b5ccdccbebb 100644
--- a/src/drafts/MarkdownViewer/_useLinkInterception.ts
+++ b/src/drafts/MarkdownViewer/_useLinkInterception.ts
@@ -1,7 +1,7 @@
import {useEffect} from 'react'
type UseLinkInterceptionSettings = {
- htmlContainer: HTMLDivElement
+ htmlContainer?: HTMLElement
onLinkClick?: (event: MouseEvent) => void
openLinksInNewTab: boolean
}
@@ -23,6 +23,8 @@ export const useLinkInterception = ({htmlContainer, onLinkClick, openLinksInNewT
}
}
+ if (!htmlContainer) return
+
htmlContainer.addEventListener('click', clickHandler)
return () => {
diff --git a/src/drafts/MarkdownViewer/_useListInteraction.ts b/src/drafts/MarkdownViewer/_useListInteraction.ts
index 1f7c461ed19..1ce972970a1 100644
--- a/src/drafts/MarkdownViewer/_useListInteraction.ts
+++ b/src/drafts/MarkdownViewer/_useListInteraction.ts
@@ -1,4 +1,5 @@
-import {useCallback, useEffect, useLayoutEffect, useMemo, useRef} from 'react'
+import {useCallback, useEffect, useRef, useState} from 'react'
+import useIsomorphicLayoutEffect from '../../utils/useIsomorphicLayoutEffect'
import {ListItem, listItemToString, parseListItem} from '../MarkdownEditor/_useListEditing'
type TaskListItem = ListItem & {taskBox: '[ ]' | '[x]'}
@@ -11,10 +12,11 @@ const toggleTaskListItem = (item: TaskListItem): TaskListItem => ({
})
type UseListInteractionSettings = {
- htmlContainer: HTMLDivElement | null
+ htmlContainer?: HTMLElement
markdownValue: string
onChange: (markdown: string) => void | Promise
disabled?: boolean
+ dependencies?: Array
}
/**
@@ -28,11 +30,13 @@ export const useListInteraction = ({
markdownValue,
onChange,
disabled = false,
+ dependencies = [],
}: UseListInteractionSettings) => {
// Storing the value in a ref allows not using the markdown value as a depdency of
// onToggleItem, which would mean we'd have to re-bind the event handlers on every change
const markdownRef = useRef(markdownValue)
- useLayoutEffect(() => {
+
+ useIsomorphicLayoutEffect(() => {
markdownRef.current = markdownValue
}, [markdownValue])
@@ -62,12 +66,18 @@ export const useListInteraction = ({
[onChange],
)
- const checkboxElements = useMemo(
- () =>
- Array.from(
- htmlContainer?.querySelectorAll('input[type=checkbox].task-list-item-checkbox') ?? [],
- ),
- [htmlContainer],
+ const [checkboxElements, setCheckboxElements] = useState([])
+
+ useEffect(
+ () => {
+ setCheckboxElements(
+ Array.from(
+ htmlContainer?.querySelectorAll('input[type=checkbox].task-list-item-checkbox') ?? [],
+ ),
+ )
+ },
+ // eslint-disable-next-line react-hooks/exhaustive-deps
+ [htmlContainer, ...dependencies],
)
// This could be combined with the other effect, but then the checkboxes might have a flicker