Skip to content

Commit 8512d11

Browse files
authored
Implement unified, collapsed diff by default and jump to problem line (#115)
* Display unified, collapsed diff by default; otherwise show basic file viewer * Implement jumping to problem * Run prettier
1 parent 4c120af commit 8512d11

4 files changed

Lines changed: 332 additions & 80 deletions

File tree

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
import React from "react";
2+
3+
import Editor from "@monaco-editor/react";
4+
5+
import "./FileViewer.css";
6+
7+
function BasicFileViewer({ code, language, lightMode, editorRef }) {
8+
return (
9+
<>
10+
<Editor
11+
onMount={(editor) => {
12+
editorRef.current = editor;
13+
}}
14+
defaultLanguage={language}
15+
defaultValue={code}
16+
theme={lightMode ? "light" : "vs-dark"}
17+
options={{
18+
renderValidationDecorations: "on",
19+
domReadOnly: true,
20+
readOnly: true,
21+
renderLineHighlight: "all",
22+
renderWhitespace: "all",
23+
rulers: [80],
24+
scrollBeyondLastLine: false,
25+
}}
26+
/>
27+
</>
28+
);
29+
}
30+
31+
export default BasicFileViewer;

src/snapshots-app/client/bundles/components/submission/tabs/timeline/DiffViewer.jsx

Lines changed: 75 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import React, { useRef } from "react";
1+
import React from "react";
22

33
import Dialog from "@mui/material/Dialog";
44
import DialogContent from "@mui/material/DialogContent";
@@ -13,9 +13,16 @@ import { DiffEditor } from "@monaco-editor/react";
1313
// TODO I have been accidentally relying on this import for styling the entire website
1414
import "@git-diff-view/react/styles/diff-view.css";
1515

16-
function DiffViewer({ open, onClose, prevFileContents, currentFileContents }) {
17-
const editorRef = useRef(null);
18-
16+
// TODO refactor properly
17+
function DiffViewer({
18+
open,
19+
onClose,
20+
editorRef,
21+
prevFileContents,
22+
currentFileContents,
23+
lightMode = false,
24+
renderSideBySide = false,
25+
}) {
1926
const onDiffEditorMount = (editor, monaco) => {
2027
editorRef.current = editor;
2128

@@ -34,58 +41,71 @@ function DiffViewer({ open, onClose, prevFileContents, currentFileContents }) {
3441
};
3542

3643
return (
37-
<Dialog
38-
open={open}
39-
onClose={onClose}
40-
aria-labelledby="diff-viewer-dialog-title"
41-
aria-describedby="diff-viewer-dialog-description"
42-
maxWidth="xl"
43-
fullWidth
44-
>
45-
<DialogTitle id="diff-viewer-dialog-title">Diff Viewer</DialogTitle>
46-
<DialogContent>
47-
<ButtonGroup
48-
sx={{
49-
marginBottom: "1rem",
50-
}}
44+
// <Dialog
45+
// open={open}
46+
// onClose={onClose}
47+
// aria-labelledby="diff-viewer-dialog-title"
48+
// aria-describedby="diff-viewer-dialog-description"
49+
// maxWidth="xl"
50+
// fullWidth
51+
// >
52+
// <DialogTitle id="diff-viewer-dialog-title">Diff Viewer</DialogTitle>
53+
// <DialogContent>
54+
<>
55+
<ButtonGroup
56+
sx={{
57+
marginBottom: "1rem",
58+
position: "sticky",
59+
}}
60+
>
61+
<Button
62+
size="small"
63+
variant="outlined"
64+
startIcon={<ArrowUpwardIcon />}
65+
onClick={goToPreviousDiff}
66+
>
67+
Previous Change
68+
</Button>
69+
<Button
70+
size="small"
71+
variant="outlined"
72+
endIcon={<ArrowDownwardIcon />}
73+
onClick={goToNextDiff}
5174
>
52-
<Button
53-
size="small"
54-
variant="outlined"
55-
startIcon={<ArrowUpwardIcon />}
56-
onClick={goToPreviousDiff}
57-
>
58-
Previous Change
59-
</Button>
60-
<Button
61-
size="small"
62-
variant="outlined"
63-
endIcon={<ArrowDownwardIcon />}
64-
onClick={goToNextDiff}
65-
>
66-
Next Change
67-
</Button>
68-
</ButtonGroup>
69-
<DiffEditor
70-
height="100vh"
71-
original={prevFileContents}
72-
modified={currentFileContents}
73-
language="python"
74-
onMount={onDiffEditorMount}
75-
// https://github.com/suren-atoyan/monaco-react/issues/647#issuecomment-2897027817
76-
keepCurrentOriginalModel={true}
77-
keepCurrentModifiedModel={true}
78-
options={{
79-
readOnly: true,
80-
domReadOnly: true,
81-
renderLineHighlight: "all",
82-
renderWhitespace: "all",
83-
rulers: [80],
84-
scrollBeyondLastLine: false,
85-
}}
86-
/>
87-
</DialogContent>
88-
</Dialog>
75+
Next Change
76+
</Button>
77+
</ButtonGroup>
78+
<DiffEditor
79+
height="100vh"
80+
original={prevFileContents}
81+
modified={currentFileContents}
82+
language="python"
83+
theme={lightMode ? "light" : "vs-dark"}
84+
onMount={onDiffEditorMount}
85+
// https://github.com/suren-atoyan/monaco-react/issues/647#issuecomment-2897027817
86+
keepCurrentOriginalModel={true}
87+
keepCurrentModifiedModel={true}
88+
options={{
89+
readOnly: true,
90+
domReadOnly: true,
91+
renderLineHighlight: "all",
92+
renderWhitespace: "all",
93+
rulers: [80],
94+
scrollBeyondLastLine: false,
95+
renderSideBySide: renderSideBySide,
96+
// Enable the auto-collapse feature
97+
hideUnchangedRegions: {
98+
enabled: true,
99+
contextLineCount: 5, // Lines of unchanged code to show around a diff
100+
minimumLineCount: 3, // Minimum unchanged lines required to trigger a collapse
101+
// TODO this isn't working properly
102+
revealLineCount: 20, // How many lines to reveal when clicking the "expand" button
103+
},
104+
}}
105+
/>
106+
</>
107+
// {/* </DialogContent>
108+
// </Dialog> */}
89109
);
90110
}
91111

src/snapshots-app/client/bundles/components/submission/tabs/timeline/Graphs.jsx

Lines changed: 133 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ import ErrorOutlineIcon from "@mui/icons-material/ErrorOutline";
66
import { Tooltip } from "@mui/material";
77
import Typography from "@mui/material/Typography";
88
import Box from "@mui/material/Box";
9+
import { Link } from "@mui/material";
10+
import { List, ListItem } from "@mui/material";
911

1012
function LinearProgressWithLabel(props) {
1113
return (
@@ -27,7 +29,67 @@ function LinearProgressWithLabel(props) {
2729
);
2830
}
2931

30-
function AssignmentProblems({ history, allProblemDisplayNames, numSolved }) {
32+
const ProblemJumpLink = ({ lineNumber, editorRef, label }) => {
33+
const handleJump = (event) => {
34+
// Prevent default link behavior if necessary
35+
event.preventDefault();
36+
37+
const editorInstance = editorRef.current;
38+
if (!editorInstance) return;
39+
40+
const targetEditor = editorInstance.getModifiedEditor
41+
? editorInstance.getModifiedEditor()
42+
: editorInstance;
43+
44+
targetEditor.revealLineInCenter(lineNumber);
45+
targetEditor.setPosition({ lineNumber: lineNumber, column: 1 });
46+
targetEditor.focus();
47+
};
48+
49+
return (
50+
<Link
51+
component="button"
52+
variant="body2"
53+
onClick={handleJump}
54+
sx={{
55+
textAlign: "left",
56+
verticalAlign: "baseline",
57+
textDecoration: "none",
58+
"&:hover": {
59+
textDecoration: "underline",
60+
},
61+
cursor: "pointer",
62+
color: "primary.main",
63+
fontWeight: 500,
64+
}}
65+
>
66+
{label || `Line ${lineNumber}`}
67+
</Link>
68+
);
69+
};
70+
71+
function AssignmentProblems({
72+
history,
73+
allProblemDisplayNames,
74+
numSolved,
75+
editorRef,
76+
problemLines,
77+
}) {
78+
function goToLine(lineNumber) {
79+
const editor = editorRef.current;
80+
if (!editor) return;
81+
82+
// 1. Check if it's a Diff Editor (has getModifiedEditor method)
83+
// 2. Otherwise treat as a standard editor
84+
const targetEditor = editor.getModifiedEditor
85+
? editor.getModifiedEditor()
86+
: editor;
87+
88+
targetEditor.revealLineInCenter(lineNumber);
89+
targetEditor.setPosition({ lineNumber: lineNumber, column: 1 });
90+
targetEditor.focus();
91+
}
92+
3193
function getIcon(problemDisplayName) {
3294
const problemData = history.find(
3395
(p) => p.display_name === problemDisplayName,
@@ -51,11 +113,72 @@ function AssignmentProblems({ history, allProblemDisplayNames, numSolved }) {
51113
return (numSolved / allProblemDisplayNames.length) * 100;
52114
}
53115

54-
const problems = allProblemDisplayNames.map((problemDisplayName) => (
55-
<div>
56-
{getIcon(problemDisplayName)} {problemDisplayName}
57-
</div>
58-
));
116+
// TODO span styling and improve accessibility?
117+
const problems = allProblemDisplayNames.map((problemDisplayName) => {
118+
const lines = problemLines[problemDisplayName];
119+
120+
if (problemLines[problemDisplayName].length === 0) {
121+
return (
122+
<Box
123+
key={problemDisplayName}
124+
sx={{ display: "flex", alignItems: "center", my: 0.5 }}
125+
>
126+
{getIcon(problemDisplayName)}
127+
<Typography variant="body2" sx={{ ml: 1, color: "text.disabled" }}>
128+
{problemDisplayName} (Not found)
129+
</Typography>
130+
</Box>
131+
);
132+
} else if (problemLines[problemDisplayName].length === 1) {
133+
return (
134+
<Box
135+
key={problemDisplayName}
136+
sx={{ display: "flex", alignItems: "center", my: 0.5 }}
137+
>
138+
{getIcon(problemDisplayName)}
139+
<Box sx={{ ml: 1 }}>
140+
<ProblemJumpLink
141+
lineNumber={lines[0]}
142+
editorRef={editorRef}
143+
label={problemDisplayName}
144+
/>
145+
</Box>
146+
</Box>
147+
);
148+
} else {
149+
return (
150+
<Box key={problemDisplayName} sx={{ my: 1 }}>
151+
<Box sx={{ display: "flex", alignItems: "center" }}>
152+
{getIcon(problemDisplayName)}
153+
<Typography variant="body2" sx={{ ml: 1 }}>
154+
{problemDisplayName}
155+
</Typography>
156+
</Box>
157+
<Box
158+
sx={{
159+
pl: 4,
160+
display: "flex",
161+
flexDirection: "column",
162+
flexWrap: "wrap",
163+
}}
164+
>
165+
<List sx={{ listStyleType: "disc", pl: "1rem" }}>
166+
{lines.map((lineNumber) => (
167+
<ListItem disablePadding sx={{ display: "list-item" }}>
168+
<ProblemJumpLink
169+
key={`${problemDisplayName}-${lineNumber}`}
170+
lineNumber={lineNumber}
171+
editorRef={editorRef}
172+
label={`Line ${lineNumber}`}
173+
/>
174+
</ListItem>
175+
))}
176+
</List>
177+
</Box>
178+
</Box>
179+
);
180+
}
181+
});
59182

60183
return (
61184
<div style={{ paddingTop: "1rem", paddingBottom: "1rem" }}>
@@ -71,6 +194,8 @@ function Graphs({
71194
currBackupHistory,
72195
allProblemDisplayNames,
73196
selectedBackup,
197+
problemLines,
198+
editorRef,
74199
}) {
75200
return (
76201
<div>
@@ -79,6 +204,8 @@ function Graphs({
79204
history={currBackupHistory}
80205
allProblemDisplayNames={allProblemDisplayNames}
81206
numSolved={numQuestionsSolved[selectedBackup]}
207+
problemLines={problemLines}
208+
editorRef={editorRef}
82209
/>
83210
</div>
84211
);

0 commit comments

Comments
 (0)