Review pane behavior on document switch #57

Open
opened 2026-06-14 00:47:28 +00:00 by benstull · 0 comments
Owner

Review pane behavior on document switch

Summary

Deliberately decide — and implement — what happens to the review pane when the
active editor switches documents: the fate of the outgoing document's review
pane, and whether the switched-to document gets a review experience at all.

Problem / pain

The review panel's behavior on document switch is incidental, not designed.
The panel is one-per-document, keyed by URI (src/trackChangesPreview.ts:45),
and the controller registers no onDidChangeActiveTextEditor listener. So
today, when you switch the active editor to a different document:

  • the open review pane stays pinned to its original document (it does not
    follow you), and
  • the document you switched to gets no review experience — you must
    explicitly re-invoke cowriting.showTrackChangesPreview on it.

That landing was never a decision — it's just where the absence of a listener
left things. As the review pane becomes the primary interaction surface (F11,
#43), this navigation gap becomes user-visible friction: open a review pane,
click into another markdown file to compare or copy something, and the pane is
now silently showing the wrong document — or you have two panes and no clear
rule for which is "live." There is no defined, predictable model for how the
review experience tracks the document you're actually looking at.

Who feels it

The coauthor (the human writer using the plugin) — anyone working across
more than one markdown document in a session, navigating between files while a
review pane is open.

Desired outcome / value

A predictable, deliberate rule for how the review experience relates to the
active document, so that switching documents never leaves the writer looking at
a review pane for the wrong file or wondering whether the new file is "covered."
The review surface behaves coherently as the writer navigates a multi-document
workspace.

Acceptance — "done right"

  • The behavior on document switch is a documented decision (recorded in the
    relevant spec / design), not an accident of which listeners happen to exist.
  • Both coupled questions are answered and implemented:
    1. Outgoing pane — what happens to the review pane of the document you
      switch away from (stays pinned / detaches / closes / retargets).
    2. Incoming doc — whether the switched-to document automatically gets a
      review experience (auto-open the pane and/or auto-establish review state)
      or stays inert until the user explicitly asks.
  • The chosen behavior is consistent across the entry points that open the pane
    (right-click menu #41, toolbar #43, command/keybinding) and across authorable
    vs. non-authorable / non-markdown documents.
  • Switching to a non-markdown or non-authorable document does something sane and
    defined (e.g. the review pane does not try to review a file it cannot review).
  • E2E browser coverage exercises the switch behavior (open pane on doc A, switch
    to doc B, assert the defined outcome for both A's pane and B).

Scope / non-goals

In scope: the lifecycle/targeting rule for the review pane (the
TrackChangesPreviewController) as the active editor changes between documents,
and whether/how review state is established for the newly-active document.

Non-goals:

  • Scroll-sync between editor and pane (that is #32 — a different concern).
  • Changing what the review experience renders (attribution, proposals, diff)
    — this is about which document the pane targets and when a review exists,
    not the rendering.
  • Multi-pane layout management beyond the single targeting rule (no new
    split/grid features).

Assumptions · constraints · dependencies

  • The review pane today is per-document (Map<uriString, WebviewPanel>,
    src/trackChangesPreview.ts:45); whatever rule is chosen must reconcile with
    that (e.g. "follow active editor" may imply collapsing toward a single live
    pane, or revealing the per-doc pane).
  • Per-document review state (F6 baseline, F4 proposals, F3 attribution) already
    auto-creates on demand when a pane is shown for a doc
    (diffViewController.ts ensureBaseline, proposalController.ts per-doc
    state) — an "auto-review on switch" option leans on that; a "do nothing"
    option must avoid creating state for files the user never asked to review.
  • Markdown-only product: non-markdown / non-authorable docs (isAuthorable)
    must be handled explicitly.
  • Related (not blocking): #41 (right-click → Open Review Panel), #43 / F11
    (preview toolbar as primary surface), #32 (scroll-sync). This sharpens the
    same surface those touch.

Source / signal

Self-originated — capture session vscode-cowriting-plugin-0050
(2026-06-13). Dev observation while working the review-panel surface: "When
changing documents, we should decide what happens to the review pane of the
current document, and if the document switched to should get a review experience
or not." Current behavior confirmed by reading
src/trackChangesPreview.ts (no onDidChangeActiveTextEditor listener; panel
stays pinned).

Priority (WSJF)

Provisional: Value 5 · Time-criticality 3 · Opportunity-enablement 3 ÷ Size 3 =
~3.7. priority/P2 — real navigation friction and a visible incoherence as the
pane becomes primary, but not blocking the inner-loop triad; worth doing once
the decision is made.

Decomposition

Feature → design-then-build slices (one increment):

  • Decide the two coupled rules (outgoing pane fate; incoming-doc review
    experience) in a short design, with the option set below evaluated.
  • Implement the active-editor-change handling in
    TrackChangesPreviewController per the decision.
  • Handle non-markdown / non-authorable active documents explicitly.
  • E2E coverage of the switch behavior (doc A pane + switch to doc B).

Solution notes (optional — NOT a design)

Non-binding option space for the design to choose among:

Outgoing pane on switch away:

  • Stay pinned (today) — pane keeps showing doc A; predictable but can be
    stale/confusing.
  • Retarget/follow — the live pane re-points to the now-active doc B.
  • Close/hide — collapse the pane when you leave its document.

Incoming (switched-to) document:

  • Auto-open a review experience — show the pane and establish review state for
    doc B automatically (leans on existing on-demand state creation).
  • On-demand only — do nothing until the user explicitly opens review for B.
  • Conditional — auto-review only if the pane was already open (i.e. "follow
    mode" is implicitly on once you've opened one), inert otherwise.

A likely-coherent default to test against: a single "live" review pane that
follows the active markdown editor
once opened, leaving non-markdown docs
untouched and not fabricating review state for files the user never opened a
pane on — but this is for the design to settle, not pre-decided here.

<!-- Set the type/* label: type/feature --> # Review pane behavior on document switch ## Summary Deliberately decide — and implement — what happens to the review pane when the active editor switches documents: the fate of the **outgoing** document's review pane, and whether the **switched-to** document gets a review experience at all. ## Problem / pain The review panel's behavior on document switch is **incidental, not designed**. The panel is one-per-document, keyed by URI (`src/trackChangesPreview.ts:45`), and the controller registers **no** `onDidChangeActiveTextEditor` listener. So today, when you switch the active editor to a different document: - the open review pane **stays pinned** to its original document (it does not follow you), and - the document you switched **to** gets **no review experience** — you must explicitly re-invoke `cowriting.showTrackChangesPreview` on it. That landing was never a decision — it's just where the absence of a listener left things. As the review pane becomes the primary interaction surface (F11, #43), this navigation gap becomes user-visible friction: open a review pane, click into another markdown file to compare or copy something, and the pane is now silently showing the wrong document — or you have two panes and no clear rule for which is "live." There is no defined, predictable model for how the review experience tracks the document you're actually looking at. ## Who feels it The **coauthor** (the human writer using the plugin) — anyone working across more than one markdown document in a session, navigating between files while a review pane is open. ## Desired outcome / value A **predictable, deliberate** rule for how the review experience relates to the active document, so that switching documents never leaves the writer looking at a review pane for the wrong file or wondering whether the new file is "covered." The review surface behaves coherently as the writer navigates a multi-document workspace. ## Acceptance — "done right" - The behavior on document switch is a **documented decision** (recorded in the relevant spec / design), not an accident of which listeners happen to exist. - Both coupled questions are answered and implemented: 1. **Outgoing pane** — what happens to the review pane of the document you switch away from (stays pinned / detaches / closes / retargets). 2. **Incoming doc** — whether the switched-to document automatically gets a review experience (auto-open the pane and/or auto-establish review state) or stays inert until the user explicitly asks. - The chosen behavior is consistent across the entry points that open the pane (right-click menu #41, toolbar #43, command/keybinding) and across authorable vs. non-authorable / non-markdown documents. - Switching to a non-markdown or non-authorable document does something sane and defined (e.g. the review pane does not try to review a file it cannot review). - E2E browser coverage exercises the switch behavior (open pane on doc A, switch to doc B, assert the defined outcome for both A's pane and B). ## Scope / non-goals **In scope:** the lifecycle/targeting rule for the review pane (the `TrackChangesPreviewController`) as the active editor changes between documents, and whether/how review state is established for the newly-active document. **Non-goals:** - Scroll-sync between editor and pane (that is #32 — a different concern). - Changing what the review experience *renders* (attribution, proposals, diff) — this is about *which document* the pane targets and *when* a review exists, not the rendering. - Multi-pane layout management beyond the single targeting rule (no new split/grid features). ## Assumptions · constraints · dependencies - The review pane today is per-document (`Map<uriString, WebviewPanel>`, `src/trackChangesPreview.ts:45`); whatever rule is chosen must reconcile with that (e.g. "follow active editor" may imply collapsing toward a single live pane, or revealing the per-doc pane). - Per-document review state (F6 baseline, F4 proposals, F3 attribution) already auto-creates on demand when a pane is shown for a doc (`diffViewController.ts` `ensureBaseline`, `proposalController.ts` per-doc state) — an "auto-review on switch" option leans on that; a "do nothing" option must avoid creating state for files the user never asked to review. - Markdown-only product: non-markdown / non-authorable docs (`isAuthorable`) must be handled explicitly. - Related (not blocking): #41 (right-click → Open Review Panel), #43 / F11 (preview toolbar as primary surface), #32 (scroll-sync). This sharpens the same surface those touch. ## Source / signal Self-originated — capture session **vscode-cowriting-plugin-0050** (2026-06-13). Dev observation while working the review-panel surface: "When changing documents, we should decide what happens to the review pane of the current document, and if the document switched to should get a review experience or not." Current behavior confirmed by reading `src/trackChangesPreview.ts` (no `onDidChangeActiveTextEditor` listener; panel stays pinned). ## Priority (WSJF) Provisional: Value 5 · Time-criticality 3 · Opportunity-enablement 3 ÷ Size 3 = ~3.7. `priority/P2` — real navigation friction and a visible incoherence as the pane becomes primary, but not blocking the inner-loop triad; worth doing once the decision is made. ## Decomposition Feature → design-then-build slices (one increment): - [ ] Decide the two coupled rules (outgoing pane fate; incoming-doc review experience) in a short design, with the option set below evaluated. - [ ] Implement the active-editor-change handling in `TrackChangesPreviewController` per the decision. - [ ] Handle non-markdown / non-authorable active documents explicitly. - [ ] E2E coverage of the switch behavior (doc A pane + switch to doc B). ## Solution notes (optional — NOT a design) Non-binding option space for the design to choose among: **Outgoing pane on switch away:** - *Stay pinned* (today) — pane keeps showing doc A; predictable but can be stale/confusing. - *Retarget/follow* — the live pane re-points to the now-active doc B. - *Close/hide* — collapse the pane when you leave its document. **Incoming (switched-to) document:** - *Auto-open a review experience* — show the pane and establish review state for doc B automatically (leans on existing on-demand state creation). - *On-demand only* — do nothing until the user explicitly opens review for B. - *Conditional* — auto-review only if the pane was already open (i.e. "follow mode" is implicitly on once you've opened one), inert otherwise. A likely-coherent default to test against: a **single "live" review pane that follows the active markdown editor** once opened, leaving non-markdown docs untouched and not fabricating review state for files the user never opened a pane on — but this is for the design to settle, not pre-decided here.
benstull added the type/featurepriority/P2 labels 2026-06-14 00:47:28 +00:00
Sign in to join this conversation.