Render the MarkdownContent string fallback React-natively and drop the imperative enhancers (#6619) (#61967)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: heiskr

src/frame/components/CodeTabs.tsx

@@ -17,7 +17,6 @@ import { RestRedirect } from '@/rest/components/RestRedirect'
 import { Breadcrumbs } from '@/frame/components/page-header/Breadcrumbs'
 import { LinkPreviewPopover } from '@/links/components/LinkPreviewPopover'
 import { UtmPreserver } from '@/frame/components/UtmPreserver'
-import { CodeTabs } from '@/frame/components/CodeTabs'
 import { JourneyTrackCard, JourneyTrackNav } from '@/journeys/components'
 import { CopyMarkdownMenu } from './ViewMarkdownButton'
 import { ExperimentContentSwap } from '@/events/components/experiments/ExperimentContentSwap'
@@ -103,9 +102,6 @@ export const ArticlePage = () => {
         <CodeTabsProvider>
           <LinkPreviewPopover />
           <UtmPreserver />
-          {/* The imperative CodeTabs enhancer only runs for the string fallback
-              path; on the hast path, CodeTabsGroup renders tabs React-natively. */}
-          {!renderedPageHast && <CodeTabs />}
           {isDev && <ClientSideRefresh />}
           {router.pathname.includes('/rest/') && <RestRedirect />}
           {currentLayout === 'inline' ? (

src/frame/components/article/ArticlePage.tsx

@@ -6,6 +6,7 @@ import type { Root as HastRoot } from 'hast'
 import cx from 'classnames'
 
 import { markdownComponents } from './markdownComponents'
+import { renderHTMLString } from '@/frame/components/ui/RenderedHTML/render-html-string'
 import styles from './MarkdownContent.module.scss'
 
 export type MarkdownContentPropsT = {
@@ -15,22 +16,23 @@ export type MarkdownContentPropsT = {
   as?: keyof JSX.IntrinsicElements
 }
 
-// Memoized so that re-renders of the parent (e.g. when ToolPicker/PlatformPicker
-// state updates) don't cause React 19 to re-apply `dangerouslySetInnerHTML` and
-// wipe out the inline `display` styles set imperatively by the pickers.
+// Memoized so that unrelated parent re-renders (e.g. ToolPicker/PlatformPicker
+// state updates) don't re-parse the HTML string and rebuild the body's element
+// tree, which is the expensive part of rendering an article.
 export const MarkdownContent = memo(function MarkdownContent({
   children,
   hast,
   as: Component = 'div',
   className,
   ...restProps
 }: MarkdownContentPropsT) {
-  // When a hast (HTML AST) tree is provided, render it as real React elements
-  // instead of injecting an HTML string via dangerouslySetInnerHTML (#6619).
+  // Render trusted HTML as real React elements instead of injecting it as raw
+  // innerHTML (#6619). Prefer a hast (HTML AST) tree when provided; otherwise
+  // parse a rendered HTML string. Non-string children render as-is.
   const childProps = hast
     ? { children: toJsxRuntime(hast, { Fragment, jsx, jsxs, components: markdownComponents }) }
     : typeof children === 'string'
-      ? { dangerouslySetInnerHTML: { __html: children } }
+      ? { children: renderHTMLString(children, markdownComponents) }
       : { children }
 
   return (

src/frame/components/ui/MarkdownContent/MarkdownContent.tsx

@@ -15,40 +15,8 @@ const platforms = [
 
 // Nota bene: platform === os
 
-// Imperatively modify article content to show only the selected platform
-// find all platform-specific *block* elements and hide or show as appropriate
-// example: {% mac %} block content {% endmac %}
-function showPlatformSpecificContent(platform: string) {
-  const markdowns = Array.from(document.querySelectorAll<HTMLElement>('.ghd-tool'))
-  const platformMarkdowns = markdowns.filter((xel) =>
-    platforms.some((platformValue) => xel.classList.contains(platformValue.value)),
-  )
-  for (const el of platformMarkdowns) {
-    el.style.display = el.classList.contains(platform) ? '' : 'none'
-
-    // hack: special handling for minitoc links -- we can't pass the tool classes
-    // directly to the Primer NavList.Item generated <li>, it gets passed down
-    // to the child <a>.  So if we find an <a> that has the tool class and its
-    // parent is an <li>, we hide/unhide that element as well.
-    if (el.tagName === 'A' && el.parentElement && el.parentElement.tagName === 'LI') {
-      el.parentElement.style.display = el.classList.contains(platform) ? '' : 'none'
-    }
-  }
-
-  // find all platform-specific *inline* elements and hide or show as appropriate
-  // example: <span class="platform-mac">inline content</span>
-  const platformEls = Array.from(
-    document.querySelectorAll<HTMLElement>(
-      platforms.map((platformOption) => `.platform-${platformOption.value}`).join(', '),
-    ),
-  )
-  for (const el of platformEls) {
-    el.style.display = el.classList.contains(`platform-${platform}`) ? '' : 'none'
-  }
-}
-
 export const PlatformPicker = () => {
-  const { defaultPlatform, detectedPlatforms, renderedPageHast } = useArticleContext()
+  const { defaultPlatform, detectedPlatforms } = useArticleContext()
   const { setPlatform } = useSelection()
 
   const [defaultUA, setDefaultUA] = useState('')
@@ -78,11 +46,10 @@ export const PlatformPicker = () => {
       cookieKey={OS_PREFERRED_COOKIE_NAME}
       queryStringKey={platformQueryKey}
       onValue={(value: string) => {
-        // Drive visibility through React state on the hast path (#6619). Only the
-        // string fallback (renderedPageHast undefined) still needs the imperative
-        // DOM mutation, since that markup isn't React-owned.
+        // Visibility is driven by React state via ToggleableContent/MiniTocs
+        // (#6619); the article body is React-owned on both the hast and string
+        // paths, so no imperative DOM mutation is needed.
         setPlatform(value)
-        if (!renderedPageHast) showPlatformSpecificContent(value)
       }}
       preferenceName="os"
       ariaLabel="Platform"

src/tools/components/PlatformPicker.tsx

@@ -1,5 +1,3 @@
-import { preserveAnchorNodePosition } from 'scroll-anchoring'
-
 import { useArticleContext } from '@/frame/components/context/ArticleContext'
 import { InArticlePicker } from './InArticlePicker'
 import { useSelection } from './SelectionContext'
@@ -10,38 +8,6 @@ import { TOOL_PREFERRED_COOKIE_NAME } from '@/frame/lib/constants'
 // Nota bene: tool === application
 // Nota bene: picker === switcher
 
-// Imperatively modify article content to show only the selected tool
-// find all platform-specific *block* elements and hide or show as appropriate
-// example: {% webui %} block content {% endwebui %}
-function showToolSpecificContent(tool: string, supportedTools: Array<string>) {
-  const markdowns = Array.from(document.querySelectorAll<HTMLElement>('.ghd-tool'))
-  const supportedMarkdowns = markdowns.filter((xel) =>
-    supportedTools.some((toolName) => xel.classList.contains(toolName)),
-  )
-  for (const el of supportedMarkdowns) {
-    el.style.display = el.classList.contains(tool) ? '' : 'none'
-
-    // hack: special handling for minitoc links -- we can't pass the tool classes
-    // directly to the Primer NavList.Item generated <li>, it gets passed down
-    // to the child <a>.  So if we find an <a> that has the tool class and its
-    // parent is an <li>, we hide/unhide that element as well.
-    if (el.tagName === 'A' && el.parentElement && el.parentElement.tagName === 'LI') {
-      el.parentElement.style.display = el.classList.contains(tool) ? '' : 'none'
-    }
-  }
-
-  // find all tool-specific *inline* elements and hide or show as appropriate
-  // example: <span class="tool-webui">inline content</span>
-  const toolEls = Array.from(
-    document.querySelectorAll<HTMLElement>(
-      supportedTools.map((toolOption) => `.tool-${toolOption}`).join(', '),
-    ),
-  )
-  for (const el of toolEls) {
-    el.style.display = el.classList.contains(`tool-${tool}`) ? '' : 'none'
-  }
-}
-
 function getDefaultTool(defaultTool: string | undefined, detectedTools: Array<string>): string {
   // If there is a default tool and the tool is present on this page
   if (defaultTool && detectedTools.includes(defaultTool)) return defaultTool
@@ -59,7 +25,7 @@ function getDefaultTool(defaultTool: string | undefined, detectedTools: Array<st
 const toolQueryKey = 'tool'
 export const ToolPicker = () => {
   // allTools comes from the ArticleContext which contains the list of tools available
-  const { defaultTool, detectedTools, allTools, renderedPageHast } = useArticleContext()
+  const { defaultTool, detectedTools, allTools } = useArticleContext()
   const { setTool } = useSelection()
 
   if (!detectedTools.length) return null
@@ -74,14 +40,10 @@ export const ToolPicker = () => {
       cookieKey={TOOL_PREFERRED_COOKIE_NAME}
       queryStringKey={toolQueryKey}
       onValue={(value: string) => {
-        // Drive visibility through React state on the hast path (#6619). Only the
-        // string fallback still needs the imperative DOM mutation.
+        // Visibility is driven by React state via ToggleableContent/MiniTocs
+        // (#6619); the article body is React-owned on both the hast and string
+        // paths, so no imperative DOM mutation is needed.
         setTool(value)
-        if (!renderedPageHast) {
-          preserveAnchorNodePosition(document, () => {
-            showToolSpecificContent(value, Object.keys(allTools))
-          })
-        }
       }}
       preferenceName="application"
       ariaLabel="Tool"