Merge pull request #407 from nicolethoen/admonitions_and_accordions

fix: correctly render admonitions and accordions

Author: wise-king-sullyman

packages/module/package.json

@@ -66,7 +66,7 @@
     "@rollup/plugin-commonjs": "^17.0.0",
     "@rollup/plugin-json": "^4.1.0",
     "@rollup/plugin-node-resolve": "^11.1.0",
-    "@testing-library/react": "^11.2.2",
+    "@testing-library/react": "^13.4.0",
     "@types/dompurify": "^3.0.5",
     "@types/enzyme": "^3.10.7",
     "@types/enzyme-adapter-react-16": "^1.0.6",

packages/module/src/ConsoleInternal/components/markdown-view.tsx

@@ -25,7 +25,7 @@ export const markdownConvert = async (markdown: string, extensions?: ShowdownExt
       return node;
     }
 
-    // add PF content classes
+    // add PF content classes to standard elements (details blocks get handled separately)
     if (node.nodeType === 1) {
       const contentElements = [
         'ul',
@@ -85,15 +85,98 @@ export const markdownConvert = async (markdown: string, extensions?: ShowdownExt
   );
   const markdownWithSubstitutedCodeFences = reverseString(reverseMarkdownWithSubstitutedCodeFences);
 
-  const parsedMarkdown = await marked.parse(markdownWithSubstitutedCodeFences);
+  // Fix malformed HTML entities early in the process
+  let preprocessedMarkdown = markdownWithSubstitutedCodeFences;
+  preprocessedMarkdown = preprocessedMarkdown
+    .replace(/&nbsp([^;])/g, ' $1')
+    .replace(/ /g, ' ');
+  preprocessedMarkdown = preprocessedMarkdown.replace(/&nbsp(?![;])/g, ' ');
+
+  // Process content in segments to ensure markdown parsing continues after HTML blocks
+  const htmlBlockRegex =
+    /(<(?:details|div|section|article)[^>]*>[\s\S]*?<\/(?:details|div|section|article)>)/g;
+
+  let parsedMarkdown = '';
+
+  // Check if there are any HTML blocks
+  if (htmlBlockRegex.test(preprocessedMarkdown)) {
+    // Reset regex for actual processing
+    htmlBlockRegex.lastIndex = 0;
+
+    let lastIndex = 0;
+    let match;
+
+    while ((match = htmlBlockRegex.exec(preprocessedMarkdown)) !== null) {
+      // Process markdown before the HTML block
+      const markdownBefore = preprocessedMarkdown.slice(lastIndex, match.index).trim();
+      if (markdownBefore) {
+        const parsed = await marked.parse(markdownBefore);
+        parsedMarkdown += parsed;
+      }
+
+      // Process the HTML block: parse markdown content inside while preserving HTML structure
+      let htmlBlock = match[1];
+
+      // Find and process markdown content inside HTML tags
+      const contentRegex = />(\s*[\s\S]*?)\s*</g;
+      const contentMatches = [];
+      let contentMatch;
+
+      while ((contentMatch = contentRegex.exec(htmlBlock)) !== null) {
+        const content = contentMatch[1];
+        // Only process content that has markdown formatting but no extension syntax
+        if (
+          content.trim() &&
+          !content.includes('{{') &&
+          (content.includes('**') || content.includes('- ') || content.includes('\n'))
+        ) {
+          // This looks like markdown content without extensions - parse it as block content
+          const parsedContent = await marked.parse(content.trim());
+          // Remove wrapping <p> tags if they exist since we're inside HTML already
+          const cleanedContent = parsedContent.replace(/^<p[^>]*>([\s\S]*)<\/p>[\s]*$/g, '$1');
+          contentMatches.push({
+            original: contentMatch[0],
+            replacement: `>${cleanedContent}<`,
+          });
+        }
+      }
+
+      // Apply the content replacements
+      contentMatches.forEach(({ original, replacement }) => {
+        htmlBlock = htmlBlock.replace(original, replacement);
+      });
+
+      // Apply extensions (like admonitions) to the processed HTML block
+      if (extensions) {
+        extensions.forEach(({ regex, replace }) => {
+          if (regex) {
+            htmlBlock = htmlBlock.replace(regex, replace);
+          }
+        });
+      }
+
+      parsedMarkdown += htmlBlock;
+      lastIndex = htmlBlockRegex.lastIndex;
+    }
+
+    // Process any remaining markdown after the last HTML block
+    const markdownAfter = preprocessedMarkdown.slice(lastIndex).trim();
+    if (markdownAfter) {
+      const parsed = await marked.parse(markdownAfter);
+      parsedMarkdown += parsed;
+    }
+  } else {
+    // No HTML blocks found, process normally
+    parsedMarkdown = await marked.parse(preprocessedMarkdown);
+  }
   // Swap the temporary tokens back to code fences before we run the extensions
   let md = parsedMarkdown.replace(/@@@/g, '```');
 
   if (extensions) {
     // Convert code spans back to md format before we run the custom extension regexes
     md = md.replace(/<code>(.*)<\/code>/g, '`$1`');
 
-    extensions.forEach(({ regex, replace }) => {
+    extensions.forEach(({ regex, replace }, _index) => {
       if (regex) {
         md = md.replace(regex, replace);
       }
@@ -102,6 +185,7 @@ export const markdownConvert = async (markdown: string, extensions?: ShowdownExt
     // Convert any remaining backticks back into code spans
     md = md.replace(/`(.*)`/g, '<code>$1</code>');
   }
+
   return DOMPurify.sanitize(md);
 };
 
@@ -210,7 +294,10 @@ const InlineMarkdownView: FC<InnerSyncMarkdownProps> = ({
   const id = useMemo(() => uniqueId('markdown'), []);
   return (
     <div className={css({ 'is-empty': isEmpty } as any, className)} id={id}>
-      <div dangerouslySetInnerHTML={{ __html: markup }} />
+      <div
+        style={{ marginBlockEnd: 'var(--pf-t-global--spacer--md)' }}
+        dangerouslySetInnerHTML={{ __html: markup }}
+      />
       {renderExtension && (
         <RenderExtension renderExtension={renderExtension} selector={`#${id}`} markup={markup} />
       )}
@@ -299,6 +386,7 @@ const IFrameMarkdownView: FC<InnerSyncMarkdownProps> = ({
   return (
     <>
       <iframe
+        title="Markdown content preview"
         sandbox="allow-popups allow-popups-to-escape-sandbox allow-same-origin"
         srcDoc={contents}
         style={{ border: '0px', display: 'block', width: '100%', height: '0' }}

packages/module/src/ConsoleShared/src/components/markdown-extensions/__tests__/accordion-extension.spec.tsx

@@ -0,0 +1,105 @@
+// Generated by Cursor
+// AI-assisted implementation with human review and modifications
+import { renderHook } from '@testing-library/react';
+import useAccordionShowdownExtension from '../accordion-extension';
+import { ACCORDION_MARKDOWN_BUTTON_ID, ACCORDION_MARKDOWN_CONTENT_ID } from '../const';
+import { marked } from 'marked';
+
+// Mock marked
+jest.mock('marked', () => ({
+  marked: {
+    parseInline: jest.fn((text) => `<em>${text}</em>`),
+  },
+}));
+
+// Mock DOMPurify
+jest.mock('dompurify', () => ({
+  sanitize: jest.fn((html) => html),
+}));
+
+describe('useAccordionShowdownExtension', () => {
+  beforeEach(() => {
+    jest.clearAllMocks();
+  });
+
+  it('should return a showdown extension with correct properties', () => {
+    const { result } = renderHook(() => useAccordionShowdownExtension());
+    const extension = result.current;
+
+    expect(extension.type).toBe('lang');
+    expect(extension.regex).toEqual(/\[(.+)]{{(accordion) (&quot;(.*?)&quot;)}}/g);
+    expect(typeof extension.replace).toBe('function');
+  });
+
+  it('should match accordion syntax with HTML-encoded quotes', () => {
+    const { result } = renderHook(() => useAccordionShowdownExtension());
+    const { regex } = result.current;
+
+    const testText = '[Some content]{{accordion &quot;My Title&quot;}}';
+    const matches = regex.exec(testText);
+
+    expect(matches).not.toBeNull();
+    if (matches) {
+      expect(matches[1]).toBe('Some content');
+      expect(matches[2]).toBe('accordion');
+      expect(matches[4]).toBe('My Title');
+    }
+  });
+
+  it('should not match accordion syntax with regular quotes', () => {
+    const { result } = renderHook(() => useAccordionShowdownExtension());
+    const { regex } = result.current;
+
+    const testText = '[Some content]{{accordion "My Title"}}';
+    expect(testText.match(regex)).toBeNull();
+  });
+
+  it('should generate correct accordion HTML structure', () => {
+    const { result } = renderHook(() => useAccordionShowdownExtension());
+    const { replace } = result.current;
+
+    const html = replace(
+      '[Test content]{{accordion &quot;Test Title&quot;}}',
+      'Test content',
+      'accordion',
+      '&quot;Test Title&quot;',
+      'Test Title',
+    );
+
+    expect(html).toContain('pf-v6-c-accordion');
+    expect(html).toContain('pf-v6-c-accordion__toggle');
+    expect(html).toContain(`${ACCORDION_MARKDOWN_BUTTON_ID}-Test-Title`);
+    expect(html).toContain(`${ACCORDION_MARKDOWN_CONTENT_ID}-Test-Title`);
+    expect(html).toContain('Test Title');
+  });
+
+  it('should process content through marked and sanitize HTML', () => {
+    const { result } = renderHook(() => useAccordionShowdownExtension());
+    const { replace } = result.current;
+
+    replace(
+      '[**Bold text**]{{accordion &quot;Title&quot;}}',
+      '**Bold text**',
+      'accordion',
+      '&quot;Title&quot;',
+      'Title',
+    );
+
+    expect(marked.parseInline).toHaveBeenCalledWith('**Bold text**');
+  });
+
+  it('should handle titles with spaces in IDs', () => {
+    const { result } = renderHook(() => useAccordionShowdownExtension());
+    const { replace } = result.current;
+
+    const html = replace(
+      '[Content]{{accordion &quot;My Test Title&quot;}}',
+      'Content',
+      'accordion',
+      '&quot;My Test Title&quot;',
+      'My Test Title',
+    );
+
+    expect(html).toContain(`${ACCORDION_MARKDOWN_BUTTON_ID}-My-Test-Title`);
+  });
+});

packages/module/src/ConsoleShared/src/components/markdown-extensions/__tests__/admonition-extension.spec.tsx

@@ -0,0 +1,121 @@
+import { renderHook } from '@testing-library/react';
+import useAdmonitionShowdownExtension from '../admonition-extension';
+import { marked } from 'marked';
+
+// Mock marked
+jest.mock('marked', () => ({
+  marked: {
+    parseInline: jest.fn((text) => `<strong>${text}</strong>`),
+  },
+}));
+
+// Mock DOMPurify
+jest.mock('dompurify', () => ({
+  sanitize: jest.fn((html) => html),
+}));
+
+describe('useAdmonitionShowdownExtension', () => {
+  beforeEach(() => {
+    jest.clearAllMocks();
+  });
+
+  it('should return a showdown extension with correct properties', () => {
+    const { result } = renderHook(() => useAdmonitionShowdownExtension());
+    const extension = result.current;
+
+    expect(extension.type).toBe('lang');
+    expect(extension.regex).toEqual(/\[(.+)]{{(admonition) ([\w-]+)}}/g);
+    expect(typeof extension.replace).toBe('function');
+  });
+
+  it('should match different admonition types', () => {
+    const { result } = renderHook(() => useAdmonitionShowdownExtension());
+    const { regex } = result.current;
+
+    const admonitionTypes = ['note', 'tip', 'important', 'warning', 'caution', 'custom-type'];
+
+    admonitionTypes.forEach((type) => {
+      const testText = `[Content for ${type}]{{admonition ${type}}}`;
+      // Reset regex state for global flag
+      regex.lastIndex = 0;
+      const matches = regex.exec(testText);
+
+      expect(matches).not.toBeNull();
+      if (matches) {
+        expect(matches[1]).toBe(`Content for ${type}`);
+        expect(matches[2]).toBe('admonition');
+        expect(matches[3]).toBe(type);
+      }
+    });
+  });
+
+  it('should not match malformed admonition syntax', () => {
+    const { result } = renderHook(() => useAdmonitionShowdownExtension());
+    const { regex } = result.current;
+
+    const malformedCases = [
+      'Content]{{admonition note}}',
+      '[Content{{admonition note}}',
+      '[Content]{{admonition}}',
+      '[Content]{{notadmonition note}}',
+    ];
+
+    malformedCases.forEach((testCase) => {
+      expect(testCase.match(regex)).toBeNull();
+    });
+  });
+
+  it('should generate correct alert HTML structure', () => {
+    const { result } = renderHook(() => useAdmonitionShowdownExtension());
+    const { replace } = result.current;
+
+    const html = replace('[Test message]{{admonition note}}', 'Test message', 'admonition', 'note');
+
+    expect(html).toContain('pf-v6-c-alert');
+    expect(html).toContain('pf-m-info'); // note maps to info variant
+    expect(html).toContain('pf-m-inline');
+    expect(html).toContain('pfext-markdown-admonition');
+    expect(html).toContain('NOTE'); // uppercase title
+  });
+
+  it('should handle different admonition types with correct variants', () => {
+    const { result } = renderHook(() => useAdmonitionShowdownExtension());
+    const { replace } = result.current;
+
+    const testCases = [
+      { type: 'note', expectedClass: 'pf-m-info' },
+      { type: 'warning', expectedClass: 'pf-m-warning' },
+      { type: 'important', expectedClass: 'pf-m-danger' },
+    ];
+
+    testCases.forEach(({ type, expectedClass }) => {
+      const html = replace(`[Content]{{admonition ${type}}}`, 'Content', 'admonition', type);
+
+      expect(html).toContain(expectedClass);
+      expect(html).toContain(type.toUpperCase());
+    });
+  });
+
+  it('should process content through marked', () => {
+    const { result } = renderHook(() => useAdmonitionShowdownExtension());
+    const { replace } = result.current;
+
+    replace('[**Bold text**]{{admonition note}}', '**Bold text**', 'admonition', 'note');
+
+    expect(marked.parseInline).toHaveBeenCalledWith('**Bold text**');
+  });
+
+  it('should return original text for invalid cases', () => {
+    const { result } = renderHook(() => useAdmonitionShowdownExtension());
+    const { replace } = result.current;
+
+    // Missing content
+    const originalText = '[Content]{{admonition note}}';
+    const result1 = replace(originalText, '', 'admonition', 'note');
+    expect(result1).toBe(originalText);
+
+    // Wrong command
+    const result2 = replace(originalText, 'Content', 'not-admonition', 'note');
+    expect(result2).toBe(originalText);
+  });
+});