mirror of
https://github.com/backnotprop/plannotator.git
synced 2026-09-14 14:17:26 +08:00
fix(review): preserve dragged diff ranges on compact touch before commenting (#1333)
* feat: add mobile touch range selection prototypes * docs: redirect mobile range selection spike * revert: remove command-mediated touch range prototype * docs: align mobile line selection with DiffsHub * feat: preserve mobile diff ranges before commenting * fix(review): repaint a preserved mobile range on a second drag A preserved range leaves pendingSelection non-null, so DiffViewer hands Pierre a defined selectedLines and Pierre switches to controlled selection: updateSelection then only records a proposed range and leaves painting to the host. With no change handler wired, a second drag never repainted, so the old highlight stayed put and the finger was untracked until release. Wire onLineSelectionChange back into app state, only on compact touch, so desktop keeps an options object with no such key. Also route a null range to the toolbar host instead of swallowing it in the preserve branch, mirroring AllFilesCodeView's early return so an open composer (Ask AI included) closes with the selection it was written for. * fix(review): 44px hit area for Pierre's gutter comment button on touch With a dragged range now preserved instead of opening the composer, that button is the only way to start writing about it, and it is roughly 20px square: below the data-pn-touch-target standard the rest of the compact shell holds. Grow its invisible ::before hit area to the 44px token, leaving the glyph alone. The rule ships through the same unsafeCSS both diff surfaces already inject, which lands in Pierre's shadow root inside @layer unsafe (last in the library's layer order, so no !important). It is injected only when the shell is compact rather than gated in CSS: html:has() matches nothing from inside a shadow root, and @media (pointer: coarse) would wrongly claim a desktop with a touchscreen. * test(review): cover the compact-touch preserved range on both diff surfaces DiffViewer.compactTouchSelection.test.tsx drives the FileDiff options the component hands Pierre: a drag preserves the range instead of opening the composer and paints it through selectedLines, a second drag repaints through onLineSelectionChange, the gutter action opens the composer, a cleared range still reaches the toolbar host, and desktop keeps routing completed drags straight to the composer with no change handler at all. The last three assertions fail against the pre-fix component. The AllFilesCodeView compact test only checked what was published upward, which a range nothing paints would also satisfy; it now asserts the range reaches the CodeView props. That needs the App loop, so the mount feeds published selections back down as pendingSelection: without it the reconcile effect clears the highlight the preserve branch just painted. * chore(guides-show): regenerate viewer manifest pin for the touch selection changes
This commit is contained in:
Michael Ramos
@@ -100,6 +100,7 @@ jobs:
|
||||
packages/review-editor/edit/discardRestoreRender.test.tsx
|
||||
packages/review-editor/edit/selectionActionPopover.test.ts
|
||||
packages/review-editor/components/AllFilesCodeView.lifecycle.test.tsx
|
||||
packages/review-editor/components/DiffViewer.compactTouchSelection.test.tsx
|
||||
packages/guide-viewer/GuideSectionCard.test.tsx
|
||||
packages/guide-viewer/GuideView.test.tsx
|
||||
packages/guide-viewer/GuideViewportManager.test.tsx
|
||||
|
||||
@@ -0,0 +1,243 @@
|
||||
# Mobile touch range selection spike
|
||||
|
||||
**Status:** Code Review interaction approved for implementation; Plan interaction exploration remains open
|
||||
|
||||
**Branch:** `codex/mobile-touch-selection`
|
||||
|
||||
**Baseline:** `origin/main` at `eb6a59e2dc6e418c1c4ed7830ee210091d754061`
|
||||
**Surfaces:** Markdown Plan Review and Pierre-backed Code Review
|
||||
|
||||
## Why this exists
|
||||
|
||||
The shipped mobile foundation makes a single Plan block or diff line practical to annotate, but extending that target still depends on a desktop-shaped gesture:
|
||||
|
||||
- Safari text selection is the only way to span multiple Plan blocks in Drag mode. It raises the native Copy / Find Selection UI and can prevent Plannotator's own actions from receiving the next tap.
|
||||
- Pierre already supports direct line-number dragging with Pointer Events. Physical use in DiffsHub confirmed that gesture is practical on mobile; Plannotator's remaining mismatch was opening the composer immediately on release instead of preserving the range and exposing Pierre's contextual gutter action.
|
||||
|
||||
The goal is a touch-native way to choose one contiguous range. It is not a new annotation model and it is not a general-purpose mobile text editor.
|
||||
|
||||
## Product feedback that changed the direction
|
||||
|
||||
The first prototype's **Extend selection** and **Adjust lines** commands are rejected. They add a mode switch between the initial target and the actual range gesture, making the interaction slower and less physical than the thing it replaces. This is not a copy or presentation problem; the command-mediated model itself is wrong.
|
||||
|
||||
The next experiments must satisfy a stronger contract:
|
||||
|
||||
- no preparatory command before extending a range;
|
||||
- continuous visual feedback while the finger moves;
|
||||
- release commits the target and exposes the normal annotation actions;
|
||||
- ordinary scrolling remains available outside the active selection affordance;
|
||||
- Plan and Code Review may use different acquisition gestures because their content models are different.
|
||||
|
||||
## What the platform actually provides
|
||||
|
||||
### Safari-native text selection
|
||||
|
||||
Safari owns long-press text selection, its draggable leading/trailing handles, magnifier, and Copy / Find callout. The web Selection API lets Plannotator observe and preserve the resulting range through `selectionchange`, but it exposes no selection-handle UI and no supported extension point for adding Plannotator actions to Safari's callout.
|
||||
|
||||
`-webkit-touch-callout: none` is a non-standard Safari control for the long-press callout. It is not evidence that system selection handles will remain usable, and WebKit has version-specific long-press/loupe behavior. Any combination of native handles, suppressed callout, and custom Plannotator actions therefore requires a physical-Safari experiment rather than a code-only conclusion.
|
||||
|
||||
Primary evidence:
|
||||
|
||||
- [W3C Selection API](https://www.w3.org/TR/selection-api/) defines the document selection, `selectstart`, and `selectionchange`; it does not expose system handles or menu customization.
|
||||
- [W3C Pointer Events](https://www.w3.org/TR/pointerevents/) states that `touch-action` governs browser panning/zooming, not text selection or highlighting.
|
||||
- [Apple Safari CSS reference](https://developer.apple.com/library/archive/documentation/AppleApplications/Reference/SafariCSSRef/Articles/StandardCSSProperties.html) documents `-webkit-touch-callout` as a Safari-specific callout switch.
|
||||
- [WebKit bug 231161](https://bugs.webkit.org/show_bug.cgi?id=231161) distinguishes text selection (`user-select`) from long-press callout/loupe behavior and demonstrates why version-qualified device testing is required.
|
||||
|
||||
### Pierre line-range selection
|
||||
|
||||
`@pierre/diffs` 1.3.2 already provides the desired direct gesture. A pointer down in the line-number column seeds a range, document-level Pointer Events track across rows, the painted selection updates continuously, and pointer up emits `onLineSelectionEnd`. The gutter is already `user-select: none` and `touch-action: none`, so this path neither invokes native text selection nor hands the active drag to page scrolling.
|
||||
|
||||
At the `origin/main` baseline, Plannotator already set `enableLineSelection: true` and routed `onLineSelectionEnd` into the existing annotation toolbar in both File and All Files views. The implementation below changes only that post-gesture routing on compact touch. Range state and the interaction engine remain Pierre-owned.
|
||||
|
||||
## Method
|
||||
|
||||
1. Trace the existing acquisition, preview, draft, commit, restore, and export paths before introducing state.
|
||||
2. Reuse the canonical range already accepted by each annotation pipeline.
|
||||
3. Keep selection separate from text entry: choosing or adjusting a target must not focus a textarea or summon the software keyboard.
|
||||
4. Gate new composition behind the shared compact-touch predicate: `(max-width: 1024px) and (pointer: coarse)`.
|
||||
5. Treat desktop fine-pointer behavior as a control. Existing click, drag, keyboard, toolbar, and composer behavior must remain unchanged.
|
||||
6. Use rendered browser checks as preflight and physical iPhone/iPad Safari as the release authority.
|
||||
|
||||
## Existing contracts
|
||||
|
||||
### Plan Review
|
||||
|
||||
`usePinpoint` resolves taps through the same ordered `SemanticTargetGraph` used by Vim navigation. `useAnnotationHighlighter` already accepts a DOM `Range` that can cross block boundaries, and web-highlighter serializes one source with start/end metadata. Comments, redlines, drafts, export, reload restoration, and sidebar navigation therefore do not need a second annotation schema for a contiguous block span.
|
||||
|
||||
The missing seam is acquisition. Today Pinpoint is disabled as soon as its first selection opens a toolbar or composer.
|
||||
|
||||
### Code Review
|
||||
|
||||
`SelectedLineRange` already represents a multi-line target. `useAnnotationToolbar` extracts the selected code and commits the same start/end span, while both `DiffViewer` and `AllFilesCodeView` project `pendingSelection` back into Pierre as a controlled selection.
|
||||
|
||||
Pierre's current interaction manager already uses Pointer Events and supports a gutter drag. The rejected prototype incorrectly treated a non-drag alternative as the missing seam. The approved Code Review implementation preserves Pierre's direct gesture and changes no range type, hit geometry, or renderer internals.
|
||||
|
||||
## Rejected shared interaction model
|
||||
|
||||
The first prototype used an explicit two-step adjustment:
|
||||
|
||||
1. The user's current single target becomes the fixed anchor.
|
||||
2. The user invokes a local **Extend** / **Adjust lines** action.
|
||||
3. The active editor/composer yields without discarding its draft. A small, safe-area-aware instruction surface says what to tap and offers Cancel.
|
||||
4. The next eligible target in the same document or file becomes the other endpoint.
|
||||
5. Plannotator previews the normalized contiguous range and returns to the prior toolbar/composer.
|
||||
|
||||
This was intentionally endpoint selection rather than tap-to-toggle arbitrary items. Its data model was sound, but its interaction was not ergonomic enough to continue. The implementation remains useful as evidence that both annotation pipelines already accept contiguous ranges; it is not the proposed UI.
|
||||
|
||||
## Direct-manipulation experiments
|
||||
|
||||
### Code Review — DiffsHub reference and approved behavior
|
||||
|
||||
Physical feedback confirms multiline selection works well in the standalone DiffsHub app at `/Users/ramos/oss/pierre`. Source comparison shows that DiffsHub and Plannotator already enable the same Pierre contracts: `enableLineSelection`, `enableGutterUtility`, `onLineSelectionEnd`, and `onGutterUtilityClick`.
|
||||
|
||||
The important difference is what happens after selection:
|
||||
|
||||
- DiffsHub's `onLineSelectionEnd` only preserves the controlled `selectedLines` range and updates its line link. It does not open an editor.
|
||||
- DiffsHub's `onGutterUtilityClick` separately creates the draft comment for the selected range.
|
||||
- Plannotator currently routes both callbacks into `ToolbarHost.handleLineSelectionEnd`; compact touch therefore opens the expanded composer as soon as the range gesture ends.
|
||||
|
||||
The compact-touch behavior now matches DiffsHub's selection-first transition:
|
||||
|
||||
1. Touch a line number and begin dragging in one motion.
|
||||
2. Paint the selected range 1:1 under the finger as it crosses rows.
|
||||
3. Release with the multiline range still selected and no keyboard or composer opened.
|
||||
4. Activate the contextual gutter comment utility to open the normal feedback composer for that range.
|
||||
|
||||
There is no preparatory **Adjust lines** command and no persistent instruction. The gutter utility is a contextual action after direct selection, not a mode switch before it. Since the incumbent DiffsHub geometry already passed a physical multiline-selection check, Plannotator should first adopt the state-transition parity without changing Pierre's Shadow DOM, gesture recognizer, or gutter dimensions. Geometry changes require separate physical evidence.
|
||||
|
||||
Implementation decision: compact-touch `onLineSelectionEnd` preserves the controlled range and active file without mounting the composer. The existing `onGutterUtilityClick` path remains the explicit writing action and opens the composer. Fine-pointer desktop retains the incumbent selection-to-composer transition. This changes no Pierre option, selector, CSS variable, gesture, or range type.
|
||||
|
||||
Validation must cover vertical page-scroll intent near the gutter, horizontal code scrolling, split and unified sides, forward and reverse drag, edge auto-scroll, single-line selection, selection replacement, dismissing without writing, and selection persistence while the composer opens.
|
||||
|
||||
### Plan — candidate A: native range, actions out of the way
|
||||
|
||||
1. Long-press text and use Safari's own handles to select the desired words or blocks.
|
||||
2. Preserve the live range from `selectionchange`.
|
||||
3. Present Plannotator annotation actions in a safe-area-aware bottom dock, spatially separate from Safari's selection callout.
|
||||
4. Capture the saved range on action pointer-down so Safari collapsing the visible selection does not lose the target.
|
||||
|
||||
This candidate wins if native handles can be extended across rendered Markdown and the bottom actions remain tappable without disabling ordinary copy, lookup, accessibility, or scrolling. Suppressing Safari's callout is an optional experimental cell, not the default assumption.
|
||||
|
||||
### Plan — candidate B: semantic range handles
|
||||
|
||||
1. A normal Pinpoint tap selects one semantic block as today.
|
||||
2. The selected range exposes a direct trailing handle; there is no **Extend** command.
|
||||
3. Dragging that handle over another semantic block continuously expands or contracts the contiguous range.
|
||||
4. Edge proximity auto-scrolls the document while the handle remains attached to the finger.
|
||||
5. Releasing returns the ordinary annotation toolbar at the visible endpoint.
|
||||
|
||||
Only the handle owns `touch-action: none`; the document keeps native vertical scrolling everywhere else. This is custom web UI, but it borrows the familiar leading/trailing-handle model and maps the finger directly to the selected extent.
|
||||
|
||||
Candidate B is preferred over a whole-document drag recognizer because taking over a vertical drag anywhere in Plan would conflict with its primary reading/scrolling gesture. A hold-then-drag recognizer is also lower priority because it competes with Safari's long-press selection and introduces a disambiguation delay.
|
||||
|
||||
## Historical rejected Plan prototype: block endpoint selection
|
||||
|
||||
Eligibility:
|
||||
|
||||
- compact-touch layout;
|
||||
- Pinpoint input method;
|
||||
- ordinary text-bearing semantic targets in the same rendered Markdown document;
|
||||
- a pending selection toolbar exists.
|
||||
|
||||
Flow:
|
||||
|
||||
1. Tap a paragraph or list item in Pinpoint.
|
||||
2. Tap **Extend** in the selection toolbar.
|
||||
3. The toolbar yields; the original highlight stays as the anchor.
|
||||
4. Scroll normally and tap the last paragraph or list item.
|
||||
5. A DOM range is built from the first boundary of the earlier block to the last boundary of the later block, regardless of tap direction.
|
||||
6. The ordinary toolbar returns over the combined highlight. Comment, delete, quick-label, copy, cancel, draft, submit, and reload paths remain incumbent.
|
||||
|
||||
First-prototype limits:
|
||||
|
||||
- one contiguous range;
|
||||
- text blocks only as endpoints; code, math, tables, and raw HTML keep their specialized selection paths;
|
||||
- adjustment begins from the selection toolbar, before the comment composer opens;
|
||||
- changing documents or leaving Pinpoint cancels adjustment and restores the original pending target.
|
||||
|
||||
## Historical rejected Code Review prototype: line endpoint selection
|
||||
|
||||
Eligibility:
|
||||
|
||||
- compact-touch layout;
|
||||
- a new line comment draft, not an existing annotation edit or token-only annotation;
|
||||
- an active file and an ordinary line range.
|
||||
|
||||
Flow:
|
||||
|
||||
1. Select a line and begin a comment as today.
|
||||
2. Tap **Adjust lines** beside the range title.
|
||||
3. The composer yields without saving, clearing, or focusing anything. A compact instruction surface says **Tap the last line** and offers Cancel.
|
||||
4. Tap a line number in the same file. Plannotator combines that endpoint with the original anchor and updates Pierre through controlled `selectedLines`.
|
||||
5. The same draft composer returns with its text, labels, decorations, suggestion state, and caret data intact. The title reflects the new range.
|
||||
|
||||
First-prototype limits:
|
||||
|
||||
- same file only;
|
||||
- same diff side only, matching the annotation/export model's single-side original-code extraction;
|
||||
- no range adjustment while editing an existing submitted annotation;
|
||||
- no attempt to replace Pierre's existing mouse/trackpad drag or Shift extension.
|
||||
|
||||
## Historical rejected-prototype state and cancellation rules
|
||||
|
||||
- The committed selection is not changed until a valid endpoint is tapped.
|
||||
- Cancel returns to the prior toolbar/composer with its original range and draft.
|
||||
- Escape performs the same cancellation when a hardware keyboard is present.
|
||||
- Device rotation and visual-viewport changes preserve adjustment state.
|
||||
- Changing the active Plan document, review file, diff family, or edit session cancels adjustment.
|
||||
- A tap on an ineligible endpoint does not discard the draft and does not create a partial range.
|
||||
- Starting adjustment never opens the software keyboard. Returning to the composer does not auto-focus on a coarse pointer.
|
||||
|
||||
## Historical rejected-prototype visual and accessibility contract
|
||||
|
||||
- The anchor and candidate range use existing selection colors; no new permanent document chrome is introduced.
|
||||
- The temporary instruction surface is above Safari's home-indicator inset and never owns page scrolling.
|
||||
- Actions are at least 44 by 44 CSS pixels in compact touch layout.
|
||||
- Status text is announced through a polite live region.
|
||||
- Color is not the only state indicator: the instruction text and range label identify adjustment mode.
|
||||
- Reduced-motion mode removes any yield/re-entry transition.
|
||||
|
||||
## Desktop non-regression contract
|
||||
|
||||
At a fine primary pointer, the new props and state are inert:
|
||||
|
||||
- Plan Pinpoint and Drag selection behave exactly as on `origin/main`.
|
||||
- Pierre line-number drag, Shift extension, mouse selection, hover utility, split/unified rendering, and keyboard shortcuts are unchanged.
|
||||
- Toolbar and composer geometry are unchanged.
|
||||
- No media query uses `any-pointer: coarse`.
|
||||
|
||||
## Validation matrix
|
||||
|
||||
Automated coverage:
|
||||
|
||||
- range normalization in forward and reverse document order;
|
||||
- Plan cancellation, invalid target, mode/document change, and source replacement;
|
||||
- Code Review draft preservation, same-side range combination, invalid side/file, cancellation, and controlled-selection projection;
|
||||
- compact-touch gate on a phone/iPad profile and inert behavior on narrow fine-pointer and hybrid-primary-mouse profiles;
|
||||
- no focus call during adjustment.
|
||||
|
||||
Rendered preflight:
|
||||
|
||||
- 320 x 568 and 390 x 844 compact phone profiles;
|
||||
- 768 x 1024 iPad portrait and 1024 x 768 iPad landscape;
|
||||
- 1280 x 720 and 1440 x 900 desktop controls;
|
||||
- long plans, long diffs, a range that starts below the fold, and rotation while adjusting.
|
||||
|
||||
Physical gate:
|
||||
|
||||
- iPhone Safari: browser chrome collapses while scrolling during adjustment; no native selection menu or magnifier appears; the keyboard stays closed until the user taps the textarea.
|
||||
- iPad Safari: finger and trackpad paths both remain usable; trackpad behavior does not inherit the compact touch composition when it is the primary pointer.
|
||||
- Code Review: selection remains painted after the composer returns and the submitted annotation spans the intended lines after reload.
|
||||
|
||||
## Next decision
|
||||
|
||||
Code Review has an approved direct gesture and no longer needs an endpoint-selection experiment. Physical Safari validation remains the release gate for its compact-touch transition. Plan Review still needs a separate direct-manipulation decision; the rejected **Extend selection** command must not return under different copy.
|
||||
|
||||
## Implementation evidence
|
||||
|
||||
- A shared routing policy distinguishes Pierre's range-completion gesture from its contextual gutter comment action.
|
||||
- File and All Files views use the same compact-touch rule: gesture completion preserves the selected range; the gutter action opens the incumbent composer.
|
||||
- The All Files route also commits the owning item and active file before publishing the range, so an equal-numbered range cannot remain painted on the previously active file.
|
||||
- Fine-pointer desktop retains its incumbent selection-to-composer transition.
|
||||
- Focused policy and rendered All Files lifecycle tests pass, including a desktop control. Root typecheck and both production single-file builds (`apps/review` and `apps/hook`) pass.
|
||||
- No Pierre option, unsafe CSS, Shadow DOM selector, gesture recognizer, package version, toolbar geometry, or composer geometry changed. Physical iPhone/iPad Safari remains the authority for the final gesture check.
|
||||
@@ -5,9 +5,9 @@
|
||||
import type { GuideViewerAssets } from "./guide-format";
|
||||
|
||||
export const GUIDE_VIEWER_MANIFEST: Omit<GuideViewerAssets, "baseUrl"> = {
|
||||
js: "viewer.sFtOnb1i.js",
|
||||
js: "viewer.BhU6ea2v.js",
|
||||
css: "viewer.Dtx2gmOp.css",
|
||||
jsIntegrity: "sha384-HRAx6JL8MwurSiEBSingzilCiL9O4o93I9G45am9wtXscw419eSvXAontQL/TkHd",
|
||||
jsIntegrity: "sha384-gtNYVWIHVgo9erfFEu8RspdmcaRUrVhz0McgJacIXvD0ZvUOu4QWsDr8Nv5uSCdy",
|
||||
cssIntegrity: "sha384-jbTLuO1urA+7R65nS8yRrbSr13BuYyfeSZBvNMTAJuJCcmJVWxw6F3R0C+735ihs",
|
||||
langs: {
|
||||
"astro": "chunks/astro.BykyiR6i.js",
|
||||
|
||||
@@ -1,12 +1,14 @@
|
||||
import { afterAll, afterEach, describe, expect, mock, test } from 'bun:test';
|
||||
import React, { act, useCallback, useEffect, useImperativeHandle, useRef } from 'react';
|
||||
import React, { act, useCallback, useEffect, useImperativeHandle, useRef, useState } from 'react';
|
||||
import { createRoot, type Root } from 'react-dom/client';
|
||||
import type { SelectedLineRange } from '@plannotator/ui/types';
|
||||
import type { DiffFile } from '../types';
|
||||
|
||||
let codeViewMounts = 0;
|
||||
let codeViewUnmounts = 0;
|
||||
let scrollTargets: Array<Record<string, unknown>> = [];
|
||||
let lastCodeViewProps: Record<string, unknown> | null = null;
|
||||
let toolbarSelections: Array<SelectedLineRange | null> = [];
|
||||
|
||||
// Captured BEFORE the mocks below replace the specifiers, so this file can put
|
||||
// the real modules back when it is done. `mock.module` is process global and
|
||||
@@ -103,7 +105,15 @@ mock.module('@pierre/diffs/react', () => ({
|
||||
}));
|
||||
|
||||
mock.module('./ToolbarHost', () => ({
|
||||
ToolbarHost: React.forwardRef(function MockToolbarHost() {
|
||||
ToolbarHost: React.forwardRef(function MockToolbarHost(_props, ref) {
|
||||
useImperativeHandle(ref, () => ({
|
||||
handleLineSelectionEnd: (range: SelectedLineRange | null) => {
|
||||
toolbarSelections.push(range);
|
||||
},
|
||||
openLineAnnotation: () => {},
|
||||
handleTokenClick: () => {},
|
||||
startEdit: () => {},
|
||||
}));
|
||||
return null;
|
||||
}),
|
||||
}));
|
||||
@@ -159,6 +169,7 @@ afterEach(async () => {
|
||||
codeViewUnmounts = 0;
|
||||
scrollTargets = [];
|
||||
lastCodeViewProps = null;
|
||||
toolbarSelections = [];
|
||||
});
|
||||
|
||||
// Hand the real @pierre/diffs back to the process. Only the two library
|
||||
@@ -226,6 +237,95 @@ describe('AllFilesCodeView guide mount state', () => {
|
||||
|
||||
});
|
||||
|
||||
describe('AllFilesCodeView compact-touch line selection', () => {
|
||||
// Stands in for App: a published range comes straight back down as
|
||||
// `pendingSelection`. That loop is load-bearing — CodeView's selection is
|
||||
// controlled, and the reconcile effect clears the highlight whenever
|
||||
// pendingSelection is null, so a statically-null prop would wipe the range
|
||||
// the preserve branch just painted.
|
||||
function Harness({ compactTouchLayout, onSelection }: {
|
||||
compactTouchLayout: boolean;
|
||||
onSelection?: (range: SelectedLineRange | null) => void;
|
||||
}) {
|
||||
const [pendingSelection, setPendingSelection] = useState<SelectedLineRange | null>(null);
|
||||
return view({
|
||||
compactTouchLayout,
|
||||
pendingSelection,
|
||||
onLineSelection: (range) => {
|
||||
onSelection?.(range);
|
||||
setPendingSelection(range);
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
async function mount(
|
||||
compactTouchLayout: boolean,
|
||||
onSelection?: (range: SelectedLineRange | null) => void,
|
||||
) {
|
||||
host = document.createElement('div');
|
||||
host.style.height = '400px';
|
||||
document.body.appendChild(host);
|
||||
root = createRoot(host);
|
||||
await act(async () => {
|
||||
root!.render(<Harness compactTouchLayout={compactTouchLayout} onSelection={onSelection} />);
|
||||
await new Promise((resolve) => setTimeout(resolve, 25));
|
||||
});
|
||||
}
|
||||
|
||||
function getSelectionCallbacks() {
|
||||
const options = lastCodeViewProps?.options as {
|
||||
onLineSelectionEnd?: (
|
||||
range: SelectedLineRange | null,
|
||||
context: { item: { id: string; type: 'diff' } },
|
||||
) => void;
|
||||
onGutterUtilityClick?: (
|
||||
range: SelectedLineRange,
|
||||
context: { item: { id: string; type: 'diff' } },
|
||||
) => void;
|
||||
};
|
||||
const item = (lastCodeViewProps?.initialItems as Array<{ id: string; type: 'diff' }>)[0];
|
||||
return { options, item };
|
||||
}
|
||||
|
||||
test.skipIf(!hasDom)('preserves a dragged range, then opens the composer from the gutter action', async () => {
|
||||
const observedSelections: Array<SelectedLineRange | null> = [];
|
||||
const range: SelectedLineRange = { start: 4, end: 8, side: 'additions' };
|
||||
await mount(true, (selection) => observedSelections.push(selection));
|
||||
const { options, item } = getSelectionCallbacks();
|
||||
|
||||
await act(async () => {
|
||||
options.onLineSelectionEnd?.(range, { item });
|
||||
});
|
||||
|
||||
expect(observedSelections.at(-1)).toEqual(range);
|
||||
expect(toolbarSelections).toEqual([]);
|
||||
// Publishing the range upward is only half of "preserved": CodeView's
|
||||
// selection is controlled here, so the highlight only survives if the range
|
||||
// is also handed back down. Without this the composer would stay shut on a
|
||||
// range nothing paints.
|
||||
expect(lastCodeViewProps?.selectedLines).toEqual({ id: item.id, range });
|
||||
|
||||
await act(async () => {
|
||||
options.onGutterUtilityClick?.(range, { item });
|
||||
});
|
||||
|
||||
expect(toolbarSelections).toEqual([range]);
|
||||
});
|
||||
|
||||
test.skipIf(!hasDom)('keeps the incumbent desktop selection-to-composer transition', async () => {
|
||||
const range: SelectedLineRange = { start: 4, end: 8, side: 'additions' };
|
||||
await mount(false);
|
||||
const { options, item } = getSelectionCallbacks();
|
||||
|
||||
await act(async () => {
|
||||
options.onLineSelectionEnd?.(range, { item });
|
||||
await new Promise((resolve) => setTimeout(resolve, 0));
|
||||
});
|
||||
|
||||
expect(toolbarSelections).toEqual([range]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('AllFilesCodeView readOnly (portable guide host)', () => {
|
||||
// The portable Guided Review viewer renders this component with no server
|
||||
// and no review state behind it (decision record D2/D4). These guard the
|
||||
|
||||
@@ -30,6 +30,10 @@ import { buildCodeNavRequest } from '../utils/buildCodeNavRequest';
|
||||
import { getDiffSelection, getLineNumberFromNode, getSideFromNode } from '../utils/diffSelection';
|
||||
import { isContentConsistentWithPatch } from '../utils/patchConsistency';
|
||||
import { hashString } from '../utils/hashString';
|
||||
import {
|
||||
resolveLineSelectionBehavior,
|
||||
type LineSelectionSource,
|
||||
} from '../utils/lineSelectionBehavior';
|
||||
import { isContentlessBinaryPatch, isOversizedReviewStubPatch } from '@plannotator/shared/diff-paths';
|
||||
import { OversizedFileNotice } from './OversizedFileNotice';
|
||||
import { ToolbarHost, type ToolbarHostHandle } from './ToolbarHost';
|
||||
@@ -560,7 +564,12 @@ export const AllFilesCodeView: React.FC<AllFilesCodeViewProps> = ({
|
||||
// header-custom slot, no [data-title] element), so that rule is moot either
|
||||
// way — we keep `true` to be explicit that the built-in title is irrelevant
|
||||
// here (our FileHeader owns all header chrome).
|
||||
const pierreTheme = usePierreTheme({ fontFamily, fontSize, showFileHeader: true });
|
||||
const pierreTheme = usePierreTheme({
|
||||
fontFamily,
|
||||
fontSize,
|
||||
showFileHeader: true,
|
||||
compactTouchLayout,
|
||||
});
|
||||
// Worker-pool highlighting: wait for the pool so the first tokenization
|
||||
// wave runs in workers (not a main-thread fallback), and keep the pool's
|
||||
// theme pair in step with the UI theme.
|
||||
@@ -1820,25 +1829,41 @@ export const AllFilesCodeView: React.FC<AllFilesCodeViewProps> = ({
|
||||
}
|
||||
}, [activeFilePath, pendingSelection, filePathToItemId]);
|
||||
|
||||
const handleLineSelectionEnd = useStableCallback(
|
||||
(range: SelectedLineRange | null, item: CodeViewItem<DiffAnnotationMetadata>) => {
|
||||
const handleLineSelectionInteraction = useStableCallback(
|
||||
(
|
||||
source: LineSelectionSource,
|
||||
range: SelectedLineRange | null,
|
||||
item: CodeViewItem<DiffAnnotationMetadata>,
|
||||
) => {
|
||||
if (range == null || item.type !== 'diff') return;
|
||||
// The file being edited owns its pointer interactions — opening the
|
||||
// annotation toolbar over an active editor would fight its focus.
|
||||
if (item.id === editSession.editingItemIdRef.current) return;
|
||||
const filePath = itemIdToFilePath.get(item.id);
|
||||
if (filePath == null) return;
|
||||
if (resolveLineSelectionBehavior({
|
||||
source,
|
||||
compactTouchLayout: compactTouchLayout === true,
|
||||
}) === 'preserve-selection') {
|
||||
pendingToolbarRange.current = null;
|
||||
setActiveFilePath(filePath);
|
||||
setSelectedLines({ id: item.id, range });
|
||||
onLineSelection(range);
|
||||
return;
|
||||
}
|
||||
routeSelectionToToolbar(range, filePath);
|
||||
},
|
||||
);
|
||||
|
||||
const handleLineSelectionEnd = useStableCallback(
|
||||
(range: SelectedLineRange | null, item: CodeViewItem<DiffAnnotationMetadata>) => {
|
||||
handleLineSelectionInteraction('range-gesture', range, item);
|
||||
},
|
||||
);
|
||||
|
||||
const handleGutterUtilityClick = useStableCallback(
|
||||
(range: SelectedLineRange, item: CodeViewItem<DiffAnnotationMetadata>) => {
|
||||
if (item.type !== 'diff') return;
|
||||
if (item.id === editSession.editingItemIdRef.current) return;
|
||||
const filePath = itemIdToFilePath.get(item.id);
|
||||
if (filePath == null) return;
|
||||
routeSelectionToToolbar(range, filePath);
|
||||
handleLineSelectionInteraction('gutter-comment-action', range, item);
|
||||
},
|
||||
);
|
||||
|
||||
|
||||
@@ -0,0 +1,253 @@
|
||||
/**
|
||||
* Compact touch keeps a dragged line range on screen instead of opening the
|
||||
* composer, so the range has to survive as a PAINTED selection until the
|
||||
* reviewer taps Pierre's gutter comment button.
|
||||
*
|
||||
* The failures guarded here:
|
||||
* - a drag on compact touch force-opening the composer again (the incumbent
|
||||
* desktop behaviour, which is what made mobile range selection unusable);
|
||||
* - the preserved range never reaching Pierre, so nothing is highlighted;
|
||||
* - a SECOND drag not repainting. A non-null `selectedLines` puts Pierre in
|
||||
* controlled-selection mode, where `InteractionManager.updateSelection`
|
||||
* only records a proposal and leaves painting to the host — so without
|
||||
* `onLineSelectionChange` flowing back into app state the old highlight
|
||||
* stays put and the finger is untracked until release;
|
||||
* - a cleared selection being swallowed by the preserve branch and leaving an
|
||||
* open composer behind;
|
||||
* - any of it leaking onto desktop, which must keep handing every completed
|
||||
* drag straight to the toolbar host.
|
||||
*
|
||||
* DOM-gated (DOM_TESTS=1) and registered in .github/workflows/test.yml's
|
||||
* "Run UI seam-contract + DOM tests" step.
|
||||
*/
|
||||
import { afterAll, afterEach, describe, expect, mock, test } from 'bun:test';
|
||||
import React, { act, useCallback, useImperativeHandle, useState } from 'react';
|
||||
import { createRoot, type Root } from 'react-dom/client';
|
||||
import type { SelectedLineRange } from '@plannotator/ui/types';
|
||||
|
||||
interface CapturedFileDiffProps {
|
||||
selectedLines?: SelectedLineRange;
|
||||
options: {
|
||||
onLineSelectionEnd?: (range: SelectedLineRange | null) => void;
|
||||
onLineSelectionChange?: (range: SelectedLineRange | null) => void;
|
||||
onGutterUtilityClick?: (range: SelectedLineRange) => void;
|
||||
};
|
||||
}
|
||||
|
||||
let lastFileDiffProps: CapturedFileDiffProps | null = null;
|
||||
let toolbarSelections: Array<SelectedLineRange | null> = [];
|
||||
|
||||
// Spread-captured before the mock replaces the module record — a bare namespace
|
||||
// handle would be a live view of the record `mock.module` rewrites, so the
|
||||
// afterAll restore would silently re-install this file's stub as itself. Same
|
||||
// idiom (and the same reason) as AllFilesCodeView.lifecycle.test.tsx.
|
||||
const realPierreDiffsReact = { ...(await import('@pierre/diffs/react')) };
|
||||
const realResolveSyntaxTheme = (await import('@plannotator/ui/utils/syntaxTheme')).resolveSyntaxTheme;
|
||||
|
||||
mock.module('../workerPool', () => ({
|
||||
useIsWorkerPoolReadyOrDisabled: () => true,
|
||||
useWorkerPoolThemeSync: () => {},
|
||||
}));
|
||||
|
||||
mock.module('../hooks/usePierreTheme', () => ({
|
||||
buildLineBgOverrides: () => '',
|
||||
resolveSyntaxTheme: realResolveSyntaxTheme,
|
||||
usePierreTheme: () => ({ type: 'light', css: '' }),
|
||||
}));
|
||||
|
||||
// Only FileDiff is replaced; everything else the review-editor graph imports
|
||||
// from this specifier stays real.
|
||||
mock.module('@pierre/diffs/react', () => ({
|
||||
...realPierreDiffsReact,
|
||||
FileDiff: function MockFileDiff(props: CapturedFileDiffProps) {
|
||||
lastFileDiffProps = props;
|
||||
return null;
|
||||
},
|
||||
}));
|
||||
|
||||
mock.module('./ToolbarHost', () => ({
|
||||
ToolbarHost: React.forwardRef(function MockToolbarHost(_props, ref) {
|
||||
useImperativeHandle(ref, () => ({
|
||||
handleLineSelectionEnd: (range: SelectedLineRange | null) => {
|
||||
toolbarSelections.push(range);
|
||||
},
|
||||
openLineAnnotation: () => {},
|
||||
handleTokenClick: () => {},
|
||||
startEdit: () => {},
|
||||
}));
|
||||
return null;
|
||||
}),
|
||||
}));
|
||||
|
||||
const { DiffViewer } = await import('./DiffViewer');
|
||||
|
||||
const hasDom = typeof document !== 'undefined';
|
||||
|
||||
const PATCH = [
|
||||
'diff --git a/calc.ts b/calc.ts',
|
||||
'index 0000000..1111111 100644',
|
||||
'--- a/calc.ts',
|
||||
'+++ b/calc.ts',
|
||||
'@@ -1,3 +1,3 @@',
|
||||
' const a = 1;',
|
||||
'-const b = 1;',
|
||||
'+const b = 2;',
|
||||
' const c = 3;',
|
||||
'',
|
||||
].join('\n');
|
||||
|
||||
const FIRST_DRAG: SelectedLineRange = { start: 2, end: 3, side: 'additions' };
|
||||
const SECOND_DRAG: SelectedLineRange = { start: 1, end: 3, side: 'additions' };
|
||||
|
||||
/** Stands in for App: mirrors published selections back down as
|
||||
* `pendingSelection`, which is the loop the repaint depends on. */
|
||||
function Harness({ compactTouchLayout, onSelection }: {
|
||||
compactTouchLayout: boolean;
|
||||
onSelection: (range: SelectedLineRange | null) => void;
|
||||
}) {
|
||||
const [pendingSelection, setPendingSelection] = useState<SelectedLineRange | null>(null);
|
||||
const handleLineSelection = useCallback((range: SelectedLineRange | null) => {
|
||||
onSelection(range);
|
||||
setPendingSelection(range);
|
||||
}, [onSelection]);
|
||||
return (
|
||||
<DiffViewer
|
||||
patch={PATCH}
|
||||
filePath="calc.ts"
|
||||
diffStyle="unified"
|
||||
annotations={[]}
|
||||
selectedAnnotationId={null}
|
||||
scrollTargetAnnotation={null}
|
||||
pendingSelection={pendingSelection}
|
||||
compactTouchLayout={compactTouchLayout}
|
||||
onLineSelection={handleLineSelection}
|
||||
onAddAnnotation={() => {}}
|
||||
onAddFileComment={() => {}}
|
||||
onEditAnnotation={() => {}}
|
||||
onSelectAnnotation={() => {}}
|
||||
onDeleteAnnotation={() => {}}
|
||||
/>
|
||||
);
|
||||
}
|
||||
|
||||
describe.if(hasDom)('DiffViewer compact-touch line selection (DOM)', () => {
|
||||
let root: Root | null = null;
|
||||
let host: HTMLDivElement | null = null;
|
||||
const originalFetch = globalThis.fetch;
|
||||
|
||||
async function mount(compactTouchLayout: boolean) {
|
||||
globalThis.fetch = (async () =>
|
||||
new Response(JSON.stringify({ oldContent: null, newContent: null }), {
|
||||
headers: { 'content-type': 'application/json' },
|
||||
})) as typeof fetch;
|
||||
const selections: Array<SelectedLineRange | null> = [];
|
||||
host = document.createElement('div');
|
||||
document.body.appendChild(host);
|
||||
root = createRoot(host);
|
||||
await act(async () => {
|
||||
root!.render(
|
||||
<Harness
|
||||
compactTouchLayout={compactTouchLayout}
|
||||
onSelection={(range) => selections.push(range)}
|
||||
/>,
|
||||
);
|
||||
await new Promise((resolve) => setTimeout(resolve, 25));
|
||||
});
|
||||
return selections;
|
||||
}
|
||||
|
||||
function pierre() {
|
||||
if (lastFileDiffProps == null) throw new Error('FileDiff never rendered');
|
||||
return lastFileDiffProps;
|
||||
}
|
||||
|
||||
afterEach(async () => {
|
||||
if (root) {
|
||||
await act(async () => root!.unmount());
|
||||
root = null;
|
||||
}
|
||||
host?.remove();
|
||||
host = null;
|
||||
lastFileDiffProps = null;
|
||||
toolbarSelections = [];
|
||||
globalThis.fetch = originalFetch;
|
||||
});
|
||||
|
||||
afterAll(() => {
|
||||
mock.module('@pierre/diffs/react', () => realPierreDiffsReact);
|
||||
});
|
||||
|
||||
test('a drag preserves the range instead of opening the composer', async () => {
|
||||
const selections = await mount(true);
|
||||
|
||||
await act(async () => {
|
||||
pierre().options.onLineSelectionEnd?.(FIRST_DRAG);
|
||||
});
|
||||
|
||||
expect(toolbarSelections).toEqual([]);
|
||||
expect(selections.at(-1)).toEqual(FIRST_DRAG);
|
||||
// The range has to come back down as a painted selection, not just be
|
||||
// published: this prop is the only thing Pierre highlights from.
|
||||
expect(pierre().selectedLines).toEqual(FIRST_DRAG);
|
||||
});
|
||||
|
||||
test('a second drag repaints while the first range is still preserved', async () => {
|
||||
await mount(true);
|
||||
|
||||
await act(async () => {
|
||||
pierre().options.onLineSelectionEnd?.(FIRST_DRAG);
|
||||
});
|
||||
expect(pierre().selectedLines).toEqual(FIRST_DRAG);
|
||||
|
||||
// Pierre is controlled now, so in-flight deltas of the next drag arrive on
|
||||
// onLineSelectionChange and repaint nothing on their own.
|
||||
const onChange = pierre().options.onLineSelectionChange;
|
||||
expect(onChange).toBeDefined();
|
||||
await act(async () => {
|
||||
onChange?.(SECOND_DRAG);
|
||||
});
|
||||
|
||||
expect(pierre().selectedLines).toEqual(SECOND_DRAG);
|
||||
expect(toolbarSelections).toEqual([]);
|
||||
});
|
||||
|
||||
test('the gutter comment action opens the composer for the preserved range', async () => {
|
||||
await mount(true);
|
||||
|
||||
await act(async () => {
|
||||
pierre().options.onLineSelectionEnd?.(FIRST_DRAG);
|
||||
pierre().options.onGutterUtilityClick?.(FIRST_DRAG);
|
||||
});
|
||||
|
||||
expect(toolbarSelections).toEqual([FIRST_DRAG]);
|
||||
});
|
||||
|
||||
test('a cleared selection still reaches the toolbar host so an open composer closes', async () => {
|
||||
await mount(true);
|
||||
|
||||
await act(async () => {
|
||||
pierre().options.onLineSelectionEnd?.(FIRST_DRAG);
|
||||
});
|
||||
await act(async () => {
|
||||
pierre().options.onLineSelectionEnd?.(null);
|
||||
});
|
||||
|
||||
// The real host clears its toolbar state (and republishes the null
|
||||
// selection) from here; swallowing the null in the preserve branch would
|
||||
// leave the composer open over a range that no longer exists.
|
||||
expect(toolbarSelections).toEqual([null]);
|
||||
});
|
||||
|
||||
test('desktop keeps handing completed drags straight to the composer', async () => {
|
||||
await mount(false);
|
||||
|
||||
await act(async () => {
|
||||
pierre().options.onLineSelectionEnd?.(FIRST_DRAG);
|
||||
});
|
||||
|
||||
expect(toolbarSelections).toEqual([FIRST_DRAG]);
|
||||
// Desktop never enters the preserved-range state, so it must not even carry
|
||||
// the controlled-repaint handler.
|
||||
expect('onLineSelectionChange' in pierre().options).toBe(false);
|
||||
});
|
||||
});
|
||||
@@ -34,6 +34,10 @@ import {
|
||||
retryScrollToSearchMatch,
|
||||
swapActiveSearchHighlight,
|
||||
} from '../utils/reviewSearchHighlight';
|
||||
import {
|
||||
resolveLineSelectionBehavior,
|
||||
type LineSelectionSource,
|
||||
} from '../utils/lineSelectionBehavior';
|
||||
|
||||
interface PierreDiffContentProps {
|
||||
filePath: string;
|
||||
@@ -49,6 +53,10 @@ interface PierreDiffContentProps {
|
||||
mergedAnnotations: DiffLineAnnotation<DiffAnnotationMetadata>[];
|
||||
pendingSelection: SelectedLineRange | null;
|
||||
onLineSelectionEnd: (range: SelectedLineRange | null) => void;
|
||||
/** In-flight selection deltas. Only wired when Pierre needs the host to
|
||||
* repaint (see the options block below); undefined leaves the option off
|
||||
* the object entirely. */
|
||||
onLineSelectionChange?: (range: SelectedLineRange | null) => void;
|
||||
onGutterUtilityClick: (range: SelectedLineRange) => void;
|
||||
renderAnnotation: (annotation: { side: string; lineNumber: number; metadata?: DiffAnnotationMetadata }) => React.ReactNode;
|
||||
onTokenClick?: (props: DiffTokenEventBaseProps, event: MouseEvent) => void;
|
||||
@@ -70,6 +78,7 @@ const PierreDiffContent = React.memo(({
|
||||
mergedAnnotations,
|
||||
pendingSelection,
|
||||
onLineSelectionEnd,
|
||||
onLineSelectionChange,
|
||||
onGutterUtilityClick,
|
||||
renderAnnotation,
|
||||
onTokenClick,
|
||||
@@ -99,6 +108,13 @@ const PierreDiffContent = React.memo(({
|
||||
enableGutterUtility: true,
|
||||
onGutterUtilityClick,
|
||||
onLineSelectionEnd,
|
||||
// A defined `selectedLines` prop puts Pierre in controlled-selection
|
||||
// mode, where `InteractionManager.updateSelection` only stores a
|
||||
// proposed range and leaves the painted highlight to whatever the host
|
||||
// hands back. Without a change handler a second drag therefore never
|
||||
// repaints. Spread conditionally so surfaces that don't need it keep an
|
||||
// options object with no such key at all.
|
||||
...(onLineSelectionChange ? { onLineSelectionChange } : {}),
|
||||
// Pierre's renderer-options builder drops onToken* before it evaluates
|
||||
// shouldUseTokenTransformer, so passing the handlers alone never wraps
|
||||
// tokens (no data-char) and code-nav/token events never fire. Enable
|
||||
@@ -130,6 +146,7 @@ const PierreDiffContent = React.memo(({
|
||||
prev.mergedAnnotations === next.mergedAnnotations &&
|
||||
prev.pendingSelection === next.pendingSelection &&
|
||||
prev.onLineSelectionEnd === next.onLineSelectionEnd &&
|
||||
prev.onLineSelectionChange === next.onLineSelectionChange &&
|
||||
prev.onGutterUtilityClick === next.onGutterUtilityClick &&
|
||||
prev.renderAnnotation === next.renderAnnotation &&
|
||||
prev.onTokenClick === next.onTokenClick &&
|
||||
@@ -164,6 +181,8 @@ interface DiffViewerProps {
|
||||
selectedAnnotationId: string | null;
|
||||
scrollTargetAnnotation: AnnotationScrollTarget | null;
|
||||
pendingSelection: SelectedLineRange | null;
|
||||
/** Compact coarse-pointer shell. Keeps range selection separate from writing. */
|
||||
compactTouchLayout?: boolean;
|
||||
onLineSelection: (range: SelectedLineRange | null) => void;
|
||||
onAddAnnotation: (type: CodeAnnotationType, text?: string, suggestedCode?: string, originalCode?: string, conventionalLabel?: ConventionalLabel, decorations?: ConventionalDecoration[], tokenMeta?: TokenAnnotationMeta) => void;
|
||||
onAddFileComment: (text: string) => void;
|
||||
@@ -224,6 +243,7 @@ export const DiffViewer: React.FC<DiffViewerProps> = ({
|
||||
selectedAnnotationId,
|
||||
scrollTargetAnnotation,
|
||||
pendingSelection,
|
||||
compactTouchLayout = false,
|
||||
onLineSelection,
|
||||
onAddAnnotation,
|
||||
onAddFileComment,
|
||||
@@ -254,7 +274,7 @@ export const DiffViewer: React.FC<DiffViewerProps> = ({
|
||||
aiHistoryMessages = [],
|
||||
onCodeNavRequest,
|
||||
}) => {
|
||||
const pierreTheme = usePierreTheme({ fontFamily, fontSize });
|
||||
const pierreTheme = usePierreTheme({ fontFamily, fontSize, compactTouchLayout });
|
||||
// Worker-pool highlighting: keep the pool's theme pair in step with the UI
|
||||
// theme. (No mount gating here — the single-file panel renders one diff;
|
||||
// a main-thread fallback frame at startup is invisible.)
|
||||
@@ -622,9 +642,38 @@ export const DiffViewer: React.FC<DiffViewerProps> = ({
|
||||
);
|
||||
}, [filePath, selectedAnnotationId, onSelectAnnotation, handleEdit, onDeleteAnnotation, onClickAIMarker]);
|
||||
|
||||
const handleGutterUtilityClick = useCallback((range: SelectedLineRange) => {
|
||||
const handleLineSelectionInteraction = useCallback((
|
||||
source: LineSelectionSource,
|
||||
range: SelectedLineRange | null,
|
||||
) => {
|
||||
// A cleared selection is never something to preserve. AllFilesCodeView
|
||||
// early-returns on a null range; single-file has to route it to the toolbar
|
||||
// host so an open composer (including the Ask AI window) closes with it —
|
||||
// that call also publishes the null selection upwards.
|
||||
if (range == null) {
|
||||
toolbarHostRef.current?.handleLineSelectionEnd(null);
|
||||
return;
|
||||
}
|
||||
if (resolveLineSelectionBehavior({ source, compactTouchLayout }) === 'preserve-selection') {
|
||||
onLineSelection(range);
|
||||
return;
|
||||
}
|
||||
toolbarHostRef.current?.handleLineSelectionEnd(range);
|
||||
}, []);
|
||||
}, [compactTouchLayout, onLineSelection]);
|
||||
|
||||
// Compact touch keeps a dragged range on screen instead of opening the
|
||||
// composer, so `pendingSelection` is non-null for the whole time the reviewer
|
||||
// may drag again — and a non-null `selectedLines` is exactly what puts Pierre
|
||||
// in controlled-selection mode. Feed the in-flight range back so the second
|
||||
// drag repaints and the finger stays tracked. Desktop never enters that state
|
||||
// through a preserved range, and gets no handler at all.
|
||||
const handlePierreLineSelectionChange = useCallback((range: SelectedLineRange | null) => {
|
||||
onLineSelection(range);
|
||||
}, [onLineSelection]);
|
||||
|
||||
const handleGutterUtilityClick = useCallback((range: SelectedLineRange) => {
|
||||
handleLineSelectionInteraction('gutter-comment-action', range);
|
||||
}, [handleLineSelectionInteraction]);
|
||||
|
||||
useEffect(() => {
|
||||
const root = diffContentRef.current;
|
||||
@@ -651,8 +700,8 @@ export const DiffViewer: React.FC<DiffViewerProps> = ({
|
||||
}, []);
|
||||
|
||||
const handlePierreLineSelectionEnd = useCallback((range: SelectedLineRange | null) => {
|
||||
toolbarHostRef.current?.handleLineSelectionEnd(range);
|
||||
}, []);
|
||||
handleLineSelectionInteraction('range-gesture', range);
|
||||
}, [handleLineSelectionInteraction]);
|
||||
|
||||
// Token interaction handlers (code area clicks)
|
||||
const handleTokenClick = useCallback((props: DiffTokenEventBaseProps, event: MouseEvent) => {
|
||||
@@ -788,6 +837,7 @@ export const DiffViewer: React.FC<DiffViewerProps> = ({
|
||||
mergedAnnotations={mergedAnnotations}
|
||||
pendingSelection={pendingSelection ?? selectedAnnotationRange}
|
||||
onLineSelectionEnd={handlePierreLineSelectionEnd}
|
||||
onLineSelectionChange={compactTouchLayout ? handlePierreLineSelectionChange : undefined}
|
||||
onGutterUtilityClick={handleGutterUtilityClick}
|
||||
renderAnnotation={renderAnnotation}
|
||||
onTokenClick={handleTokenClick}
|
||||
|
||||
@@ -89,6 +89,7 @@ export const ReviewDiffPanel: React.FC<IDockviewPanelProps> = (props) => {
|
||||
selectedAnnotationId={state.selectedAnnotationId}
|
||||
scrollTargetAnnotation={state.scrollTargetAnnotation}
|
||||
pendingSelection={state.pendingSelection}
|
||||
compactTouchLayout={state.isCompactTouchLayout}
|
||||
onLineSelection={state.onLineSelection}
|
||||
onAddAnnotation={state.onAddAnnotation}
|
||||
onAddFileComment={state.onAddFileComment}
|
||||
|
||||
@@ -169,11 +169,46 @@ export function buildLineBgOverrides(intensity: DiffLineBgIntensity, mode: 'ligh
|
||||
`;
|
||||
}
|
||||
|
||||
export function usePierreTheme(options?: { fontFamily?: string; fontSize?: string; showFileHeader?: boolean }): PierreTheme {
|
||||
/**
|
||||
* Pierre's gutter comment button (`[data-utility-button]`) is `1lh` square —
|
||||
* 20px at the default line height — plus a 4px leftward bleed on its invisible
|
||||
* `::before`. On compact touch that button is the ONLY way to open the composer
|
||||
* for a preserved range, so it has to meet the same 44px standard
|
||||
* `[data-pn-touch-target]` enforces everywhere else in the shell.
|
||||
*
|
||||
* The glyph keeps its size; only the `::before` hit area grows, centred on the
|
||||
* button, to `max(44px, its previous size)`. `--pn-touch-target` is a custom
|
||||
* property on `:root` and custom properties inherit across the shadow boundary,
|
||||
* so the token still drives the number inside Pierre's shadow DOM.
|
||||
*
|
||||
* Injected conditionally rather than through the `html:has([data-pn-compact-
|
||||
* touch-layout])` gate: this CSS is applied INSIDE Pierre's shadow root, where
|
||||
* a selector rooted at `html` matches nothing. A `@media (pointer: coarse)`
|
||||
* query is not a substitute either — the shell's compact classification is
|
||||
* deliberately not "any coarse pointer is present", so a desktop with a
|
||||
* touchscreen must not pick this up.
|
||||
*/
|
||||
const COMPACT_TOUCH_GUTTER_UTILITY_CSS = `
|
||||
[data-utility-button]::before {
|
||||
inset: 50% auto auto 50%;
|
||||
width: max(var(--pn-touch-target, 2.75rem), calc(100% + 4px));
|
||||
height: max(var(--pn-touch-target, 2.75rem), 100%);
|
||||
transform: translate(-50%, -50%);
|
||||
}
|
||||
`;
|
||||
|
||||
export function usePierreTheme(options?: {
|
||||
fontFamily?: string;
|
||||
fontSize?: string;
|
||||
showFileHeader?: boolean;
|
||||
compactTouchLayout?: boolean;
|
||||
}): PierreTheme {
|
||||
const { colorTheme, resolvedMode } = useTheme();
|
||||
const fontFamily = options?.fontFamily;
|
||||
const fontSize = options?.fontSize;
|
||||
const showFileHeader = options?.showFileHeader ?? false;
|
||||
const compactTouchLayout = options?.compactTouchLayout === true;
|
||||
const compactTouchCSS = compactTouchLayout ? COMPACT_TOUCH_GUTTER_UTILITY_CSS : '';
|
||||
const lineBgIntensity = useConfigValue('diffLineBgIntensity');
|
||||
|
||||
const [pierreTheme, setPierreTheme] = useState<PierreTheme>(() => {
|
||||
@@ -191,6 +226,7 @@ export function usePierreTheme(options?: { fontFamily?: string; fontSize?: strin
|
||||
[data-separator='line-info'], [data-separator='line-info-basic'] { height: 24px !important; }
|
||||
[data-separator='line-info'] { margin-block: 4px !important; }
|
||||
${buildLineBgOverrides(lineBgIntensity, resolvedMode ?? 'dark')}
|
||||
${compactTouchCSS}
|
||||
`};
|
||||
});
|
||||
|
||||
@@ -297,10 +333,12 @@ export function usePierreTheme(options?: { fontFamily?: string; fontSize?: strin
|
||||
${fontCSS}
|
||||
|
||||
${buildLineBgOverrides(lineBgIntensity, resolvedMode)}
|
||||
|
||||
${compactTouchCSS}
|
||||
`,
|
||||
});
|
||||
});
|
||||
}, [resolvedMode, colorTheme, fontFamily, fontSize, showFileHeader, lineBgIntensity]);
|
||||
}, [resolvedMode, colorTheme, fontFamily, fontSize, showFileHeader, lineBgIntensity, compactTouchCSS]);
|
||||
|
||||
return pierreTheme;
|
||||
}
|
||||
|
||||
@@ -0,0 +1,25 @@
|
||||
import { describe, expect, test } from 'bun:test';
|
||||
import { resolveLineSelectionBehavior } from './lineSelectionBehavior';
|
||||
|
||||
describe('resolveLineSelectionBehavior', () => {
|
||||
test('preserves a completed range gesture in the compact touch shell', () => {
|
||||
expect(resolveLineSelectionBehavior({
|
||||
source: 'range-gesture',
|
||||
compactTouchLayout: true,
|
||||
})).toBe('preserve-selection');
|
||||
});
|
||||
|
||||
test('opens the composer from the explicit gutter action on compact touch', () => {
|
||||
expect(resolveLineSelectionBehavior({
|
||||
source: 'gutter-comment-action',
|
||||
compactTouchLayout: true,
|
||||
})).toBe('open-composer');
|
||||
});
|
||||
|
||||
test('preserves the incumbent desktop selection-to-composer behavior', () => {
|
||||
expect(resolveLineSelectionBehavior({
|
||||
source: 'range-gesture',
|
||||
compactTouchLayout: false,
|
||||
})).toBe('open-composer');
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,23 @@
|
||||
export type LineSelectionSource = 'range-gesture' | 'gutter-comment-action';
|
||||
|
||||
export type LineSelectionBehavior = 'preserve-selection' | 'open-composer';
|
||||
|
||||
interface ResolveLineSelectionBehaviorOptions {
|
||||
readonly source: LineSelectionSource;
|
||||
readonly compactTouchLayout: boolean;
|
||||
}
|
||||
|
||||
/**
|
||||
* Keep mobile range acquisition separate from writing, matching DiffsHub.
|
||||
* Desktop retains its incumbent selection-to-composer flow, while the
|
||||
* contextual gutter action always represents an explicit writing intent.
|
||||
*/
|
||||
export function resolveLineSelectionBehavior({
|
||||
source,
|
||||
compactTouchLayout,
|
||||
}: ResolveLineSelectionBehaviorOptions): LineSelectionBehavior {
|
||||
if (compactTouchLayout && source === 'range-gesture') {
|
||||
return 'preserve-selection';
|
||||
}
|
||||
return 'open-composer';
|
||||
}
|
||||
Reference in New Issue
Block a user