feat: added workspace file browser #59
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Changes requested ❌
Reviewed everything up to 9ab203f in 1 minute and 27 seconds. Click for details.
- Reviewed
480lines of code in7files - Skipped
0files when reviewing. - Skipped posting
3draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. src/components/workspace/FilePreview.tsx:103
- Draft comment:
Default preview type returns null. Consider providing a fallback UI for unsupported file types to improve user experience. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% The code already handles the main file types (text, image, binary). The default case returning null is likely intentional as a safeguard. The preview.type should always be one of these types as it comes from the API. If an unknown type comes through, it's probably an error condition that should be handled by the error state rather than a fallback UI. I might be wrong about the API contract - there could be legitimate file types that aren't covered by these three cases. Also, from a UX perspective, showing something is generally better than showing nothing. However, if there were other valid file types, they should be handled explicitly rather than with a catch-all message. The null case is likely intentional as a development safeguard. The comment should be deleted as it suggests handling an edge case that shouldn't occur in practice, and if it did occur, should be handled through proper error handling rather than a catch-all UI message.
2. src/components/RightSidebar.tsx:35
- Draft comment:
The nested ternary for sidebar width is complex; consider refactoring it into a helper function or using clearer conditional logic for better readability. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
3. src/components/RightSidebar.tsx:68
- Draft comment:
Typo: Consider adding spaces in the CSS calc expression for clarity and correct CSS syntax. For example, change "calc(100%-3rem)" to "calc(100% - 3rem)". - Reason this comment was not posted:
Comment was on unchanged code.
Workflow ID: wflow_kchUvLsvdIcAH0BF
You can customize 
| if (contentType?.startsWith('image/')) { | ||
| return { | ||
| type: 'image', | ||
| content: URL.createObjectURL(await response.blob()), |
There was a problem hiding this comment.
Image preview: URL.createObjectURL is not revoked. Consider revoking the URL when it's no longer needed to avoid memory leaks.
ErikBjare
force-pushed
the
dev/workspace-file-browser
branch
from
9ab203f to
d32da16
Compare
There was a problem hiding this comment.
Caution
Changes requested ❌
Reviewed d32da16 in 2 minutes and 0 seconds. Click for details.
- Reviewed
482lines of code in7files - Skipped
0files when reviewing. - Skipped posting
4draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. src/components/RightSidebar.tsx:29
- Draft comment:
Consider using enum/constants for tab names instead of string literals to avoid typos. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
2. src/components/workspace/FileList.tsx:51
- Draft comment:
Memoize or pre-calculate date conversion for file.modified to improve performance for large lists. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
3. src/components/workspace/FilePreview.tsx:80
- Draft comment:
When rendering an image preview, consider revoking the blob URL (e.g. via URL.revokeObjectURL) to prevent memory leaks. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% While revoking blob URLs is generally good practice, we don't have evidence that blob URLs are being used here. The preview.content could be any type of URL or image data. Making assumptions about the implementation without seeing it could lead to unnecessary or incorrect changes. The comment is speculative without more context. The comment could be correct if preview.content is indeed a blob URL. Memory leaks from unreleased blob URLs are a real issue. Without seeing the implementation of previewFile or knowing the type of preview.content, we can't be certain blob URLs are used, making this comment speculative. Delete this comment as it makes assumptions about the implementation that we can't verify from the available code.
4. src/components/workspace/WorkspaceExplorer.tsx:24
- Draft comment:
The file list is re-fetched on every change (currentPath/showHidden). Consider debouncing API calls if rapid UI interactions occur. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
Workflow ID: wflow_qmeqX1g3qJnHa2ig
You can customize
| showHidden = false | ||
| ): Promise<FileType[]> { | ||
| const url = new URL( | ||
| `/api/v2/conversations/${conversationId}/workspace${path ? `/${path}` : ''}`, |
There was a problem hiding this comment.
Encode the path parameter using encodeURIComponent to safely handle special characters in the URL.
Suggested change
| `/api/v2/conversations/${conversationId}/workspace${path ? `/${path}` : ''}`, | |
| `/api/v2/conversations/${conversationId}/workspace${path ? `/${encodeURIComponent(path)}` : ''}`, |
| if (contentType?.startsWith('image/')) { | ||
| return { | ||
| type: 'image', | ||
| content: URL.createObjectURL(await response.blob()), |
There was a problem hiding this comment.
The blob URL created for image previews is not revoked. Ensure you clean it up (e.g., via URL.revokeObjectURL) when it’s no longer needed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Depends on: gptme/gptme#530
Important
Adds a workspace file browser with navigation and preview capabilities to the application.
WorkspaceExplorercomponent toRightSidebar.tsxfor browsing workspace files.FileList,FilePreview, andPathSegmentscomponents inworkspacedirectory for file navigation and preview.useWorkspaceApiinworkspaceApi.tsfor API interactions to list and preview files.RightSidebar.tsxwithFolderOpenicon.FileListdisplays files and directories, supports navigation to parent directory.FilePreviewsupports text, image, and binary file previews.PathSegmentsallows navigation through directory path segments.FileType,TextPreview,BinaryPreview,ImagePreview, andFilePreviewinworkspace.ts.This description was created by
for d32da16. You can customize this summary. It will automatically update as commits are pushed.