Skip to content

Commit a11b87b

Browse files
committed
feat(review-editor/mobile): inline annotation indicators in the diff
Annotations now show up where they belong instead of in a flat list at the bottom of each file: - Lines that are part of any annotation range get an amber left border + faint background tint and a small comment-bubble icon at the right edge so you can spot them while scrolling the diff. - Annotation cards render inline directly under the last line of their range, breaking the diff into chunks separated by the cards (each chunk has its own horizontal scroll). - File-scoped annotations (scope: 'file') get their own "File-level comments" header at the top of the file body, above the diff. - Selection ring still takes precedence over the annotation tint so the active tap target stays unambiguous.
1 parent d58c632 commit a11b87b

1 file changed

Lines changed: 119 additions & 44 deletions

File tree

packages/review-editor/components/MobileReviewView.tsx

Lines changed: 119 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,57 @@ const FileCard: React.FC<FileCardProps> = ({
182182
[selection, fileIdx],
183183
);
184184

185+
// Split annotations into file-scoped (rendered above the diff) and
186+
// line-scoped (rendered inline below their last line). The latter is
187+
// keyed by `${side}:${lineEnd}` for fast lookup while walking lines.
188+
const { fileScopedAnnotations, annotationsEndingAt, isLineAnnotated } = useMemo(() => {
189+
const fileScoped: CodeAnnotation[] = [];
190+
const endMap = new Map<string, CodeAnnotation[]>();
191+
const inRange = (side: 'old' | 'new', num: number) =>
192+
fileAnnotations.some(
193+
ann => ann.scope !== 'file' && ann.side === side && num >= ann.lineStart && num <= ann.lineEnd,
194+
);
195+
for (const ann of fileAnnotations) {
196+
if (ann.scope === 'file') {
197+
fileScoped.push(ann);
198+
continue;
199+
}
200+
const key = `${ann.side}:${ann.lineEnd}`;
201+
const existing = endMap.get(key);
202+
if (existing) existing.push(ann);
203+
else endMap.set(key, [ann]);
204+
}
205+
return {
206+
fileScopedAnnotations: fileScoped,
207+
annotationsEndingAt: endMap,
208+
isLineAnnotated: inRange,
209+
};
210+
}, [fileAnnotations]);
211+
212+
// Walk the diff lines and split them into "chunks": runs of consecutive
213+
// lines that share a single horizontally-scrollable container, broken
214+
// wherever an annotation needs to be inserted. Each break carries the
215+
// annotation cards that should appear after the last line of the chunk.
216+
const chunks = useMemo(() => {
217+
const result: { lines: DiffLine[]; cards: CodeAnnotation[] }[] = [];
218+
let current: DiffLine[] = [];
219+
for (const line of lines) {
220+
current.push(line);
221+
const side: 'old' | 'new' | null =
222+
line.kind === 'add' ? 'new' : line.kind === 'del' ? 'old' : null;
223+
const lineNum = line.kind === 'add' ? line.newNum : line.kind === 'del' ? line.oldNum : undefined;
224+
if (side && lineNum != null) {
225+
const cards = annotationsEndingAt.get(`${side}:${lineNum}`);
226+
if (cards && cards.length) {
227+
result.push({ lines: current, cards });
228+
current = [];
229+
}
230+
}
231+
}
232+
if (current.length) result.push({ lines: current, cards: [] });
233+
return result;
234+
}, [lines, annotationsEndingAt]);
235+
185236
return (
186237
<section className="border border-border/60 rounded-lg bg-card overflow-hidden">
187238
<button
@@ -219,56 +270,80 @@ const FileCard: React.FC<FileCardProps> = ({
219270

220271
{isOpen && (
221272
<div className="border-t border-border/40">
222-
<div className="overflow-x-auto">
223-
<pre className="text-[11px] leading-relaxed font-mono py-1 min-w-fit">
224-
{lines.map((line, i) => {
225-
const tappable = line.kind === 'add' || line.kind === 'del';
226-
const side: 'old' | 'new' | null = line.kind === 'add' ? 'new' : line.kind === 'del' ? 'old' : null;
227-
const lineNum = line.kind === 'add' ? line.newNum : line.kind === 'del' ? line.oldNum : undefined;
228-
const selected = side ? isLineSelected(side, lineNum) : false;
229-
const Inner = (
230-
<>
231-
<span className="select-none w-8 flex-shrink-0 text-right pr-2 opacity-50 text-[10px] tabular-nums">
232-
{line.kind === 'add' && line.newNum ? line.newNum : line.kind === 'del' && line.oldNum ? line.oldNum : ''}
233-
</span>
234-
<span className="select-none w-3 flex-shrink-0 opacity-60">{linePrefix[line.kind]}</span>
235-
<span className="select-none whitespace-pre flex-1">{line.text || ' '}</span>
236-
</>
237-
);
238-
if (tappable && side && lineNum != null) {
239-
return (
240-
<button
241-
key={i}
242-
type="button"
243-
onClick={() => onTapLine(fileIdx, side, lineNum)}
244-
style={{ WebkitTouchCallout: 'none', WebkitUserSelect: 'none', touchAction: 'manipulation' } as React.CSSProperties}
245-
className={`select-none flex w-full text-left px-1 py-0.5 border-l-2 ${lineClass[line.kind]} ${
246-
selected ? 'border-primary ring-1 ring-primary bg-primary/5' : 'border-transparent'
247-
} active:opacity-60`}
248-
>
249-
{Inner}
250-
</button>
251-
);
252-
}
253-
return (
254-
<div key={i} className={`flex px-1 border-l-2 border-transparent ${lineClass[line.kind]}`}>
255-
{Inner}
256-
</div>
257-
);
258-
})}
259-
</pre>
260-
</div>
261-
262-
{fileAnnotations.length > 0 && (
263-
<div className="border-t border-border/40 p-3 space-y-2 bg-muted/20">
273+
{fileScopedAnnotations.length > 0 && (
274+
<div className="border-b border-border/40 p-3 space-y-2 bg-muted/20">
264275
<div className="text-[10px] uppercase tracking-wider text-muted-foreground font-medium">
265-
Annotations
276+
File-level comments
266277
</div>
267-
{fileAnnotations.map(ann => (
278+
{fileScopedAnnotations.map(ann => (
268279
<AnnotationCard key={ann.id} annotation={ann} onDelete={onDeleteAnnotation} />
269280
))}
270281
</div>
271282
)}
283+
{chunks.map((chunk, ci) => (
284+
<React.Fragment key={ci}>
285+
<div className="overflow-x-auto">
286+
<div className="text-[11px] leading-relaxed font-mono py-1 min-w-fit">
287+
{chunk.lines.map((line, i) => {
288+
const tappable = line.kind === 'add' || line.kind === 'del';
289+
const side: 'old' | 'new' | null = line.kind === 'add' ? 'new' : line.kind === 'del' ? 'old' : null;
290+
const lineNum = line.kind === 'add' ? line.newNum : line.kind === 'del' ? line.oldNum : undefined;
291+
const selected = side ? isLineSelected(side, lineNum) : false;
292+
const annotated = side && lineNum != null ? isLineAnnotated(side, lineNum) : false;
293+
const Inner = (
294+
<>
295+
<span className="select-none w-8 flex-shrink-0 text-right pr-2 opacity-50 text-[10px] tabular-nums">
296+
{line.kind === 'add' && line.newNum ? line.newNum : line.kind === 'del' && line.oldNum ? line.oldNum : ''}
297+
</span>
298+
<span className="select-none w-3 flex-shrink-0 opacity-60">{linePrefix[line.kind]}</span>
299+
<span className="select-none whitespace-pre flex-1">{line.text || ' '}</span>
300+
{annotated && (
301+
<span className="select-none flex-shrink-0 ml-1 text-amber-600 dark:text-amber-400" aria-label="Has annotation">
302+
<svg className="w-3 h-3 inline" fill="currentColor" viewBox="0 0 24 24">
303+
<path d="M4 4h16a2 2 0 012 2v10a2 2 0 01-2 2h-7l-5 4v-4H4a2 2 0 01-2-2V6a2 2 0 012-2z" />
304+
</svg>
305+
</span>
306+
)}
307+
</>
308+
);
309+
// Selection ring takes precedence over annotation highlight
310+
// for the active selection target; annotation tint shows
311+
// through on non-selected annotated lines.
312+
const borderClass = selected
313+
? 'border-primary ring-1 ring-primary bg-primary/5'
314+
: annotated
315+
? 'border-amber-500/70 bg-amber-500/[0.06]'
316+
: 'border-transparent';
317+
if (tappable && side && lineNum != null) {
318+
return (
319+
<button
320+
key={i}
321+
type="button"
322+
onClick={() => onTapLine(fileIdx, side, lineNum)}
323+
style={{ WebkitTouchCallout: 'none', WebkitUserSelect: 'none', touchAction: 'manipulation' } as React.CSSProperties}
324+
className={`select-none flex w-full text-left px-1 py-0.5 border-l-2 ${lineClass[line.kind]} ${borderClass} active:opacity-60`}
325+
>
326+
{Inner}
327+
</button>
328+
);
329+
}
330+
return (
331+
<div key={i} className={`flex px-1 border-l-2 border-transparent ${lineClass[line.kind]}`}>
332+
{Inner}
333+
</div>
334+
);
335+
})}
336+
</div>
337+
</div>
338+
{chunk.cards.length > 0 && (
339+
<div className="px-3 py-2.5 space-y-2 bg-amber-500/5 border-y border-amber-500/20">
340+
{chunk.cards.map(ann => (
341+
<AnnotationCard key={ann.id} annotation={ann} onDelete={onDeleteAnnotation} />
342+
))}
343+
</div>
344+
)}
345+
</React.Fragment>
346+
))}
272347
</div>
273348
)}
274349
</section>

0 commit comments

Comments
 (0)