fix: address file preview review feedback

Author: ryota-murakami

src/renderer/src/components/SkipToMainContentLink.tsx

@@ -38,7 +38,7 @@ export const SkipToMainContentLink = memo(
         href="#main-content"
         className="sr-only focus:not-sr-only focus:fixed focus:top-0 focus:left-0 focus:z-50 focus:bg-primary focus:text-primary-foreground focus:p-2"
       >
-        [Press Enter] Skip focus to main content
+        Skip to main content
       </a>
     )
   },

src/renderer/src/components/skills/FileContent.browser.test.tsx

@@ -67,6 +67,73 @@ describe('FileContent Markdown modes', () => {
     await expect.element(screen.getByText('Visible body')).toBeInTheDocument()
   })
 
+  it('renders language-less code fences as block code without AST attributes', async () => {
+    const { FileContent } = await import('./FileContent')
+    const screen = await render(
+      <FileContent
+        content={makeTextContent({
+          content:
+            '# Skill\n\n```\nnpx skills list --json\n```\n\nInline `skill` stays compact.',
+        })}
+      />,
+    )
+
+    await screen.getByRole('radio', { name: /Show rendered Markdown/i }).click()
+
+    const blockCode = screen.getByText(/npx skills list --json/).query()
+    const inlineCode = screen.getByText('skill', { exact: true }).query()
+
+    expect(blockCode).toBeInstanceOf(HTMLElement)
+    expect(inlineCode).toBeInstanceOf(HTMLElement)
+    expect(blockCode?.closest('pre')).toBeInstanceOf(HTMLPreElement)
+    expect(inlineCode?.closest('pre')).toBeNull()
+    expect(screen.container.querySelector('[node]')).toBeNull()
+  })
+
+  it('renders language-tagged code fences as block code', async () => {
+    const { FileContent } = await import('./FileContent')
+    const screen = await render(
+      <FileContent
+        content={makeTextContent({
+          content: '# Skill\n\n```ts\nconst ok = true\n```',
+        })}
+      />,
+    )
+
+    await screen.getByRole('radio', { name: /Show rendered Markdown/i }).click()
+
+    const blockCode = screen.getByText(/const ok = true/).query()
+
+    expect(blockCode).toBeInstanceOf(HTMLElement)
+    expect(blockCode?.closest('pre')).toBeInstanceOf(HTMLPreElement)
+  })
+
+  it('locks Reading Mode to vertical scrolling when Markdown is wider than the pane', async () => {
+    const { FileContent } = await import('./FileContent')
+    const wideInline = 'very-long-inline-token-'.repeat(30)
+    const wideBlock = 'wide command '.repeat(40)
+    const screen = await render(
+      <FileContent
+        content={makeTextContent({
+          content: `# Wide\n\n\`${wideInline}\`\n\n\`\`\`\n${wideBlock}\n\`\`\``,
+        })}
+      />,
+    )
+
+    await screen.getByRole('radio', { name: /Show rendered Markdown/i }).click()
+
+    const scrollPane = screen.container.querySelector(
+      '[data-markdown-reading-scroll]',
+    )
+    expect(scrollPane).toBeInstanceOf(HTMLElement)
+    const pane = scrollPane as HTMLElement
+
+    // Programmatic scroll mirrors trackpad horizontal gestures in the renderer.
+    expect(pane.scrollWidth).toBeGreaterThan(pane.clientWidth)
+    pane.scrollTo({ left: 240 })
+    expect(pane.scrollLeft).toBe(0)
+  })
+
   it('adds a bottom spacer after source code so the final line can breathe', async () => {
     const { FileContent } = await import('./FileContent')
     const screen = await render(

src/renderer/src/components/skills/FileContent.tsx

@@ -71,11 +71,12 @@ const TextPreview = React.memo(function TextPreview({
   file,
 }: TextPreviewProps): React.ReactElement {
   const isMarkdown = isMarkdownPreview(file)
+  const fileIdentity = `${file.name}:${file.extension}`
   const [mode, setMode] = useState<TextPreviewMode>('code')
 
   useEffect(() => {
     setMode('code')
-  }, [file.name, file.extension])
+  }, [fileIdentity])
 
   const handleModeChange = (next: string): void => {
     // Radix emits an empty string when the active item is clicked again.
@@ -249,8 +250,11 @@ const MarkdownReadingPreview = React.memo(function MarkdownReadingPreview({
   )
 
   return (
-    <div className="flex-1 min-h-0 overflow-auto bg-background">
-      <article className="max-w-none px-7 py-6 pb-10 text-sm leading-7 text-foreground">
+    <div
+      className="flex-1 min-h-0 overflow-y-auto overflow-x-hidden bg-background"
+      data-markdown-reading-scroll
+    >
+      <article className="min-w-0 max-w-full break-words px-7 py-6 pb-10 text-sm leading-7 text-foreground">
         <ReactMarkdown
           remarkPlugins={[remarkGfm]}
           components={markdownComponents}
@@ -298,11 +302,49 @@ function looksLikeYamlFrontmatter(rawBlock: string): boolean {
   })
 }
 
+/**
+ * Remove react-markdown's internal AST prop before DOM spreading.
+ * @param props - Component override props from react-markdown.
+ * @returns The same props without the non-DOM `node` field.
+ * @example
+ * omitMarkdownNode({ node: astNode, href: '/docs' }) // => { href: '/docs' }
+ */
+function omitMarkdownNode<Props extends { node?: unknown }>(
+  props: Props,
+): Omit<Props, 'node'> {
+  const { node, ...domProps } = props
+  // `node` is useful for custom renderers, but invalid on real DOM elements.
+  void node
+  return domProps
+}
+
+/**
+ * Detect code blocks produced by react-markdown.
+ * @param children - Rendered code text from the Markdown AST.
+ * @param className - Optional `language-*` class from a fenced code info string.
+ * @returns True for fenced/indented code blocks, including fences with no language.
+ * @example
+ * isMarkdownCodeBlock('pnpm validate\n') // => true
+ */
+function isMarkdownCodeBlock(
+  children: React.ReactNode,
+  className?: string,
+): boolean {
+  // Language-tagged fences are always block code.
+  if (className?.includes('language-')) return true
+
+  // Guard the react-markdown v10 newline heuristic with browser tests: block
+  // code keeps a trailing newline, while inline code does not.
+  return typeof children === 'string' && children.endsWith('\n')
+}
+
 const markdownComponents: Components = {
   a({ children, className, href, ...props }) {
+    const domProps = omitMarkdownNode(props)
+
     return (
       <a
-        {...props}
+        {...domProps}
         href={href}
         target="_blank"
         rel="noreferrer"
@@ -316,9 +358,11 @@ const markdownComponents: Components = {
     )
   },
   blockquote({ children, className, ...props }) {
+    const domProps = omitMarkdownNode(props)
+
     return (
       <blockquote
-        {...props}
+        {...domProps}
         className={cn(
           'my-4 border-l-2 border-primary/60 pl-4 text-muted-foreground',
           className,
@@ -329,11 +373,12 @@ const markdownComponents: Components = {
     )
   },
   code({ children, className, ...props }) {
-    const isBlock = className?.includes('language-') ?? false
-    if (isBlock) {
+    const domProps = omitMarkdownNode(props)
+
+    if (isMarkdownCodeBlock(children, className)) {
       return (
         <code
-          {...props}
+          {...domProps}
           className={cn(
             'block overflow-x-auto rounded-md border border-border bg-muted px-3 py-2 font-mono text-xs leading-6 text-foreground',
             className,
@@ -346,7 +391,7 @@ const markdownComponents: Components = {
 
     return (
       <code
-        {...props}
+        {...domProps}
         className={cn(
           'rounded bg-muted px-1.5 py-0.5 font-mono text-[0.85em] text-foreground',
           className,
@@ -357,9 +402,11 @@ const markdownComponents: Components = {
     )
   },
   h1({ children, className, ...props }) {
+    const domProps = omitMarkdownNode(props)
+
     return (
       <h1
-        {...props}
+        {...domProps}
         className={cn(
           'mb-4 mt-0 border-b border-border pb-3 text-2xl font-semibold leading-tight',
           className,
@@ -370,9 +417,11 @@ const markdownComponents: Components = {
     )
   },
   h2({ children, className, ...props }) {
+    const domProps = omitMarkdownNode(props)
+
     return (
       <h2
-        {...props}
+        {...domProps}
         className={cn(
           'mb-3 mt-7 border-b border-border/70 pb-2 text-xl font-semibold leading-tight',
           className,
@@ -383,48 +432,60 @@ const markdownComponents: Components = {
     )
   },
   h3({ children, className, ...props }) {
+    const domProps = omitMarkdownNode(props)
+
     return (
       <h3
-        {...props}
+        {...domProps}
         className={cn('mb-2 mt-6 text-base font-semibold', className)}
       >
         {children}
       </h3>
     )
   },
   li({ children, className, ...props }) {
+    const domProps = omitMarkdownNode(props)
+
     return (
-      <li {...props} className={cn('my-1 pl-1', className)}>
+      <li {...domProps} className={cn('my-1 pl-1', className)}>
         {children}
       </li>
     )
   },
   ol({ children, className, ...props }) {
+    const domProps = omitMarkdownNode(props)
+
     return (
-      <ol {...props} className={cn('my-4 list-decimal pl-6', className)}>
+      <ol {...domProps} className={cn('my-4 list-decimal pl-6', className)}>
         {children}
       </ol>
     )
   },
   p({ children, className, ...props }) {
+    const domProps = omitMarkdownNode(props)
+
     return (
-      <p {...props} className={cn('my-3', className)}>
+      <p {...domProps} className={cn('my-3', className)}>
         {children}
       </p>
     )
   },
   pre({ children, className, ...props }) {
+    const domProps = omitMarkdownNode(props)
+
     return (
-      <pre {...props} className={cn('my-4', className)}>
+      <pre {...domProps} className={cn('my-4', className)}>
         {children}
       </pre>
     )
   },
   table({ children, className, ...props }) {
+    const domProps = omitMarkdownNode(props)
+
     return (
       <div className="my-4 overflow-x-auto rounded-md border border-border">
         <table
-          {...props}
+          {...domProps}
           className={cn('w-full border-collapse text-left text-sm', className)}
         >
           {children}
@@ -433,19 +494,23 @@ const markdownComponents: Components = {
     )
   },
   td({ children, className, ...props }) {
+    const domProps = omitMarkdownNode(props)
+
     return (
       <td
-        {...props}
+        {...domProps}
         className={cn('border-t border-border px-3 py-2 align-top', className)}
       >
         {children}
       </td>
     )
   },
   th({ children, className, ...props }) {
+    const domProps = omitMarkdownNode(props)
+
     return (
       <th
-        {...props}
+        {...domProps}
         className={cn(
           'border-b border-border bg-muted px-3 py-2 font-semibold',
           className,
@@ -456,8 +521,10 @@ const markdownComponents: Components = {
     )
   },
   ul({ children, className, ...props }) {
+    const domProps = omitMarkdownNode(props)
+
     return (
-      <ul {...props} className={cn('my-4 list-disc pl-6', className)}>
+      <ul {...domProps} className={cn('my-4 list-disc pl-6', className)}>
         {children}
       </ul>
     )

src/renderer/src/components/skills/filePreviewLanguage.test.ts

@@ -10,8 +10,29 @@ describe('filePreviewLanguage', () => {
   it('maps common skill file extensions to Shiki language ids', () => {
     expect(languageFromExtension('.ts')).toBe('typescript')
     expect(languageFromExtension('.tsx')).toBe('tsx')
+    expect(languageFromExtension('.TS')).toBe('typescript')
+    expect(languageFromExtension('.TSX')).toBe('tsx')
+    expect(languageFromExtension('ts')).toBe('typescript')
     expect(languageFromExtension('.env.example')).toBe('dotenv')
     expect(languageFromExtension('.svg')).toBe('xml')
+    expect(languageFromExtension('')).toBeUndefined()
+    expect(languageFromExtension(null)).toBeUndefined()
+    expect(languageFromExtension(undefined)).toBeUndefined()
+  })
+
+  it('maps known file extensions to correct Shiki language IDs for preview', () => {
+    expect(languageForPreview({ name: 'file.ts', extension: '.ts' })).toBe(
+      'typescript',
+    )
+    expect(languageForPreview({ name: 'file.tsx', extension: '.tsx' })).toBe(
+      'tsx',
+    )
+    expect(languageForPreview({ name: 'file.js', extension: '.js' })).toBe(
+      'javascript',
+    )
+    expect(languageForPreview({ name: 'file.json', extension: '.json' })).toBe(
+      'json',
+    )
   })
 
   it('falls back to safe text highlighting for unknown extensions', () => {
@@ -25,8 +46,26 @@ describe('filePreviewLanguage', () => {
 
   it('detects Markdown files for Reading Mode', () => {
     expect(isMarkdownPreview({ name: 'SKILL.md', extension: '.md' })).toBe(true)
+    expect(
+      isMarkdownPreview({ name: 'README.markdown', extension: '.markdown' }),
+    ).toBe(true)
+    expect(
+      isMarkdownPreview({ name: 'README.mdown', extension: '.mdown' }),
+    ).toBe(true)
+    expect(isMarkdownPreview({ name: 'SKILL.MD', extension: '.MD' })).toBe(true)
+    expect(isMarkdownPreview({ name: 'Skill.Md', extension: '.Md' })).toBe(true)
+    expect(isMarkdownPreview({ name: 'SKILL.md', extension: 'md' })).toBe(true)
+    expect(isMarkdownPreview({ name: 'README', extension: '' })).toBe(true)
     expect(
       isMarkdownPreview({ name: 'component.tsx', extension: '.tsx' }),
     ).toBe(false)
+    expect(
+      isMarkdownPreview({ name: 'readme.md.txt', extension: '.txt' }),
+    ).toBe(false)
+    expect(isMarkdownPreview({ name: 'notes', extension: '' })).toBe(false)
+    expect(isMarkdownPreview({ name: 'notes', extension: null })).toBe(false)
+    expect(isMarkdownPreview({ name: 'notes', extension: undefined })).toBe(
+      false,
+    )
   })
 })