Merge pull request 'Wrap mobile review diffs without horizontal panning' (#446) from timmy/445-wrap-mobile-review-diffs into main

README.md

@@ -94,7 +94,12 @@ Approve, and Request changes reviews, and assigned-PR merge require repository
 write permission. Native Comment, Approve, and Request changes reviews support
 head-scoped draft comments anchored to changed lines; the dashboard validates each
 comment path and submits the summary, decision, and inline comments in one review
-request. The dashboard rechecks the current pull-request head, CI success, draft
+request. On phone-width review sheets, changed lines wrap inside a stable old/new
+line-number gutter by default so long source lines remain readable and commentable
+without horizontal panning. **Lines wrapped** toggles back to whitespace-preserving
+horizontal inspection, and an explicit choice persists on the device across files,
+review-sheet reopen, and reload. Desktop review diffs remain horizontally scrollable
+until wrapping is explicitly enabled. The dashboard rechecks the current pull-request head, CI success, draft
 state, and mergeability immediately before every merge.
 Serve the dashboard only to trusted users on its own origin; cross-origin API
 access is intentionally disabled. Authentication defaults to fail-closed

frontend/dashboard.css

@@ -187,9 +187,18 @@ textarea { resize: vertical; min-height: 120px; }
 .review-copy-fallback { min-height:140px; }
 .review-progress-actions { position:sticky; bottom:0; z-index:2; display:flex; align-items:center; justify-content:space-between; gap:10px; margin:8px -4px 0; padding:10px 4px; background:rgba(11,21,38,.96); border-top:1px solid #2a496e; }
 .review-progress-actions button { min-height:44px; max-width:100%; }
+.review-display-tools { display:flex; align-items:center; gap:10px; margin-bottom:8px; }
+.review-display-tools button { min-height:44px; flex:0 0 auto; }
+.review-display-tools button[aria-pressed="true"] { color:#bfdbfe; background:#17365a; outline:1px solid #60a5fa; }
 .review-diff { overflow-x:auto; max-width:100%; margin-top:8px; white-space:pre; }
-.review-diff-line { display:block; min-width:max-content; }
-.review-inline-target { min-height:44px; width:100%; padding:8px; border:0; border-radius:0; text-align:left; font:inherit; white-space:pre; }
+.review-diff-line { display:grid; grid-template-columns:max-content minmax(max-content,1fr); align-items:stretch; min-width:max-content; }
+.review-line-numbers { position:sticky; left:0; z-index:1; display:grid; grid-template-columns:4ch 4ch; align-self:stretch; background:#0b1526; color:#7694b6; user-select:none; }
+.review-line-number { padding:8px 4px; text-align:right; border-right:1px solid #244363; }
+.review-line-code { min-width:0; padding:8px; white-space:pre; overflow-wrap:anywhere; }
+.review-inline-target { min-height:44px; width:100%; padding:0; border:0; border-radius:0; text-align:left; font:inherit; white-space:normal; }
+.wrap-lines .review-diff { overflow-x:hidden; white-space:normal; }
+.wrap-lines .review-diff-line { grid-template-columns:max-content minmax(0,1fr); min-width:0; }
+.wrap-lines .review-line-code { white-space:pre-wrap; overflow-wrap:anywhere; }
 .review-inline-target.has-draft { box-shadow:inset 4px 0 #fbbf24; }
 .review-inline-composer { position:sticky; bottom:0; z-index:4; display:grid; gap:8px; padding:10px; padding-bottom:calc(10px + env(safe-area-inset-bottom)); border:1px solid #60a5fa; border-radius:10px; background:#0b1526; }
 .review-inline-composer[hidden] { display:none; }

frontend/dashboard.js

@@ -237,6 +237,10 @@
     return payload;
   }
   const reviewController = createReviewController({ fetchJson: fetchReviewJson, storage: localStorage });
+  const wrapPreference = createReviewController.createWrapPreference({
+    storage: localStorage,
+    mobile: window.matchMedia('(max-width: 600px)').matches,
+  });
   const issueController = createIssueSheet({ fetchJson: fetchReviewJson, storage: localStorage });
   const planningLoader = createIssueSheet.createPlanningLoader({
     loadLabels: item => issueController.loadLabels(item),
@@ -1765,6 +1769,14 @@
     );
   }
 
+  function applyReviewWrap(snapshot = wrapPreference.snapshot()) {
+    const reviewFilesElement = qs('#review-files');
+    const button = qs('#review-wrap-lines');
+    reviewFilesElement.classList.toggle('wrap-lines', snapshot.wrapped);
+    button.setAttribute('aria-pressed', String(snapshot.wrapped));
+    button.textContent = snapshot.wrapped ? 'Lines wrapped' : 'Wrap lines';
+  }
+
   function showReviewProgress(snapshot) {
     const complete = snapshot.total > 0 && snapshot.reviewedCount === snapshot.total;
     qs('#review-progress').textContent = complete ?
@@ -2582,6 +2594,7 @@
     qs('#retry-review-load').hidden = true;
     qs('#review-sheet-body').textContent = '';
     qs('#review-files').textContent = '';
+    applyReviewWrap();
     qs('#review-progress').textContent = '0 of 0 files reviewed';
     qs('#next-unreviewed-review').disabled = true;
     qs('#review-history').textContent = '';
@@ -3910,6 +3923,10 @@
   qs('#next-unreviewed-review').addEventListener('click', () => {
     if (progress) openNextUnreviewed(progress.snapshot());
   });
+  qs('#review-wrap-lines').addEventListener('click', () => {
+    const next = !wrapPreference.snapshot().wrapped;
+    applyReviewWrap(wrapPreference.setWrapped(next));
+  });
   qs('#review-summary').addEventListener('input', event => draft?.setSummary(event.target.value));
   qs('#review-decision').addEventListener('change', event => draft?.setDecision(event.target.value));
   qs('#save-inline-comment').addEventListener('click', () => {

frontend/index.html

@@ -539,6 +539,10 @@
     <div class="review-sheet-body markdown-content" id="review-sheet-body"></div>
     <div class="row"><span class="pill" id="review-ci-state">CI unknown</span><button class="share-work-route" type="button">Share</button><a id="open-review-gitea" href="#" target="_blank" rel="noopener noreferrer">Open in Gitea</a></div>
     <h2>Changed files</h2>
+    <div class="review-display-tools">
+      <button id="review-wrap-lines" type="button" aria-pressed="false" aria-controls="review-files">Wrap lines</button>
+      <span class="small">Keep long diffs inside the review sheet.</span>
+    </div>
     <div id="review-files" class="muted"></div>
     <section class="review-inline-composer" id="review-inline-composer" aria-labelledby="review-inline-title" hidden>
       <strong id="review-inline-title">Comment on changed line</strong>

frontend/review-sheet.js

@@ -47,6 +47,28 @@ function createReviewController({
   return { load, submit };
 }
 
+function createWrapPreference({ storage, mobile }) {
+  const key = 'stackchain.review-wrap.v1';
+  let explicit = false;
+  let wrapped = Boolean(mobile);
+  try {
+    const saved = storage?.getItem(key);
+    if (saved === 'true' || saved === 'false') {
+      explicit = true;
+      wrapped = saved === 'true';
+    }
+  } catch (_error) { /* keep the viewport default */ }
+
+  function snapshot() { return { wrapped, explicit }; }
+  function setWrapped(value) {
+    wrapped = Boolean(value);
+    explicit = true;
+    try { storage?.setItem(key, String(wrapped)); } catch (_error) { /* keep in memory */ }
+    return snapshot();
+  }
+  return { snapshot, setWrapped, storageKey: key };
+}
+
 function diffLineClass(line) {
   if (line.startsWith('@@')) return 'hunk';
   if (line.startsWith('+')) return 'added';
@@ -70,16 +92,19 @@ function parseDiffLines(lines) {
       return { text, kind: diffLineClass(text), commentable: false };
     }
     if (text.startsWith('-')) {
-      const row = { text, kind: 'removed', commentable: true, old_position: oldLine };
+      const row = { text, kind: 'removed', commentable: true, old_line: oldLine, old_position: oldLine };
       oldLine += 1;
       return row;
     }
     if (text.startsWith('+')) {
-      const row = { text, kind: 'added', commentable: true, new_position: newLine };
+      const row = { text, kind: 'added', commentable: true, new_line: newLine, new_position: newLine };
       newLine += 1;
       return row;
     }
-    const row = { text, kind: 'context', commentable: true, new_position: newLine };
+    const row = {
+      text, kind: 'context', commentable: true,
+      old_line: oldLine, new_line: newLine, new_position: newLine,
+    };
     oldLine += 1;
     newLine += 1;
     return row;
@@ -91,8 +116,14 @@ function renderDiffFile(file, index, escapeHtml) {
   let preview;
   if (file.diff_available) {
     const lines = parseDiffLines(file.diff_lines).map(row => {
+      const oldNumber = row.old_line || row.old_position || '';
+      const newNumber = row.new_line || row.new_position || '';
+      const content = '<span class="review-line-numbers" aria-hidden="true">' +
+        '<span class="review-line-number old">' + oldNumber + '</span>' +
+        '<span class="review-line-number new">' + newNumber + '</span></span>' +
+        '<span class="review-line-code">' + escapeHtml(row.text) + '</span>';
       if (!row.commentable) {
-        return '<span class="review-diff-line ' + row.kind + '">' + escapeHtml(row.text) + '</span>';
+        return '<span class="review-diff-line ' + row.kind + '">' + content + '</span>';
       }
       const position = row.old_position
         ? ' data-old-position="' + row.old_position + '"'
@@ -100,7 +131,7 @@ function renderDiffFile(file, index, escapeHtml) {
       return '<button type="button" class="review-diff-line review-inline-target ' + row.kind +
         '" data-review-filename="' + escapeHtml(file.filename || '') + '"' + position +
         ' aria-label="Comment on ' + escapeHtml(file.filename || 'changed file') + ' line ' +
-        (row.old_position || row.new_position) + '">' + escapeHtml(row.text) + '</button>';
+        (row.old_position || row.new_position) + '">' + content + '</button>';
     }).join('');
     preview = '<pre class="review-diff" id="' + panelId + '" hidden>' + lines +
       (file.diff_truncated ? '<span class="review-diff-note">Preview truncated · open in Gitea for the full diff.</span>' : '') +
@@ -325,6 +356,7 @@ function prepareMergeContinuation({ storage, item, headSha, reviewed, decision }
 }
 
 createReviewController.renderDiffFile = renderDiffFile;
+createReviewController.createWrapPreference = createWrapPreference;
 createReviewController.parseDiffLines = parseDiffLines;
 createReviewController.toggleDiff = toggleDiff;
 createReviewController.createProgress = createProgress;

tests/test_my_work.py

@@ -3950,12 +3950,45 @@ process.stdout.write(JSON.stringify({{ html, expanded: button.attrs['aria-expand
     assert 'class="review-diff-line review-inline-target removed"' in output["html"]
     assert 'data-old-position="1"' in output["html"]
     assert 'data-new-position="1"' in output["html"]
+    assert '<span class="review-line-numbers" aria-hidden="true"><span class="review-line-number old">1</span><span class="review-line-number new"></span></span>' in output["html"]
+    assert '<span class="review-line-numbers" aria-hidden="true"><span class="review-line-number old"></span><span class="review-line-number new">1</span></span>' in output["html"]
+    assert '<span class="review-line-code">-old &lt;token&gt;</span>' in output["html"]
+    assert '<span class="review-line-code">+new &amp; safe</span>' in output["html"]
     assert 'aria-label="Comment on src/&lt;api&gt;.py line 1"' in output["html"]
     assert 'Mark reviewed' in output["html"]
     assert output["expanded"] == "true"
     assert output["hidden"] is False
 
 
+def test_review_wrap_preference_defaults_to_phone_layout_and_persists_explicit_choice():
+    script = f"""
+const reviewSheet = require({json.dumps(str(REVIEW_SHEET))});
+const values = new Map();
+const storage = {{
+  getItem: key => values.has(key) ? values.get(key) : null,
+  setItem: (key, value) => values.set(key, value),
+}};
+const phone = reviewSheet.createWrapPreference({{storage, mobile: true}});
+const initial = phone.snapshot();
+const disabled = phone.setWrapped(false);
+const restoredPhone = reviewSheet.createWrapPreference({{storage, mobile: true}}).snapshot();
+const restoredDesktop = reviewSheet.createWrapPreference({{storage, mobile: false}}).snapshot();
+process.stdout.write(JSON.stringify({{initial, disabled, restoredPhone, restoredDesktop,
+  stored: values.get('stackchain.review-wrap.v1')}}));
+"""
+    result = subprocess.run(
+        ["node", "-e", script], check=True, capture_output=True, text=True
+    )
+
+    assert json.loads(result.stdout) == {
+        "initial": {"wrapped": True, "explicit": False},
+        "disabled": {"wrapped": False, "explicit": True},
+        "restoredPhone": {"wrapped": False, "explicit": True},
+        "restoredDesktop": {"wrapped": False, "explicit": True},
+        "stored": "false",
+    }
+
+
 def test_review_diff_parser_maps_multi_hunk_lines_to_old_or_new_positions():
     script = f"""
 const reviewSheet = require({json.dumps(str(REVIEW_SHEET))});
@@ -3973,14 +4006,14 @@ process.stdout.write(JSON.stringify(rows));
 
     assert json.loads(result.stdout) == [
         {"text": "@@ -10,3 +20,4 @@ function run()", "kind": "hunk", "commentable": False},
-        {"text": " context", "kind": "context", "commentable": True, "new_position": 20},
-        {"text": "-removed", "kind": "removed", "commentable": True, "old_position": 11},
-        {"text": "+added", "kind": "added", "commentable": True, "new_position": 21},
-        {"text": "+second", "kind": "added", "commentable": True, "new_position": 22},
+        {"text": " context", "kind": "context", "commentable": True, "old_line": 10, "new_line": 20, "new_position": 20},
+        {"text": "-removed", "kind": "removed", "commentable": True, "old_line": 11, "old_position": 11},
+        {"text": "+added", "kind": "added", "commentable": True, "new_line": 21, "new_position": 21},
+        {"text": "+second", "kind": "added", "commentable": True, "new_line": 22, "new_position": 22},
         {"text": "\\ No newline at end of file", "kind": "note", "commentable": False},
         {"text": "@@ -40 +51 @@", "kind": "hunk", "commentable": False},
-        {"text": "-old tail", "kind": "removed", "commentable": True, "old_position": 40},
-        {"text": "+new tail", "kind": "added", "commentable": True, "new_position": 51},
+        {"text": "-old tail", "kind": "removed", "commentable": True, "old_line": 40, "old_position": 40},
+        {"text": "+new tail", "kind": "added", "commentable": True, "new_line": 51, "new_position": 51},
     ]
 
 
@@ -4207,6 +4240,25 @@ async def test_review_requests_open_an_accessible_mobile_detail_sheet():
     assert '.review-diff' in html and 'overflow-x:auto' in html
 
 
+@pytest.mark.anyio
+async def test_mobile_review_wraps_long_lines_with_gutters_and_persisted_toggle():
+    html = await dashboard()
+
+    assert 'id="review-wrap-lines"' in html
+    assert 'aria-pressed="false"' in html
+    assert 'aria-controls="review-files"' in html
+    assert "createReviewController.createWrapPreference" in html
+    assert "matchMedia('(max-width: 600px)').matches" in html
+    assert "reviewFilesElement.classList.toggle('wrap-lines', snapshot.wrapped)" in html
+    assert "wrapPreference.setWrapped" in html
+    assert '.review-display-tools button' in html and 'min-height:44px' in html
+    assert '.review-line-numbers' in html and 'grid-template-columns:4ch 4ch' in html
+    assert '.review-line-code' in html and 'overflow-wrap:anywhere' in html
+    assert '.wrap-lines .review-diff' in html and 'overflow-x:hidden' in html
+    assert '.wrap-lines .review-diff-line' in html and 'min-width:0' in html
+    assert '.wrap-lines .review-line-code' in html and 'white-space:pre-wrap' in html
+
+
 @pytest.mark.anyio
 async def test_review_attention_drafts_require_revision_instead_of_resending_stale_payload():
     html = await dashboard()