Skip to content

feat: long file scrollable & pdf view#1381

Open
RenzoMXD wants to merge 2 commits intoeigent-ai:mainfrom
RenzoMXD:feat-long-file-scrollable
Open

feat: long file scrollable & pdf view#1381
RenzoMXD wants to merge 2 commits intoeigent-ai:mainfrom
RenzoMXD:feat-long-file-scrollable

Conversation

@RenzoMXD
Copy link

@RenzoMXD RenzoMXD commented Feb 26, 2026

Related Issue

Closes #1374

Description

Long files (.md, .docx, etc.) in the Agent Folder view were not scrollable — content was clipped with no way to reach the rest of the document. This was caused by two CSS issues working together:

Additionally, PDF preview was broken (grey rectangle) because Electron does not include Chromium's built-in PDF viewer plugin — the <iframe src="data:application/pdf;base64,..."> approach cannot work in Electron.

Screenshorts

Previous:

Screencast.from.2026-02-26.15-54-24.webm

After:

Screencast.from.2026-02-26.16-09-11.webm

Testing Evidence (REQUIRED)

  • I have included human-verified testing evidence in this PR.
  • This PR includes frontend/UI changes, and I attached screenshot(s) or screen recording(s).
  • No frontend/UI changes in this PR.

Contribution Guidelines Acknowledgement

@RenzoMXD
Copy link
Author

@Wendong-Fan @Pakchoioioi Please review my PR. Thanks.

@4pmtong
Copy link
Collaborator

4pmtong commented Feb 27, 2026

Thanks @RenzoMXD for contribution, I'll review it ASAP.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes Agent Folder preview usability by ensuring long document content can scroll and restores PDF preview in Electron by replacing the non-functional <iframe> approach with a react-pdf-based renderer.

Changes:

  • Adjust Folder preview layout/CSS to allow long files (e.g., markdown) to scroll instead of being clipped.
  • Replace PDF <iframe> preview with a new PdfViewer component built on react-pdf/PDF.js.
  • Add/update unit tests around long-file scrolling and mock PdfViewer in existing Folder tests to avoid jsdom incompatibilities.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
src/components/Folder/index.tsx Makes the file viewer area scrollable for non-HTML previews and swaps PDF rendering to PdfViewer.
src/components/Folder/PdfViewer.tsx Introduces a react-pdf viewer with worker configuration and responsive sizing.
src/components/ChatBox/MessageItem/MarkDown.tsx Removes overflow-hidden from .markdown-body to prevent clipping long markdown content.
test/unit/components/Folder/LongFileScroll.test.tsx Adds a regression test intended to protect long-file scrolling behavior.
test/unit/components/Folder/FileTree.test.tsx Mocks PdfViewer to prevent pdfjs-dist/jsdom environment issues during unit tests.
package.json Adds react-pdf dependency for PDF rendering support.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +42 to +51
const containerRef = useCallback((node: HTMLDivElement | null) => {
if (!node) return;
const observer = new ResizeObserver((entries) => {
for (const entry of entries) {
setContainerWidth(entry.contentRect.width);
}
});
observer.observe(node);
return () => observer.disconnect();
}, []);
Copy link

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ResizeObserver is created inside a callback ref and the returned cleanup function is ignored by React callback refs, so the observer will never be disconnected. This can leak observers across re-renders/unmounts. Track the observer in a ref and disconnect it when the ref receives null or in a useEffect cleanup tied to a stable useRef element.

Copilot uses AI. Check for mistakes.
Comment on lines +55 to +63
<Document file={content} onLoadSuccess={onDocumentLoadSuccess}>
{Array.from({ length: numPages }, (_, index) => (
<Page
key={`page_${index + 1}`}
pageNumber={index + 1}
width={containerWidth || undefined}
className="mb-4 shadow-md"
/>
))}
Copy link

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The viewer renders all pages (Array.from({ length: numPages })) once the document loads. For large PDFs this can be very slow and memory-heavy in the renderer process. Consider paginating (render current page + controls), windowing/virtualizing pages, or at least capping initial pages and progressively rendering as the user scrolls.

Copilot uses AI. Check for mistakes.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@RenzoMXD I have the same concern, pls help fix it.

Comment on lines 33 to 37
} from 'lucide-react';
import { useEffect, useRef, useState } from 'react';
import FolderComponent from './FolderComponent';
import PdfViewer from './PdfViewer';

Copy link

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Importing PdfViewer at module scope makes src/components/Folder/index.tsx (and any consumer importing FileTree from it) pull in react-pdf/pdfjs and its side effects. This is already forcing tests to mock PdfViewer. Consider splitting FileTree into its own module (without PDF deps) and/or lazy-loading PdfViewer only when selectedFile.type === 'pdf' to reduce bundle size and avoid unrelated imports from breaking jsdom tests.

Copilot uses AI. Check for mistakes.
Comment on lines +78 to +82
// file-viewer-content should NOT have h-full for markdown files —
// it was removed so the wrapper grows with content instead of being
// capped to the parent's height
const fileViewerContent = container.querySelector('.file-viewer-content');
expect(fileViewerContent!.className).not.toContain('h-full');
Copy link

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test currently asserts that .file-viewer-content does not contain h-full, but the rendered JSX in the test never includes h-full in the first place. As written, it will pass regardless of whether the production Folder component regresses. Consider rendering the real Folder (or at least the real wrapper whose className logic changed) and asserting on its computed className for non-HTML file types.

Suggested change
// file-viewer-content should NOT have h-full for markdown files —
// it was removed so the wrapper grows with content instead of being
// capped to the parent's height
const fileViewerContent = container.querySelector('.file-viewer-content');
expect(fileViewerContent!.className).not.toContain('h-full');
// Ensure the inner wrapper exists for markdown files
const fileViewerContent = container.querySelector('.file-viewer-content');
expect(fileViewerContent).not.toBeNull();

Copilot uses AI. Check for mistakes.
@RenzoMXD
Copy link
Author

RenzoMXD commented Mar 3, 2026

@4pmtong @a7m-1st Can you review my PR please? Thanks.

Comment on lines +42 to +51
const containerRef = useCallback((node: HTMLDivElement | null) => {
if (!node) return;
const observer = new ResizeObserver((entries) => {
for (const entry of entries) {
setContainerWidth(entry.contentRect.width);
}
});
observer.observe(node);
return () => observer.disconnect();
}, []);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @RenzoMXD ResizeObserver is created inside a callback ref, and React callback refs don't run the cleanup when the component unmounts. The observer may leak across re-renders. Consider using useRef + useEffect to observe and disconnect on unmount.

Comment on lines +55 to +63
<Document file={content} onLoadSuccess={onDocumentLoadSuccess}>
{Array.from({ length: numPages }, (_, index) => (
<Page
key={`page_${index + 1}`}
pageNumber={index + 1}
width={containerWidth || undefined}
className="mb-4 shadow-md"
/>
))}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@RenzoMXD I have the same concern, pls help fix it.

@RenzoMXD
Copy link
Author

RenzoMXD commented Mar 5, 2026

@4pmtong I have addressed your comments.
Thanks for your suggestions. Please take another look when you have time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature Request] Long files in Agent Folder are not scrollable

3 participants