From d8937f0096c45f1f2570355889c0495d4a535cc8 Mon Sep 17 00:00:00 2001 From: timmy Date: Thu, 6 Aug 2026 17:54:35 +0000 Subject: [PATCH] feat: resume mobile review progress (#125) --- frontend/index.html | 67 +++++++++++++++++++++++++++++++++ frontend/review-sheet.js | 48 +++++++++++++++++++++-- src/gitea_proxy.py | 1 + tests/test_gitea_work_search.py | 1 + tests/test_my_work.py | 66 ++++++++++++++++++++++++++++++++ 5 files changed, 180 insertions(+), 3 deletions(-) diff --git a/frontend/index.html b/frontend/index.html index 2308b95..1f64ff4 100644 --- a/frontend/index.html +++ b/frontend/index.html @@ -73,6 +73,12 @@ textarea { resize: vertical; min-height: 120px; } .review-file, .review-history { padding:8px 0; border-bottom:1px solid #1b2d45; overflow-wrap:anywhere; } .review-file-toggle { min-height:44px; width:100%; display:flex; align-items:flex-start; justify-content:space-between; gap:8px; text-align:left; } .review-file-toggle strong { overflow-wrap:anywhere; } +.review-file.reviewed { opacity:.72; } +.review-file.reviewed .review-file-toggle { border-color:#22c55e; } +.review-mark { min-height:44px; width:100%; margin-top:8px; } +.review-mark[aria-pressed="true"] { border-color:#22c55e; background:#123c2a; } +.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-diff { overflow-x:auto; max-width:100%; margin-top:8px; white-space:pre; } .review-diff-line { display:block; min-width:max-content; } .review-diff-line.hunk { color:#93c5fd; } @@ -220,6 +226,10 @@ textarea { resize: vertical; min-height: 120px; }
CI unknownOpen in Gitea

Changed files

+
+ 0 of 0 files reviewed + +

Review history

@@ -271,6 +281,7 @@ textarea { resize: vertical; min-height: 120px; } let hasContextSnapshot = false; let selectedReview = null; let reviewTrigger = null; + let progress = null; async function fetchReviewJson(url, options) { const response = await fetch(url, options); @@ -364,6 +375,39 @@ textarea { resize: vertical; min-height: 120px; } }); } + function reviewFileElement(filename) { + return Array.from(document.querySelectorAll('.review-file')).find( + element => element.dataset.reviewFilename === filename + ); + } + + function showReviewProgress(snapshot) { + const complete = snapshot.total > 0 && snapshot.reviewedCount === snapshot.total; + qs('#review-progress').textContent = complete ? + 'All files reviewed — finish in Gitea' : + (snapshot.newHead ? 'New commits detected · ' : '') + snapshot.reviewedCount + ' of ' + snapshot.total + ' files reviewed'; + qs('#next-unreviewed-review').disabled = !snapshot.nextFilename; + document.querySelectorAll('.review-mark').forEach(button => { + const reviewed = snapshot.reviewed.includes(button.dataset.reviewFilename); + button.setAttribute('aria-pressed', String(reviewed)); + button.textContent = reviewed ? 'Reviewed' : 'Mark reviewed'; + button.closest('.review-file')?.classList.toggle('reviewed', reviewed); + }); + } + + function openNextUnreviewed(snapshot) { + if (!snapshot?.nextFilename) return; + const file = reviewFileElement(snapshot.nextFilename); + if (!file) return; + const toggle = file.querySelector('.review-file-toggle'); + const panel = toggle && document.getElementById(toggle.getAttribute('aria-controls')); + if (toggle && panel && toggle.getAttribute('aria-expanded') !== 'true') { + createReviewController.toggleDiff(toggle, panel); + } + file.scrollIntoView({ behavior: 'smooth', block: 'center' }); + toggle?.focus(); + } + async function openReviewSheet(item, trigger) { selectedReview = item; reviewTrigger = trigger; @@ -373,6 +417,8 @@ textarea { resize: vertical; min-height: 120px; } qs('#review-sheet-status').textContent = 'Loading review details…'; qs('#review-sheet-body').textContent = ''; qs('#review-files').textContent = ''; + qs('#review-progress').textContent = '0 of 0 files reviewed'; + qs('#next-unreviewed-review').disabled = true; qs('#review-history').textContent = ''; qs('#open-review-gitea').href = item.url; qs('#close-review-sheet').focus(); @@ -390,6 +436,23 @@ textarea { resize: vertical; min-height: 120px; } if (panel) createReviewController.toggleDiff(button, panel); }); }); + progress = createReviewController.createProgress({ + storage: localStorage, + repository: item.repository, + number: item.number, + headSha: detail.head_sha || 'unknown', + files: detail.files || [], + }); + document.querySelectorAll('.review-mark').forEach(button => { + button.addEventListener('click', () => { + const snapshot = progress.markReviewed(button.dataset.reviewFilename); + showReviewProgress(snapshot); + openNextUnreviewed(snapshot); + }); + }); + const progressSnapshot = progress.snapshot(); + showReviewProgress(progressSnapshot); + openNextUnreviewed(progressSnapshot); qs('#review-history').innerHTML = (detail.reviews || []).length ? detail.reviews.map(review => '
' + escapeHtml(review.user?.login || 'Reviewer') + ' · ' + escapeHtml(review.state || 'commented') + (review.body ? '
' + escapeHtml(review.body) + '
' : '') + '
' @@ -403,6 +466,7 @@ textarea { resize: vertical; min-height: 120px; } function closeReviewSheet() { qs('#review-sheet').classList.remove('open'); selectedReview = null; + progress = null; if (reviewTrigger?.isConnected) reviewTrigger.focus(); } @@ -513,6 +577,9 @@ textarea { resize: vertical; min-height: 120px; } document.addEventListener('keydown', (e) => { if ((e.metaKey||e.ctrlKey) && e.key==='k') { e.preventDefault(); qs('#cmd-palette').classList.toggle('open'); if(qs('#cmd-palette').classList.contains('open')){ qs('#cmd-input').focus(); renderCommands(''); } } }); qs('#close-whiteboard').addEventListener('click', () => closeModal('whiteboard-modal')); qs('#close-review-sheet').addEventListener('click', closeReviewSheet); + qs('#next-unreviewed-review').addEventListener('click', () => { + if (progress) openNextUnreviewed(progress.snapshot()); + }); const contextPoller = createContextPoller({ fetchContext: fetchContextSnapshot, diff --git a/frontend/review-sheet.js b/frontend/review-sheet.js index cbc853c..292e08b 100644 --- a/frontend/review-sheet.js +++ b/frontend/review-sheet.js @@ -38,10 +38,12 @@ function renderDiffFile(file, index, escapeHtml) { preview = ''; } - return '
' + preview + '
'; + Number(file.deletions || 0) + '' + preview + + ''; } function toggleDiff(button, panel) { @@ -50,8 +52,48 @@ function toggleDiff(button, panel) { panel.hidden = expanded; } +function createProgress({ storage, repository, number, headSha, files }) { + const filenames = (files || []).map(file => file && file.filename).filter(Boolean); + const key = 'stackchain.review-progress.v1:' + repository + '#' + number + '@' + headSha; + const headKey = 'stackchain.review-progress.v1:' + repository + '#' + number + ':head'; + let reviewed = []; + let newHead = false; + try { + const previousHead = storage.getItem(headKey); + newHead = Boolean(previousHead && previousHead !== headSha); + storage.setItem(headKey, headSha); + const saved = JSON.parse(storage.getItem(key) || '[]'); + if (Array.isArray(saved)) reviewed = filenames.filter(filename => saved.includes(filename)); + } catch (error) { + reviewed = []; + } + + function snapshot() { + const pending = filenames.find(filename => !reviewed.includes(filename)) || null; + return { + reviewed: [...reviewed], + reviewedCount: reviewed.length, + total: filenames.length, + nextFilename: pending, + ...(newHead ? { newHead: true } : {}), + }; + } + + function markReviewed(filename) { + if (filenames.includes(filename) && !reviewed.includes(filename)) { + reviewed.push(filename); + reviewed = filenames.filter(item => reviewed.includes(item)); + try { storage.setItem(key, JSON.stringify(reviewed)); } catch (error) { /* local progress remains usable */ } + } + return snapshot(); + } + + return { snapshot, markReviewed }; +} + createReviewController.renderDiffFile = renderDiffFile; createReviewController.toggleDiff = toggleDiff; +createReviewController.createProgress = createProgress; if (typeof module !== 'undefined' && module.exports) { module.exports = createReviewController; diff --git a/src/gitea_proxy.py b/src/gitea_proxy.py index 7c41807..575b7e0 100644 --- a/src/gitea_proxy.py +++ b/src/gitea_proxy.py @@ -189,6 +189,7 @@ async def pull_review_detail(repository: str, number: int) -> dict: "body": pull.get("body") or "", "url": pull.get("html_url", ""), "author": user.get("login", ""), + "head_sha": sha, "ci_state": status.get("state", "unknown") if isinstance(status, dict) else "unknown", "files": normalized_files, "reviews": normalized_reviews, diff --git a/tests/test_gitea_work_search.py b/tests/test_gitea_work_search.py index df02f16..f700dd2 100644 --- a/tests/test_gitea_work_search.py +++ b/tests/test_gitea_work_search.py @@ -175,6 +175,7 @@ async def test_pull_review_detail_combines_pr_files_status_and_reviews(monkeypat "repos/stackchain/api/pulls/7/reviews", ] assert detail["author"] == "alex" + assert detail["head_sha"] == "abc123" assert detail["ci_state"] == "success" assert detail["files"][0]["filename"] == "src/api.py" assert detail["reviews"][0]["state"] == "APPROVED" diff --git a/tests/test_my_work.py b/tests/test_my_work.py index f99dfd9..7833597 100644 --- a/tests/test_my_work.py +++ b/tests/test_my_work.py @@ -197,10 +197,69 @@ process.stdout.write(JSON.stringify({{ html, expanded: button.attrs['aria-expand assert '-old <token>' in output["html"] assert '+new & safe' in output["html"] assert 'Preview truncated' in output["html"] + assert 'class="review-mark"' in output["html"] + assert 'data-review-filename="src/<api>.py"' in output["html"] + assert 'Mark reviewed' in output["html"] assert output["expanded"] == "true" assert output["hidden"] is False +def test_review_progress_is_explicit_and_restores_for_the_same_head_sha(): + script = f""" +const reviewSheet = require({json.dumps(str(REVIEW_SHEET))}); +const values = new Map(); +const storage = {{ + getItem(key) {{ return values.has(key) ? values.get(key) : null; }}, + setItem(key, value) {{ values.set(key, value); }} +}}; +const options = {{ + storage, repository: 'stackchain/api', number: 7, headSha: 'abc123', + files: [{{filename:'src/a.py'}}, {{filename:'src/b.py'}}, {{filename:'README.md'}}] +}}; +const first = reviewSheet.createProgress(options); +const before = first.snapshot(); +const marked = first.markReviewed('src/a.py'); +const restored = reviewSheet.createProgress(options).snapshot(); +process.stdout.write(JSON.stringify({{ before, marked, restored }})); +""" + result = subprocess.run( + ["node", "-e", script], check=True, capture_output=True, text=True + ) + output = json.loads(result.stdout) + + assert output["before"] == { + "reviewed": [], "reviewedCount": 0, "total": 3, "nextFilename": "src/a.py" + } + assert output["marked"] == { + "reviewed": ["src/a.py"], "reviewedCount": 1, "total": 3, + "nextFilename": "src/b.py", + } + assert output["restored"] == output["marked"] + + +def test_review_progress_resets_when_new_commits_change_the_head_sha(): + script = f""" +const reviewSheet = require({json.dumps(str(REVIEW_SHEET))}); +const values = new Map(); +const storage = {{ + getItem(key) {{ return values.has(key) ? values.get(key) : null; }}, + setItem(key, value) {{ values.set(key, value); }} +}}; +const base = {{ storage, repository: 'stackchain/api', number: 7, files: [{{filename:'src/a.py'}}] }}; +reviewSheet.createProgress({{...base, headSha:'abc123'}}).markReviewed('src/a.py'); +const changed = reviewSheet.createProgress({{...base, headSha:'def456'}}).snapshot(); +process.stdout.write(JSON.stringify(changed)); +""" + result = subprocess.run( + ["node", "-e", script], check=True, capture_output=True, text=True + ) + + assert json.loads(result.stdout) == { + "reviewed": [], "reviewedCount": 0, "total": 1, + "nextFilename": "src/a.py", "newHead": True, + } + + @pytest.mark.anyio async def test_review_requests_open_an_accessible_mobile_detail_sheet(): html = await dashboard() @@ -214,6 +273,13 @@ async def test_review_requests_open_an_accessible_mobile_detail_sheet(): assert '.review-sheet-panel' in html and 'width:100%' in html assert '.review-action' in html and 'min-height:44px' in html assert '.review-file-toggle' in html and 'min-height:44px' in html + assert '.review-mark' in html and 'min-height:44px' in html + assert 'id="review-progress"' in html and 'aria-live="polite"' in html + assert 'id="next-unreviewed-review"' in html + assert '.review-progress-actions' in html and 'position:sticky' in html + assert "createReviewController.createProgress" in html + assert "progress.markReviewed" in html + assert "scrollIntoView" in html assert '.review-diff' in html and 'overflow-x:auto' in html