Skip to content

Commit 07d93cb

Browse files
kylecarbskylecarbs
authored
Fix diffs having multiple scrollbars (#611)
This adds an 'Expand Diff' button which is a much better UX for viewing large diffs anyways
1 parent 7da1b59 commit 07d93cb

File tree

2 files changed

+147
-58
lines changed

2 files changed

+147
-58
lines changed

‎src/App.stories.tsx‎

Lines changed: 75 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -611,29 +611,98 @@ export const ActiveWorkspaceWithChat: Story = {
611611
},
612612
});
613613

614-
// Assistant message with file edit
614+
// Assistant message with file edit (large diff)
615615
callback({
616616
id: "msg-4",
617617
role: "assistant",
618618
parts: [
619619
{
620620
type: "text",
621-
text: "I'll add JWT token validation to the endpoint. Let me update the file.",
621+
text: "I'll add JWT token validation to the endpoint. Let me update the file with proper authentication middleware and error handling.",
622622
},
623623
{
624624
type: "dynamic-tool",
625625
toolCallId: "call-2",
626-
toolName: "search_replace",
626+
toolName: "file_edit_replace_string",
627627
state: "output-available",
628628
input: {
629629
file_path: "src/api/users.ts",
630-
old_string: "export function getUser(req, res) {",
630+
old_string:
631+
"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}",
631632
new_string:
632-
"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 }",
633+
"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}",
633634
},
634635
output: {
635636
success: true,
636-
message: "File updated successfully",
637+
diff: [
638+
"--- src/api/users.ts",
639+
"+++ src/api/users.ts",
640+
"@@ -2,0 +3,2 @@",
641+
"+import { verifyToken } from '../auth/jwt';",
642+
"+import { logger } from '../utils/logger';",
643+
"@@ -4,28 +6,14 @@",
644+
"-// TODO: Add authentication middleware",
645+
"-// Current implementation is insecure and allows unauthorized access",
646+
"-// Need to validate JWT tokens before processing requests",
647+
"-// Also need to add rate limiting to prevent abuse",
648+
"-// Consider adding request logging for audit trail",
649+
"-// Add input validation for user IDs",
650+
"-// Handle edge cases for deleted/suspended users",
651+
"-",
652+
"-/**",
653+
"- * Get user by ID",
654+
"- * @param {Object} req - Express request object",
655+
"- * @param {Object} res - Express response object",
656+
"- */",
657+
"-export function getUser(req, res) {",
658+
"- // FIXME: No authentication check",
659+
"- // FIXME: No error handling",
660+
"- // FIXME: Synchronous database call blocks event loop",
661+
"- // FIXME: No input validation",
662+
"- // FIXME: Direct database access without repository pattern",
663+
"- // FIXME: No logging",
664+
"-",
665+
"- const user = db.users.find(req.params.id);",
666+
"-",
667+
"- // TODO: Check if user exists",
668+
"- // TODO: Filter sensitive fields (password hash, etc)",
669+
"- // TODO: Check permissions - user should only access their own data",
670+
"-",
671+
"- res.json(user);",
672+
"+export async function getUser(req, res) {",
673+
"+ try {",
674+
"+ const token = req.headers.authorization?.split(' ')[1];",
675+
"+ if (!token) {",
676+
"+ logger.warn('Missing authorization token');",
677+
"+ return res.status(401).json({ error: 'Unauthorized' });",
678+
"+ }",
679+
"+ const decoded = await verifyToken(token);",
680+
"+ const user = await db.users.find(req.params.id);",
681+
"+ res.json(user);",
682+
"+ } catch (err) {",
683+
"+ logger.error('Auth error:', err);",
684+
"+ return res.status(401).json({ error: 'Invalid token' });",
685+
"+ }",
686+
"@@ -34,3 +22,2 @@",
687+
"-// TODO: Add updateUser function",
688+
"-// TODO: Add deleteUser function",
689+
"-// TODO: Add listUsers function with pagination",
690+
"+// Note: updateUser, deleteUser, and listUsers endpoints will be added in separate PR",
691+
"+// to keep changes focused and reviewable",
692+
"@@ -41,0 +29,11 @@",
693+
"+",
694+
"+export async function rotateApiKey(req, res) {",
695+
"+ const admin = await db.admins.find(req.user.id);",
696+
"+ if (!admin) {",
697+
"+ return res.status(403).json({ error: 'Forbidden' });",
698+
"+ }",
699+
"+",
700+
"+ const apiKey = await db.tokens.rotate(admin.orgId);",
701+
"+ logger.info('Rotated API key', { orgId: admin.orgId });",
702+
"+ res.json({ apiKey });",
703+
"+}",
704+
].join("\n"),
705+
edits_applied: 1,
637706
},
638707
},
639708
],

‎src/components/shared/DiffRenderer.tsx‎

Lines changed: 72 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -72,20 +72,72 @@ const getContrastColor = (type: DiffLineType): string => {
7272
export const DiffContainer: React.FC<
7373
React.PropsWithChildren<{ fontSize?: string; maxHeight?: string; className?: string }>
7474
> = ({ children, fontSize, maxHeight, className }) => {
75+
const resolvedMaxHeight = maxHeight ?? "400px";
76+
const [isExpanded, setIsExpanded] = React.useState(false);
77+
const contentRef = React.useRef<HTMLDivElement>(null);
78+
const [isOverflowing, setIsOverflowing] = React.useState(false);
79+
const clampContent = resolvedMaxHeight !== "none" && !isExpanded;
80+
81+
React.useEffect(() => {
82+
if (maxHeight === "none") {
83+
setIsExpanded(false);
84+
}
85+
}, [maxHeight]);
86+
87+
React.useEffect(() => {
88+
const element = contentRef.current;
89+
if (!element) {
90+
return;
91+
}
92+
93+
const updateOverflowState = () => {
94+
setIsOverflowing(element.scrollHeight > element.clientHeight + 1);
95+
};
96+
97+
updateOverflowState();
98+
99+
let resizeObserver: ResizeObserver | null = null;
100+
if (typeof ResizeObserver !== "undefined") {
101+
resizeObserver = new ResizeObserver(updateOverflowState);
102+
resizeObserver.observe(element);
103+
}
104+
105+
return () => {
106+
resizeObserver?.disconnect();
107+
};
108+
}, [resolvedMaxHeight, clampContent]);
109+
75110
return (
76-
<div
77-
className={cn(
78-
"m-0 py-1.5 bg-code-bg rounded grid overflow-y-auto overflow-x-auto [&_*]:text-[inherit]",
79-
className
111+
<div className={cn("relative m-0 rounded bg-code-bg py-1.5 [&_*]:text-[inherit]", className)}>
112+
<div
113+
ref={contentRef}
114+
className={cn(
115+
"grid overflow-x-auto",
116+
clampContent ? "pb-6 overflow-y-hidden" : "overflow-y-visible"
117+
)}
118+
style={{
119+
fontSize: fontSize ?? "12px",
120+
lineHeight: 1.4,
121+
maxHeight: clampContent ? resolvedMaxHeight : undefined,
122+
gridTemplateColumns: "minmax(min-content, 1fr)",
123+
}}
124+
>
125+
{children}
126+
</div>
127+
128+
{clampContent && isOverflowing && (
129+
<>
130+
<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" />
131+
<div className="absolute inset-x-0 bottom-0 flex justify-center pb-1.5">
132+
<button
133+
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"
134+
onClick={() => setIsExpanded(true)}
135+
>
136+
Expand diff
137+
</button>
138+
</div>
139+
</>
80140
)}
81-
style={{
82-
fontSize: fontSize ?? "12px",
83-
lineHeight: 1.4,
84-
maxHeight: maxHeight ?? "400px",
85-
gridTemplateColumns: "minmax(min-content, 1fr)",
86-
}}
87-
>
88-
{children}
89141
</div>
90142
);
91143
};
@@ -192,30 +244,14 @@ export const DiffRenderer: React.FC<DiffRendererProps> = ({
192244
// Show loading state while highlighting
193245
if (!highlightedChunks) {
194246
return (
195-
<div
196-
className="bg-code-bg m-0 grid overflow-auto rounded py-1.5 [&_*]:text-[inherit]"
197-
style={{
198-
fontSize: fontSize ?? "12px",
199-
lineHeight: 1.4,
200-
maxHeight: maxHeight ?? "400px",
201-
gridTemplateColumns: "minmax(min-content, 1fr)",
202-
}}
203-
>
247+
<DiffContainer fontSize={fontSize} maxHeight={maxHeight}>
204248
<div style={{ opacity: 0.5, padding: "8px" }}>Processing...</div>
205-
</div>
249+
</DiffContainer>
206250
);
207251
}
208252

209253
return (
210-
<div
211-
className="bg-code-bg m-0 grid overflow-auto rounded py-1.5 [&_*]:text-[inherit]"
212-
style={{
213-
fontSize: fontSize ?? "12px",
214-
lineHeight: 1.4,
215-
maxHeight: maxHeight ?? "400px",
216-
gridTemplateColumns: "minmax(min-content, 1fr)",
217-
}}
218-
>
254+
<DiffContainer fontSize={fontSize} maxHeight={maxHeight}>
219255
{highlightedChunks.flatMap((chunk) =>
220256
chunk.lines.map((line) => {
221257
const indicator = chunk.type === "add" ? "+" : chunk.type === "remove" ? "-" : " ";
@@ -260,7 +296,7 @@ export const DiffRenderer: React.FC<DiffRendererProps> = ({
260296
);
261297
})
262298
)}
263-
</div>
299+
</DiffContainer>
264300
);
265301
};
266302

@@ -502,33 +538,17 @@ export const SelectableDiffRenderer = React.memo<SelectableDiffRendererProps>(
502538
// Show loading state while highlighting
503539
if (!highlightedChunks || highlightedLineData.length === 0) {
504540
return (
505-
<div
506-
className="bg-code-bg m-0 grid overflow-auto rounded py-1.5 [&_*]:text-[inherit]"
507-
style={{
508-
fontSize: fontSize ?? "12px",
509-
lineHeight: 1.4,
510-
maxHeight: maxHeight ?? "400px",
511-
gridTemplateColumns: "minmax(min-content, 1fr)",
512-
}}
513-
>
541+
<DiffContainer fontSize={fontSize} maxHeight={maxHeight}>
514542
<div style={{ opacity: 0.5, padding: "8px" }}>Processing...</div>
515-
</div>
543+
</DiffContainer>
516544
);
517545
}
518546

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

522550
return (
523-
<div
524-
className="bg-code-bg m-0 grid overflow-auto rounded py-1.5 [&_*]:text-[inherit]"
525-
style={{
526-
fontSize: fontSize ?? "12px",
527-
lineHeight: 1.4,
528-
maxHeight: maxHeight ?? "400px",
529-
gridTemplateColumns: "minmax(min-content, 1fr)",
530-
}}
531-
>
551+
<DiffContainer fontSize={fontSize} maxHeight={maxHeight}>
532552
{highlightedLineData.map((lineInfo, displayIndex) => {
533553
const isSelected = isLineSelected(displayIndex);
534554
const indicator = lineInfo.type === "add" ? "+" : lineInfo.type === "remove" ? "-" : " ";
@@ -626,7 +646,7 @@ export const SelectableDiffRenderer = React.memo<SelectableDiffRendererProps>(
626646
</React.Fragment>
627647
);
628648
})}
629-
</div>
649+
</DiffContainer>
630650
);
631651
}
632652
);

0 commit comments

Comments
 (0)