-
Notifications
You must be signed in to change notification settings - Fork 5.5k
fix(web): folder links from chat open the file tree instead of a broken preview #10909
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
70ce3bb
677671b
f50cb0d
829bdc9
3eeb991
8c99ee6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -995,12 +995,20 @@ export default function FilePreviewPanel({ | |
| relativePath, | ||
| attachment === undefined && !isMedia && !isPdf, | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| ); | ||
| // A chat link cannot tell a folder from a file, so a folder arrives here as | ||
| // a file surface and the read fails. Keep the breadcrumbs, drop the preview | ||
| // pane, and let the tree fill the surface with the folder revealed. Mutation | ||
| // refresh stays on so the surface notices if the path becomes a file. A host | ||
| // path cannot be revealed in the workspace tree, so it keeps the read error. | ||
| const isDirectory = file.isNotFile && !isHostFile; | ||
| const [explorerOpen, setExplorerOpen] = useState(initialExplorerOpen); | ||
| const showExplorer = shouldShowFileExplorer({ | ||
| relativePath, | ||
| explorerOpen, | ||
| attachmentOpen: attachment !== undefined, | ||
| }); | ||
| const showExplorer = | ||
|
macroscopeapp[bot] marked this conversation as resolved.
|
||
| isDirectory || | ||
| shouldShowFileExplorer({ | ||
| relativePath, | ||
| explorerOpen, | ||
| attachmentOpen: attachment !== undefined, | ||
| }); | ||
| // Reading markdown rendered is a preference, not a property of one file. Keeping | ||
| // it on the panel meant a thread switch dropped it and forced source back. | ||
| const [renderMarkdownPreferred, setRenderMarkdownPreferred] = useLocalStorage( | ||
|
|
@@ -1027,12 +1035,13 @@ export default function FilePreviewPanel({ | |
| (handledReveal?.path === relativePath && handledReveal.requestId === revealRequestId); | ||
| const renderMarkdown = isMarkdown && renderMarkdownPreferred && revealHandled; | ||
| const renderBrowserFile = isPdf || (isHtml && renderBrowserFilePreferred && revealHandled); | ||
| const canToggleRendered = attachment === undefined && (isMarkdown || isHtml); | ||
| const canToggleRendered = !isDirectory && attachment === undefined && (isMarkdown || isHtml); | ||
| const rendered = isMarkdown ? renderMarkdown : renderBrowserFile; | ||
| const setRenderedPreferred = isMarkdown | ||
| ? setRenderMarkdownPreferred | ||
| : setRenderBrowserFilePreferred; | ||
| const canOpenInBrowser = | ||
| !isDirectory && | ||
| relativePath !== null && | ||
| attachment === undefined && | ||
| !isVideo && | ||
|
|
@@ -1190,7 +1199,7 @@ export default function FilePreviewPanel({ | |
| <TooltipPopup>Open file in preview browser</TooltipPopup> | ||
| </Tooltip> | ||
| ) : null} | ||
| {!isHostFile ? ( | ||
| {!isHostFile && !isDirectory ? ( | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| <Tooltip> | ||
| <TooltipTrigger | ||
| render={ | ||
|
|
@@ -1222,7 +1231,7 @@ export default function FilePreviewPanel({ | |
| <div | ||
| className={cn( | ||
| "min-w-0 flex-1 flex-col overflow-hidden", | ||
| relativePath ? "flex" : "hidden", | ||
| relativePath && !isDirectory ? "flex" : "hidden", | ||
| )} | ||
| > | ||
| {relativePath && attachment ? ( | ||
|
|
@@ -1332,7 +1341,7 @@ export default function FilePreviewPanel({ | |
| <aside | ||
| className={cn( | ||
| "flex min-h-0 shrink-0 bg-background", | ||
| relativePath | ||
| relativePath && !isDirectory | ||
| ? "w-[min(22rem,46%)] min-w-64 border-l border-border/60" | ||
| : "min-w-0 flex-1", | ||
| )} | ||
|
|
@@ -1346,7 +1355,7 @@ export default function FilePreviewPanel({ | |
| selectedPathRevealId={revealRequestId} | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Medium A directory link ending in 🤖 Copy this AI Prompt to have your agent fix this:
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🤖 agent replyposted by an ai agent in adam's name. model: Claude Fable 5.1. harness: Claude Code. adam hasn't read this before it went out. Done in 419f555. The store strips a trailing slash when opening a file surface, so
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry, I'm unable to act on this request because you do not have permissions within this repository.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry, I'm unable to act on this request because you do not have permissions within this repository. |
||
| onOpenFile={onOpenFile} | ||
| workspaceMutationId={workspaceMutationId} | ||
| {...(relativePath && !isMedia && !isPdf | ||
| {...(relativePath && !isMedia && !isPdf && !isDirectory | ||
| ? { onRefreshSelectedFile: file.refresh } | ||
| : {})} | ||
| /> | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟡 Medium
files/FilePreviewPanel.tsx:996Directory links whose names end in
.pdfor a media extension are still rendered as file previews instead of showing the folder tree. TheisPdf/isMediaguards preventuseProjectFileQueryfrom producing thepath_not_fileresult that setsfile.isNotFile; allow a path-type check for these extensions (or use a separate directory query) soisDirectoryis detected before selectingWorkspaceBrowserPreviewor a media preview.🤖 Copy this AI Prompt to have your agent fix this:
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤖 agent reply
posted by an ai agent in adam's name. model: Claude Fable 5.1. harness: Claude Code. adam hasn't read this before it went out.
FABLE WROTE THAT NO IDEA WHY IT SKIPPED THE "IM AN AGENT" part of my pr skill.
Leaving this one. A folder named like
report.pdforimages.pngskipped the read before this PR as well, so the broken preview there is pre-existing and unchanged here. Handling it means a stat round trip for every media path just to catch a folder with a file-like name, which is not worth it for this fix.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I'm unable to act on this request because you do not have permissions within this repository.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I'm unable to act on this request because you do not have permissions within this repository.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I'm unable to act on this request because you do not have permissions within this repository.