Skip to content

Fix diffs having multiple scrollbars #611

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.

Sign up for GitHub

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

Merged
kylecarbs merged 1 commit into from
Nov 15, 2025
Merged

Fix diffs having multiple scrollbars #611

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 75 additions & 6 deletions src/App.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -611,29 +611,98 @@ export const ActiveWorkspaceWithChat: Story = {
},
});

// Assistant message with file edit
// Assistant message with file edit (large diff)
callback({
id: "msg-4",
role: "assistant",
parts: [
{
type: "text",
text: "I'll add JWT token validation to the endpoint. Let me update the file.",
text: "I'll add JWT token validation to the endpoint. Let me update the file with proper authentication middleware and error handling.",
},
{
type: "dynamic-tool",
toolCallId: "call-2",
toolName: "search_replace",
toolName: "file_edit_replace_string",
state: "output-available",
input: {
file_path: "src/api/users.ts",
old_string: "export function getUser(req, res) {",
old_string:
"import express from 'express';\nimport { db } from '../db';\n\nexport function getUser(req, res) {\n const user = db.users.find(req.params.id);\n res.json(user);\n}",
new_string:
"import { verifyToken } from '../auth/jwt';\n\nexport function getUser(req, res) {\n const token = req.headers.authorization?.split(' ')[1];\n if (!token || !verifyToken(token)) {\n return res.status(401).json({ error: 'Unauthorized' });\n }",
"import express from 'express';\nimport { db } from '../db';\nimport { verifyToken } from '../auth/jwt';\nimport { logger } from '../utils/logger';\n\nexport async function getUser(req, res) {\n try {\n const token = req.headers.authorization?.split(' ')[1];\n if (!token) {\n logger.warn('Missing authorization token');\n return res.status(401).json({ error: 'Unauthorized' });\n }\n const decoded = await verifyToken(token);\n const user = await db.users.find(req.params.id);\n res.json(user);\n } catch (err) {\n logger.error('Auth error:', err);\n return res.status(401).json({ error: 'Invalid token' });\n }\n}",
},
output: {
success: true,
message: "File updated successfully",
diff: [
"--- src/api/users.ts",
"+++ src/api/users.ts",
"@@ -2,0 +3,2 @@",
"+import { verifyToken } from '../auth/jwt';",
"+import { logger } from '../utils/logger';",
"@@ -4,28 +6,14 @@",
"-// TODO: Add authentication middleware",
"-// Current implementation is insecure and allows unauthorized access",
"-// Need to validate JWT tokens before processing requests",
"-// Also need to add rate limiting to prevent abuse",
"-// Consider adding request logging for audit trail",
"-// Add input validation for user IDs",
"-// Handle edge cases for deleted/suspended users",
"-",
"-/**",
"- * Get user by ID",
"- * @param {Object} req - Express request object",
"- * @param {Object} res - Express response object",
"- */",
"-export function getUser(req, res) {",
"- // FIXME: No authentication check",
"- // FIXME: No error handling",
"- // FIXME: Synchronous database call blocks event loop",
"- // FIXME: No input validation",
"- // FIXME: Direct database access without repository pattern",
"- // FIXME: No logging",
"-",
"- const user = db.users.find(req.params.id);",
"-",
"- // TODO: Check if user exists",
"- // TODO: Filter sensitive fields (password hash, etc)",
"- // TODO: Check permissions - user should only access their own data",
"-",
"- res.json(user);",
"+export async function getUser(req, res) {",
"+ try {",
"+ const token = req.headers.authorization?.split(' ')[1];",
"+ if (!token) {",
"+ logger.warn('Missing authorization token');",
"+ return res.status(401).json({ error: 'Unauthorized' });",
"+ }",
"+ const decoded = await verifyToken(token);",
"+ const user = await db.users.find(req.params.id);",
"+ res.json(user);",
"+ } catch (err) {",
"+ logger.error('Auth error:', err);",
"+ return res.status(401).json({ error: 'Invalid token' });",
"+ }",
"@@ -34,3 +22,2 @@",
"-// TODO: Add updateUser function",
"-// TODO: Add deleteUser function",
"-// TODO: Add listUsers function with pagination",
"+// Note: updateUser, deleteUser, and listUsers endpoints will be added in separate PR",
"+// to keep changes focused and reviewable",
"@@ -41,0 +29,11 @@",
"+",
"+export async function rotateApiKey(req, res) {",
"+ const admin = await db.admins.find(req.user.id);",
"+ if (!admin) {",
"+ return res.status(403).json({ error: 'Forbidden' });",
"+ }",
"+",
"+ const apiKey = await db.tokens.rotate(admin.orgId);",
"+ logger.info('Rotated API key', { orgId: admin.orgId });",
"+ res.json({ apiKey });",
"+}",
].join("\n"),
edits_applied: 1,
},
},
],
Expand Down
124 changes: 72 additions & 52 deletions src/components/shared/DiffRenderer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,20 +72,72 @@ const getContrastColor = (type: DiffLineType): string => {
export const DiffContainer: React.FC<
React.PropsWithChildren<{ fontSize?: string; maxHeight?: string; className?: string }>
> = ({ children, fontSize, maxHeight, className }) => {
const resolvedMaxHeight = maxHeight ?? "400px";
const [isExpanded, setIsExpanded] = React.useState(false);
const contentRef = React.useRef<HTMLDivElement>(null);
const [isOverflowing, setIsOverflowing] = React.useState(false);
const clampContent = resolvedMaxHeight !== "none" && !isExpanded;

React.useEffect(() => {
if (maxHeight === "none") {
setIsExpanded(false);
}
}, [maxHeight]);

React.useEffect(() => {
const element = contentRef.current;
if (!element) {
return;
}

const updateOverflowState = () => {
setIsOverflowing(element.scrollHeight > element.clientHeight + 1);
};

updateOverflowState();

let resizeObserver: ResizeObserver | null = null;
if (typeof ResizeObserver !== "undefined") {
resizeObserver = new ResizeObserver(updateOverflowState);
resizeObserver.observe(element);
}

return () => {
resizeObserver?.disconnect();
};
}, [resolvedMaxHeight, clampContent]);

return (
<div
className={cn(
"m-0 py-1.5 bg-code-bg rounded grid overflow-y-auto overflow-x-auto [&_*]:text-[inherit]",
className
<div className={cn("relative m-0 rounded bg-code-bg py-1.5 [&_*]:text-[inherit]", className)}>
<div
ref={contentRef}
className={cn(
"grid overflow-x-auto",
clampContent ? "pb-6 overflow-y-hidden" : "overflow-y-visible"
)}
style={{
fontSize: fontSize ?? "12px",
lineHeight: 1.4,
maxHeight: clampContent ? resolvedMaxHeight : undefined,
gridTemplateColumns: "minmax(min-content, 1fr)",
}}
>
{children}
</div>

{clampContent && isOverflowing && (
<>
<div className="via-[color-mix(in srgb, var(--color-code-bg) 80%, transparent)] pointer-events-none absolute inset-x-0 bottom-0 h-10 bg-gradient-to-t from-[var(--color-code-bg)] to-transparent" />
<div className="absolute inset-x-0 bottom-0 flex justify-center pb-1.5">
<button
className="bg-dark/60 text-foreground/80 hover:text-foreground border border-white/20 px-2 py-0.5 text-[10px] tracking-wide uppercase backdrop-blur transition hover:border-white/40"
onClick={() => setIsExpanded(true)}
>
Expand diff
</button>
</div>
</>
)}
style={{
fontSize: fontSize ?? "12px",
lineHeight: 1.4,
maxHeight: maxHeight ?? "400px",
gridTemplateColumns: "minmax(min-content, 1fr)",
}}
>
{children}
</div>
);
};
Expand Down Expand Up @@ -192,30 +244,14 @@ export const DiffRenderer: React.FC<DiffRendererProps> = ({
// Show loading state while highlighting
if (!highlightedChunks) {
return (
<div
className="bg-code-bg m-0 grid overflow-auto rounded py-1.5 [&_*]:text-[inherit]"
style={{
fontSize: fontSize ?? "12px",
lineHeight: 1.4,
maxHeight: maxHeight ?? "400px",
gridTemplateColumns: "minmax(min-content, 1fr)",
}}
>
<DiffContainer fontSize={fontSize} maxHeight={maxHeight}>
<div style={{ opacity: 0.5, padding: "8px" }}>Processing...</div>
</div>
</DiffContainer>
);
}

return (
<div
className="bg-code-bg m-0 grid overflow-auto rounded py-1.5 [&_*]:text-[inherit]"
style={{
fontSize: fontSize ?? "12px",
lineHeight: 1.4,
maxHeight: maxHeight ?? "400px",
gridTemplateColumns: "minmax(min-content, 1fr)",
}}
>
<DiffContainer fontSize={fontSize} maxHeight={maxHeight}>
{highlightedChunks.flatMap((chunk) =>
chunk.lines.map((line) => {
const indicator = chunk.type === "add" ? "+" : chunk.type === "remove" ? "-" : " ";
Expand Down Expand Up @@ -260,7 +296,7 @@ export const DiffRenderer: React.FC<DiffRendererProps> = ({
);
})
)}
</div>
</DiffContainer>
);
};

Expand Down Expand Up @@ -502,33 +538,17 @@ export const SelectableDiffRenderer = React.memo<SelectableDiffRendererProps>(
// Show loading state while highlighting
if (!highlightedChunks || highlightedLineData.length === 0) {
return (
<div
className="bg-code-bg m-0 grid overflow-auto rounded py-1.5 [&_*]:text-[inherit]"
style={{
fontSize: fontSize ?? "12px",
lineHeight: 1.4,
maxHeight: maxHeight ?? "400px",
gridTemplateColumns: "minmax(min-content, 1fr)",
}}
>
<DiffContainer fontSize={fontSize} maxHeight={maxHeight}>
<div style={{ opacity: 0.5, padding: "8px" }}>Processing...</div>
</div>
</DiffContainer>
);
}

// Extract lines for rendering (done once, outside map)
const lines = content.split("\n").filter((line) => line.length > 0);

return (
<div
className="bg-code-bg m-0 grid overflow-auto rounded py-1.5 [&_*]:text-[inherit]"
style={{
fontSize: fontSize ?? "12px",
lineHeight: 1.4,
maxHeight: maxHeight ?? "400px",
gridTemplateColumns: "minmax(min-content, 1fr)",
}}
>
<DiffContainer fontSize={fontSize} maxHeight={maxHeight}>
{highlightedLineData.map((lineInfo, displayIndex) => {
const isSelected = isLineSelected(displayIndex);
const indicator = lineInfo.type === "add" ? "+" : lineInfo.type === "remove" ? "-" : " ";
Expand Down Expand Up @@ -626,7 +646,7 @@ export const SelectableDiffRenderer = React.memo<SelectableDiffRendererProps>(
</React.Fragment>
);
})}
</div>
</DiffContainer>
);
}
);
Expand Down