Skip to content

Commit 54a00e8

Browse files
backnotpropbacknotprop
authored
feat: resizable split pane for code review diff viewer (#383)
Add a draggable divider between left/right panes in split diff view, allowing users to resize each side. Uses CSS custom properties that inherit into Pierre's shadow DOM for zero-rerender drag performance. Ratio persists across sessions via cookie storage. Also adds pierre-guard skill to protect the @pierre/diffs integration surface (shadow DOM selectors, CSS variables, grid layout assumptions). For provenance purposes, this commit was AI assisted.
1 parent d4ff4a8 commit 54a00e8

2 files changed

Lines changed: 211 additions & 2 deletions

File tree

‎.agents/skills/pierre-guard/SKILL.md‎

Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,137 @@
1+
---
2+
name: pierre-guard
3+
description: Guard against breaking the @pierre/diffs integration in Plannotator's code review UI. Use this skill whenever modifying DiffViewer.tsx, upgrading the @pierre/diffs package, changing unsafeCSS injection, adding new props to FileDiff, or touching shadow DOM selectors or CSS variables that cross into Pierre's shadow boundary. Also trigger when someone asks "will this break the diff viewer", "is this safe to change", or when reviewing PRs that touch the review-editor package.
4+
---
5+
6+
# Pierre Integration Guard
7+
8+
Plannotator's code review UI wraps `@pierre/diffs` — an open-source diff renderer that uses Shadow DOM. The integration is concentrated in a single file but relies on undocumented internals (shadow DOM selectors, CSS variable names, grid layout assumptions). This skill helps verify changes don't break that contract.
9+
10+
## Source of Truth
11+
12+
- **Upstream repo**: https://github.com/pierrecomputer/pierre/tree/main/packages/diffs
13+
- **Local types**: `node_modules/@pierre/diffs/dist/` (`.d.ts` files)
14+
- **Integration point**: `packages/review-editor/components/DiffViewer.tsx`
15+
- **Current version**: check `packages/review-editor/package.json` for the pinned version
16+
17+
Always verify against the upstream repo or local `.d.ts` files — don't rely on memory of the API shape.
18+
19+
## What We Import
20+
21+
```typescript
22+
import { FileDiff } from '@pierre/diffs/react';
23+
import { getSingularPatch, processFile } from '@pierre/diffs';
24+
```
25+
26+
These are the only three imports. `DiffViewer.tsx` is the only file that touches Pierre.
27+
28+
## API Surface to Guard
29+
30+
### 1. Component Props (`FileDiff`)
31+
32+
Read the current prop types from `node_modules/@pierre/diffs/dist/react/index.d.ts` or the upstream source. The props we use:
33+
34+
| Prop | Type | Notes |
35+
|------|------|-------|
36+
| `fileDiff` | `FileDiffMetadata` | From `getSingularPatch()` or `processFile()` |
37+
| `options` | `FileDiffOptions<T>` | See options table below |
38+
| `lineAnnotations` | `DiffLineAnnotation<T>[]` | `{ side, lineNumber, metadata }` |
39+
| `selectedLines` | `SelectedLineRange \| null` | `{ start, end, side }` |
40+
| `renderAnnotation` | `(ann) => ReactNode` | Custom inline annotation renderer |
41+
| `renderHoverUtility` | `(getHoveredLine) => ReactNode` | The `+` button on hover (deprecated upstream — watch for removal) |
42+
43+
### 2. Options Object
44+
45+
| Option | Value We Pass | Risk |
46+
|--------|--------------|------|
47+
| `themeType` | `'dark' \| 'light'` | Low — standard enum |
48+
| `unsafeCSS` | CSS string | **High** — targets internal selectors |
49+
| `diffStyle` | `'split' \| 'unified'` | Low — standard enum |
50+
| `diffIndicators` | `'bars'` | Low |
51+
| `hunkSeparators` | `'line-info'` | Low |
52+
| `enableLineSelection` | `true` | Low |
53+
| `enableHoverUtility` | `true` | Medium — deprecated prop |
54+
| `onLineSelectionEnd` | callback | Medium — signature could change |
55+
56+
### 3. Shadow DOM Selectors (via `unsafeCSS`)
57+
58+
These are the selectors we inject CSS rules against. They target `data-*` attributes inside Pierre's shadow DOM. If Pierre renames or removes any of these, our styling breaks silently.
59+
60+
**Currently used:**
61+
- `:host` — shadow root
62+
- `[data-diff]` — root diff container
63+
- `[data-file]` — file wrapper
64+
- `[data-diffs-header]` — header bar
65+
- `[data-error-wrapper]` — error display
66+
- `[data-virtualizer-buffer]` — virtual scroll buffer
67+
- `[data-file-info]` — file metadata row
68+
- `[data-column-number]` — line number gutter
69+
- `[data-diffs-header] [data-title]` — title (we hide it)
70+
- `[data-diff-type='split']` — split layout mode
71+
- `[data-overflow='scroll']` / `[data-overflow='wrap']` — overflow mode
72+
73+
### 4. CSS Variables We Override
74+
75+
We override these `--diffs-*` variables to theme Pierre:
76+
77+
- `--diffs-bg`, `--diffs-fg` — base colors
78+
- `--diffs-dark-bg`, `--diffs-light-bg` — theme-specific backgrounds
79+
- `--diffs-dark`, `--diffs-light` — theme-specific foregrounds
80+
81+
### 5. CSS Variables We Inject (Custom)
82+
83+
We set these on a wrapper div outside the shadow DOM, relying on CSS custom property inheritance:
84+
85+
- `--split-left`, `--split-right` — control the split pane grid ratio
86+
87+
The `unsafeCSS` grid override references these: `grid-template-columns: var(--split-left, 1fr) var(--split-right, 1fr)`. The `1fr` fallback ensures the layout is safe if the variables aren't set.
88+
89+
### 6. Grid Layout Assumption
90+
91+
Pierre's split view uses CSS Grid with `grid-template-columns: 1fr 1fr`. We override this for the resizable split pane. If Pierre changes its layout engine (e.g., to flexbox or a different grid structure), the override will stop working.
92+
93+
**How to verify:** In the upstream source, search for `grid-template-columns` in the diff component styles.
94+
95+
## Verification Checklist
96+
97+
When reviewing changes that touch the Pierre integration, check:
98+
99+
### Props & Types
100+
- [ ] Read the current `.d.ts` files to confirm prop names and types haven't changed
101+
- [ ] Check if `renderHoverUtility` is still supported (it's deprecated — may be removed)
102+
- [ ] Verify `DiffLineAnnotation` still uses `side: 'deletions' | 'additions'` (not `'old' | 'new'`)
103+
- [ ] Confirm `SelectedLineRange` shape: `{ start, end, side? }`
104+
105+
### Shadow DOM Selectors
106+
- [ ] Grep the upstream source for each `data-*` attribute we target in `unsafeCSS`
107+
- [ ] If upgrading the package version, diff the old and new CSS/HTML output for renamed attributes
108+
- [ ] Test both `split` and `unified` views — selectors are layout-dependent
109+
110+
### CSS Variables
111+
- [ ] Grep upstream for `--diffs-bg`, `--diffs-fg`, and other variables we override
112+
- [ ] Verify the variable names haven't been renamed or removed
113+
- [ ] Check that `!important` is still needed (Pierre may change specificity)
114+
115+
### Theme Compliance
116+
- [ ] New UI elements must use theme tokens (`bg-border`, `bg-primary`, etc.), not hardcoded colors like `bg-blue-500`
117+
- [ ] The existing `ResizeHandle` component in `packages/ui/components/ResizeHandle.tsx` sets the visual convention — match it
118+
119+
### Build & Runtime
120+
- [ ] Run `bun run dev:review` and verify the diff renders in both split and unified modes
121+
- [ ] Check the browser console for Pierre warnings (e.g., `parseLineType: Invalid firstChar`)
122+
- [ ] Test with add-only and delete-only files (Pierre doesn't render split grid for these)
123+
- [ ] If changing UI code, remember build order: `bun run --cwd apps/review build && bun run build:hook`
124+
125+
## When Upgrading @pierre/diffs
126+
127+
1. Check the upstream changelog / commit history at https://github.com/pierrecomputer/pierre
128+
2. Diff the `.d.ts` files between old and new versions:
129+
```bash
130+
# Before upgrading, snapshot current types
131+
cp -r node_modules/@pierre/diffs/dist /tmp/pierre-old
132+
# After upgrading
133+
diff -r /tmp/pierre-old node_modules/@pierre/diffs/dist
134+
```
135+
3. Search for renamed/removed data attributes in the new version
136+
4. Run through the full verification checklist above
137+
5. Test the resizable split pane — it depends on grid layout internals

‎packages/review-editor/components/DiffViewer.tsx‎

Lines changed: 74 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { getSingularPatch, processFile } from '@pierre/diffs';
44
import { CodeAnnotation, CodeAnnotationType, SelectedLineRange, DiffAnnotationMetadata } from '@plannotator/ui/types';
55
import { useTheme } from '@plannotator/ui/components/ThemeProvider';
66
import { CommentPopover } from '@plannotator/ui/components/CommentPopover';
7+
import { storage } from '@plannotator/ui/utils/storage';
78
import { detectLanguage } from '../utils/detectLanguage';
89
import { useAnnotationToolbar } from '../hooks/useAnnotationToolbar';
910
import { FileHeader } from './FileHeader';
@@ -92,6 +93,57 @@ export const DiffViewer: React.FC<DiffViewerProps> = ({
9293
const containerRef = useRef<HTMLDivElement>(null);
9394
const [fileCommentAnchor, setFileCommentAnchor] = useState<HTMLElement | null>(null);
9495

96+
// Resizable split pane — only applies when Pierre renders a two-column grid
97+
// (files with both additions and deletions). Add-only or delete-only files
98+
// render as a single column even in split mode.
99+
const isSplitLayout = useMemo(() => {
100+
if (diffStyle !== 'split') return false;
101+
let hasAdd = false, hasDel = false;
102+
for (const line of patch.split('\n')) {
103+
if (line[0] === '+' && !line.startsWith('+++')) hasAdd = true;
104+
else if (line[0] === '-' && !line.startsWith('---')) hasDel = true;
105+
if (hasAdd && hasDel) return true;
106+
}
107+
return false;
108+
}, [patch, diffStyle]);
109+
110+
const [splitRatio, setSplitRatio] = useState(() => {
111+
const saved = storage.getItem('review-split-ratio');
112+
const n = saved ? Number(saved) : NaN;
113+
return !Number.isNaN(n) && n >= 0.2 && n <= 0.8 ? n : 0.5;
114+
});
115+
const splitRatioRef = useRef(splitRatio);
116+
splitRatioRef.current = splitRatio;
117+
const [isDraggingSplit, setIsDraggingSplit] = useState(false);
118+
119+
const handleSplitDragStart = useCallback((e: React.PointerEvent) => {
120+
e.preventDefault();
121+
const container = containerRef.current;
122+
if (!container) return;
123+
const rect = container.getBoundingClientRect();
124+
setIsDraggingSplit(true);
125+
126+
const onMove = (e: PointerEvent) => {
127+
const ratio = (e.clientX - rect.left) / rect.width;
128+
setSplitRatio(Math.min(0.8, Math.max(0.2, ratio)));
129+
};
130+
131+
const onUp = () => {
132+
setIsDraggingSplit(false);
133+
document.removeEventListener('pointermove', onMove);
134+
document.removeEventListener('pointerup', onUp);
135+
storage.setItem('review-split-ratio', String(splitRatioRef.current));
136+
};
137+
138+
document.addEventListener('pointermove', onMove);
139+
document.addEventListener('pointerup', onUp);
140+
}, []);
141+
142+
const resetSplitRatio = useCallback(() => {
143+
setSplitRatio(0.5);
144+
storage.setItem('review-split-ratio', '0.5');
145+
}, []);
146+
95147
const toolbar = useAnnotationToolbar({ patch, filePath, onLineSelection, onAddAnnotation, onEditAnnotation });
96148

97149
// Parse patch into FileDiffMetadata for @pierre/diffs FileDiff component
@@ -295,6 +347,10 @@ export const DiffViewer: React.FC<DiffViewerProps> = ({
295347
[data-file-info] { background-color: ${muted} !important; }
296348
[data-column-number] { background-color: ${bg} !important; }
297349
[data-diffs-header] [data-title] { display: none !important; }
350+
[data-diff-type='split'][data-overflow='scroll'],
351+
[data-diff-type='split'][data-overflow='wrap'] {
352+
grid-template-columns: var(--split-left, 1fr) var(--split-right, 1fr) !important;
353+
}
298354
`,
299355
});
300356
});
@@ -315,8 +371,24 @@ export const DiffViewer: React.FC<DiffViewerProps> = ({
315371
onFileComment={setFileCommentAnchor}
316372
/>
317373

318-
<div ref={containerRef} className="flex-1 overflow-auto relative" onMouseMove={toolbar.handleMouseMove}>
319-
<div className="p-4">
374+
<div ref={containerRef} className={`flex-1 overflow-auto relative ${isDraggingSplit ? 'select-none' : ''}`} onMouseMove={toolbar.handleMouseMove}>
375+
{isSplitLayout && (
376+
<div
377+
className="absolute top-0 bottom-0 z-10 cursor-col-resize group"
378+
style={{ left: `${splitRatio * 100}%`, width: 9, marginLeft: -4 }}
379+
onPointerDown={handleSplitDragStart}
380+
onDoubleClick={resetSplitRatio}
381+
>
382+
<div className="absolute inset-y-0 left-1/2 w-px bg-border group-hover:bg-primary/50 group-active:bg-primary/70 transition-colors" />
383+
</div>
384+
)}
385+
<div
386+
className="p-4"
387+
style={isSplitLayout ? {
388+
'--split-left': `${splitRatio}fr`,
389+
'--split-right': `${1 - splitRatio}fr`,
390+
} as React.CSSProperties : undefined}
391+
>
320392
<FileDiff
321393
key={filePath}
322394
fileDiff={augmentedDiff}

0 commit comments

Comments
 (0)