Add persistent mobile requested-review workspace navigation #970
|
|
@ -469,28 +469,30 @@ textarea { resize: vertical; min-height: 120px; }
|
|||
.issue-sheet-panel { width:min(560px,100%); height:100%; overflow:auto; padding:18px; background:#0b1526; border-left:1px solid #2a496e; }
|
||||
.issue-sheet-header { display:flex; align-items:center; justify-content:space-between; gap:10px; }
|
||||
.issue-sheet-header button { min-height:44px; }
|
||||
.mobile-issue-detail-nav, .mobile-pull-detail-nav, .mobile-update-detail-nav { display:none; }
|
||||
.mobile-issue-detail-nav, .mobile-pull-detail-nav, .mobile-update-detail-nav, .mobile-review-detail-nav { display:none; }
|
||||
@media (max-width:600px) {
|
||||
.issue-sheet-panel, .pull-sheet-panel, .update-sheet-panel { padding-top:max(12px,env(safe-area-inset-top)); }
|
||||
.mobile-issue-detail-nav, .mobile-pull-detail-nav, .mobile-update-detail-nav {
|
||||
.issue-sheet-panel, .pull-sheet-panel, .update-sheet-panel, .review-sheet-panel { padding-top:max(12px,env(safe-area-inset-top)); }
|
||||
.mobile-issue-detail-nav, .mobile-pull-detail-nav, .mobile-update-detail-nav, .mobile-review-detail-nav {
|
||||
position:sticky; top:env(safe-area-inset-top); z-index:6;
|
||||
display:grid; grid-template-columns:repeat(4,minmax(0,1fr)); gap:4px;
|
||||
margin:8px -6px 12px; padding:6px;
|
||||
background:rgba(11,21,38,.98); border-block:1px solid #2a496e;
|
||||
}
|
||||
.mobile-issue-detail-nav button, .mobile-pull-detail-nav button, .mobile-update-detail-nav button {
|
||||
.mobile-issue-detail-nav button, .mobile-pull-detail-nav button, .mobile-update-detail-nav button, .mobile-review-detail-nav button {
|
||||
min-width:0; min-height:44px; padding:4px; overflow-wrap:anywhere;
|
||||
border-color:transparent; font-size:12px;
|
||||
}
|
||||
.mobile-issue-detail-nav button[aria-current="location"], .mobile-pull-detail-nav button[aria-current="location"], .mobile-update-detail-nav button[aria-current="location"] {
|
||||
.mobile-issue-detail-nav button[aria-current="location"], .mobile-pull-detail-nav button[aria-current="location"], .mobile-update-detail-nav button[aria-current="location"], .mobile-review-detail-nav button[aria-current="location"] {
|
||||
border-color:#60a5fa; background:#17365a; color:#fff;
|
||||
}
|
||||
#issue-overview, #issue-conversation, #issue-comment, #issue-planning { scroll-margin-top:72px; }
|
||||
#pull-overview, #pull-conversation, #pull-comment, #pull-review { scroll-margin-top:72px; }
|
||||
#update-conversation, #update-subject-context, #update-reply-workspace { scroll-margin-top:72px; }
|
||||
#review-overview, #review-files-workspace, #review-feedback, #review-history-workspace { scroll-margin-top:72px; }
|
||||
}
|
||||
@media (min-width:601px) { .mobile-issue-detail-nav, .mobile-pull-detail-nav { display:none; } }
|
||||
@media (min-width:601px) { .mobile-update-detail-nav { display:none; } }
|
||||
@media (min-width:601px) { .mobile-review-detail-nav { display:none; } }
|
||||
.completed-filed-actions { position:fixed; right:0; bottom:0; z-index:57; box-sizing:border-box; width:min(560px,100%); display:grid; grid-template-columns:minmax(0,1fr); align-items:center; gap:8px; margin:0; padding:10px 12px calc(10px + env(safe-area-inset-bottom)); border:1px solid #4ade80; border-radius:12px 0 0; background:rgba(11,21,38,.98); overflow-wrap:anywhere; }
|
||||
.completed-filed-actions[hidden] { display:none; }
|
||||
.completed-filed-actions button { min-height:44px; min-width:0; }
|
||||
|
|
|
|||
|
|
@ -1366,6 +1366,19 @@
|
|||
prefersReducedMotion:() => window.matchMedia('(prefers-reduced-motion: reduce)').matches,
|
||||
});
|
||||
mobileUpdateDetailNavigation.start();
|
||||
const mobileReviewDetailNavigation = createMobileReviewDetailNavigation({
|
||||
root:qs('#review-sheet .review-sheet-panel'),
|
||||
buttons:Object.fromEntries(Array.from(document.querySelectorAll('[data-review-section]')).map(button => [button.dataset.reviewSection, button])),
|
||||
targets:{
|
||||
overview:qs('#review-overview'),
|
||||
files:qs('#review-files-workspace'),
|
||||
feedback:qs('#review-feedback'),
|
||||
history:qs('#review-history-workspace'),
|
||||
},
|
||||
summaryComposer:qs('#review-summary'),
|
||||
prefersReducedMotion:() => window.matchMedia('(prefers-reduced-motion: reduce)').matches,
|
||||
});
|
||||
mobileReviewDetailNavigation.start();
|
||||
const issueDetailPosition = createWorkDetailPosition({ panel: qs('#issue-sheet .issue-sheet-panel') });
|
||||
const pullDetailPosition = createWorkDetailPosition({ panel: qs('#pull-sheet .pull-sheet-panel') });
|
||||
const reviewDetailPosition = createWorkDetailPosition({ panel: qs('#review-sheet .review-sheet-panel') });
|
||||
|
|
@ -4748,6 +4761,7 @@
|
|||
async function openReviewSheet(item, trigger, cachedDetail = null) {
|
||||
if (!item || !await ensurePullWorkflow(trigger)) return;
|
||||
reviewDetailPosition.open(workDetailIdentity('review', item));
|
||||
mobileReviewDetailNavigation.reset();
|
||||
selectedReview = item;
|
||||
reviewTrigger = trigger;
|
||||
offlineReview = Boolean(cachedDetail);
|
||||
|
|
|
|||
|
|
@ -1330,6 +1330,13 @@
|
|||
</div>
|
||||
<button class="review-action" id="close-review-sheet">Close</button>
|
||||
</div>
|
||||
<nav class="mobile-review-detail-nav" aria-label="Review sections">
|
||||
<button type="button" data-review-section="overview">Overview</button>
|
||||
<button type="button" data-review-section="files">Files</button>
|
||||
<button type="button" data-review-section="feedback">Feedback</button>
|
||||
<button type="button" data-review-section="history">History</button>
|
||||
</nav>
|
||||
<section id="review-overview">
|
||||
<div id="review-sheet-status" class="small" aria-live="polite">Choose a review request.</div>
|
||||
<button class="review-retry" id="retry-review-load" hidden>Retry loading review</button>
|
||||
<div class="review-sheet-body markdown-content" id="review-sheet-body"></div>
|
||||
|
|
@ -1339,6 +1346,8 @@
|
|||
<button class="ci-check-refresh" id="refresh-review-checks" type="button">Refresh checks</button>
|
||||
<div class="ci-check-list" id="review-check-list"></div>
|
||||
</details>
|
||||
</section>
|
||||
<section id="review-files-workspace">
|
||||
<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>
|
||||
|
|
@ -1359,7 +1368,8 @@
|
|||
<span class="small" id="review-progress" aria-live="polite">0 of 0 files reviewed</span>
|
||||
<button id="next-unreviewed-review" disabled>Next unreviewed</button>
|
||||
</div>
|
||||
<section class="review-feedback" aria-labelledby="review-feedback-title">
|
||||
</section>
|
||||
<section class="review-feedback" id="review-feedback" aria-labelledby="review-feedback-title">
|
||||
<h2 id="review-feedback-title">Draft feedback</h2>
|
||||
<label>Intended decision
|
||||
<select id="review-decision">
|
||||
|
|
@ -1383,8 +1393,10 @@
|
|||
<button id="continue-review-to-merge" type="button" hidden>Continue to merge</button>
|
||||
</div>
|
||||
</section>
|
||||
<section id="review-history-workspace">
|
||||
<h2>Review history</h2>
|
||||
<div id="review-history" class="muted"></div>
|
||||
</section>
|
||||
<div class="review-sheet-actions"><details class="detail-defer"><summary>Defer</summary><div class="detail-defer-options"><button type="button" data-detail-defer-preset="today" disabled data-planning-disabled>Later today</button><button type="button" data-detail-defer-preset="tomorrow" disabled data-planning-disabled>Tomorrow</button><button type="button" data-detail-defer-custom disabled data-planning-disabled>Choose date & time</button><button type="button" data-detail-defer-cancel>Cancel</button></div></details></div>
|
||||
<nav class="work-session-nav" aria-label="Work session" hidden>
|
||||
<span class="small" aria-live="polite" data-work-session-progress></span>
|
||||
|
|
@ -1558,6 +1570,7 @@
|
|||
<script src="static/issue-filing-receipt.js"></script>
|
||||
<script src="static/mobile-issue-detail-nav.js"></script>
|
||||
<script src="static/mobile-update-detail-nav.js"></script>
|
||||
<script src="static/mobile-review-detail-nav.js"></script>
|
||||
<script src="static/dashboard.js"></script>
|
||||
</body>
|
||||
</html>
|
||||
|
|
|
|||
76
frontend/mobile-review-detail-nav.js
Normal file
76
frontend/mobile-review-detail-nav.js
Normal file
|
|
@ -0,0 +1,76 @@
|
|||
function createMobileReviewDetailNavigation(options) {
|
||||
const buttons = options.buttons || {};
|
||||
const targets = options.targets || {};
|
||||
const listeners = new Map();
|
||||
const prefersReducedMotion = options.prefersReducedMotion || (() => false);
|
||||
const targetNames = new Map(Object.entries(targets).map(([name, target]) => [target, name]));
|
||||
let observer = null;
|
||||
|
||||
function select(name) {
|
||||
Object.entries(buttons).forEach(([key, button]) => {
|
||||
if (!button) return;
|
||||
if (key === name) button.setAttribute('aria-current', 'location');
|
||||
else button.removeAttribute('aria-current');
|
||||
});
|
||||
}
|
||||
|
||||
function navigate(name) {
|
||||
const target = targets[name];
|
||||
if (!target) return false;
|
||||
target.scrollIntoView({
|
||||
block:'start',
|
||||
behavior:prefersReducedMotion() ? 'auto' : 'smooth',
|
||||
});
|
||||
if (name === 'feedback' && options.summaryComposer) {
|
||||
options.summaryComposer.focus({ preventScroll:true });
|
||||
}
|
||||
select(name);
|
||||
return true;
|
||||
}
|
||||
|
||||
function reset() {
|
||||
select('overview');
|
||||
}
|
||||
|
||||
return {
|
||||
start() {
|
||||
Object.entries(buttons).forEach(([name, button]) => {
|
||||
if (!button || listeners.has(button)) return;
|
||||
const listener = event => {
|
||||
event.preventDefault();
|
||||
navigate(name);
|
||||
};
|
||||
listeners.set(button, listener);
|
||||
button.addEventListener('click', listener);
|
||||
});
|
||||
reset();
|
||||
const observe = options.observe || ((handler, observedTargets, root) => {
|
||||
if (typeof IntersectionObserver === 'undefined') return null;
|
||||
const instance = new IntersectionObserver(handler, {
|
||||
root:root || null,
|
||||
rootMargin:'-20% 0px -60% 0px',
|
||||
threshold:[0, 0.25, 0.5, 0.75, 1],
|
||||
});
|
||||
observedTargets.forEach(target => instance.observe(target));
|
||||
return instance;
|
||||
});
|
||||
observer = observe(entries => {
|
||||
const visible = entries
|
||||
.filter(entry => entry.isIntersecting && targetNames.has(entry.target))
|
||||
.sort((left, right) => right.intersectionRatio - left.intersectionRatio)[0];
|
||||
if (visible) select(targetNames.get(visible.target));
|
||||
}, Array.from(targetNames.keys()).filter(Boolean), options.root || null);
|
||||
},
|
||||
stop() {
|
||||
listeners.forEach((listener, button) => button.removeEventListener('click', listener));
|
||||
listeners.clear();
|
||||
if (observer) observer.disconnect();
|
||||
observer = null;
|
||||
},
|
||||
navigate,
|
||||
reset,
|
||||
select,
|
||||
};
|
||||
}
|
||||
|
||||
if (typeof module !== 'undefined') module.exports = createMobileReviewDetailNavigation;
|
||||
|
|
@ -79,6 +79,7 @@ const SHELL = [
|
|||
BASE + 'static/issue-sheet.js',
|
||||
BASE + 'static/mobile-issue-detail-nav.js',
|
||||
BASE + 'static/mobile-update-detail-nav.js',
|
||||
BASE + 'static/mobile-review-detail-nav.js',
|
||||
BASE + 'static/checklist-conflict.js',
|
||||
BASE + 'static/voice-transcript-store.js',
|
||||
BASE + 'static/voice-issue-capture.js',
|
||||
|
|
|
|||
146
tests/test_mobile_review_detail_navigation.py
Normal file
146
tests/test_mobile_review_detail_navigation.py
Normal file
|
|
@ -0,0 +1,146 @@
|
|||
import json
|
||||
import subprocess
|
||||
from pathlib import Path
|
||||
|
||||
|
||||
FRONTEND = Path(__file__).resolve().parents[1] / "frontend"
|
||||
CONTROLLER = FRONTEND / "mobile-review-detail-nav.js"
|
||||
|
||||
|
||||
def test_review_navigation_moves_between_sections_and_focuses_feedback():
|
||||
script = f"""
|
||||
const createNavigation = require({json.dumps(str(CONTROLLER))});
|
||||
class FakeElement {{
|
||||
constructor(name) {{
|
||||
this.name=name; this.listeners={{}}; this.attributes={{}}; this.focuses=0; this.scrolls=[];
|
||||
}}
|
||||
addEventListener(name, callback) {{ this.listeners[name]=callback; }}
|
||||
removeEventListener(name) {{ delete this.listeners[name]; }}
|
||||
setAttribute(name, value) {{ this.attributes[name]=value; }}
|
||||
removeAttribute(name) {{ delete this.attributes[name]; }}
|
||||
focus(options) {{ this.focuses += 1; this.focusOptions=options; }}
|
||||
scrollIntoView(options) {{ this.scrolls.push(options); }}
|
||||
}}
|
||||
const names=['overview','files','feedback','history'];
|
||||
const buttons=Object.fromEntries(names.map(name => [name,new FakeElement(name)]));
|
||||
const targets=Object.fromEntries(names.map(name => [name,new FakeElement(name)]));
|
||||
const summaryComposer=new FakeElement('summary');
|
||||
const navigation=createNavigation({{
|
||||
buttons, targets, summaryComposer,
|
||||
prefersReducedMotion:() => true,
|
||||
}});
|
||||
navigation.start();
|
||||
navigation.navigate('files');
|
||||
navigation.navigate('feedback');
|
||||
const currentAfterFeedback=Object.fromEntries(Object.entries(buttons).map(([name,button]) => [name,button.attributes['aria-current'] || null]));
|
||||
navigation.reset();
|
||||
const currentAfterReset=Object.fromEntries(Object.entries(buttons).map(([name,button]) => [name,button.attributes['aria-current'] || null]));
|
||||
navigation.stop();
|
||||
process.stdout.write(JSON.stringify({{
|
||||
files:targets.files.scrolls,
|
||||
feedback:targets.feedback.scrolls,
|
||||
feedbackFocuses:summaryComposer.focuses,
|
||||
focusOptions:summaryComposer.focusOptions,
|
||||
currentAfterFeedback,
|
||||
currentAfterReset,
|
||||
listenersAfterStop:Object.values(buttons).map(button => Object.keys(button.listeners).length),
|
||||
}}));
|
||||
"""
|
||||
result = subprocess.run(["node", "-e", script], capture_output=True, text=True)
|
||||
|
||||
assert result.returncode == 0, result.stderr
|
||||
assert json.loads(result.stdout) == {
|
||||
"files": [{"block": "start", "behavior": "auto"}],
|
||||
"feedback": [{"block": "start", "behavior": "auto"}],
|
||||
"feedbackFocuses": 1,
|
||||
"focusOptions": {"preventScroll": True},
|
||||
"currentAfterFeedback": {
|
||||
"overview": None,
|
||||
"files": None,
|
||||
"feedback": "location",
|
||||
"history": None,
|
||||
},
|
||||
"currentAfterReset": {
|
||||
"overview": "location",
|
||||
"files": None,
|
||||
"feedback": None,
|
||||
"history": None,
|
||||
},
|
||||
"listenersAfterStop": [0, 0, 0, 0],
|
||||
}
|
||||
|
||||
|
||||
def test_review_navigation_tracks_the_visible_section_inside_the_review_panel():
|
||||
script = f"""
|
||||
const createNavigation = require({json.dumps(str(CONTROLLER))});
|
||||
const makeElement=name => ({{
|
||||
name, attributes:{{}},
|
||||
addEventListener() {{}}, removeEventListener() {{}},
|
||||
setAttribute(key,value) {{ this.attributes[key]=value; }},
|
||||
removeAttribute(key) {{ delete this.attributes[key]; }},
|
||||
}});
|
||||
const buttons=Object.fromEntries(['overview','files','feedback','history'].map(name => [name,makeElement(name)]));
|
||||
const targets=Object.fromEntries(['overview','files','feedback','history'].map(name => [name,makeElement(name)]));
|
||||
let callback;
|
||||
let observedRoot=null;
|
||||
let disconnected=0;
|
||||
const panel=makeElement('panel');
|
||||
const navigation=createNavigation({{
|
||||
buttons, targets, root:panel,
|
||||
observe(handler, observedTargets, root) {{
|
||||
callback=handler; observedRoot=root;
|
||||
return {{disconnect() {{ disconnected += 1; }}}};
|
||||
}},
|
||||
}});
|
||||
navigation.start();
|
||||
callback([
|
||||
{{target:targets.files,isIntersecting:true,intersectionRatio:.25}},
|
||||
{{target:targets.feedback,isIntersecting:true,intersectionRatio:.8}},
|
||||
]);
|
||||
const current=Object.fromEntries(Object.entries(buttons).map(([name,button]) => [name,button.attributes['aria-current'] || null]));
|
||||
navigation.stop();
|
||||
process.stdout.write(JSON.stringify({{current,rootMatches:observedRoot === panel,disconnected}}));
|
||||
"""
|
||||
result = subprocess.run(["node", "-e", script], capture_output=True, text=True)
|
||||
|
||||
assert result.returncode == 0, result.stderr
|
||||
assert json.loads(result.stdout) == {
|
||||
"current": {
|
||||
"overview": None,
|
||||
"files": None,
|
||||
"feedback": "location",
|
||||
"history": None,
|
||||
},
|
||||
"rootMatches": True,
|
||||
"disconnected": 1,
|
||||
}
|
||||
|
||||
|
||||
def test_review_sheet_wires_a_mobile_safe_area_navigation_rail_and_offline_asset():
|
||||
html = (FRONTEND / "index.html").read_text()
|
||||
css = (FRONTEND / "dashboard.css").read_text()
|
||||
dashboard_js = (FRONTEND / "dashboard.js").read_text()
|
||||
service_worker = (FRONTEND / "service-worker.js").read_text()
|
||||
|
||||
assert '<nav class="mobile-review-detail-nav" aria-label="Review sections">' in html
|
||||
for name, label in (
|
||||
("overview", "Overview"),
|
||||
("files", "Files"),
|
||||
("feedback", "Feedback"),
|
||||
("history", "History"),
|
||||
):
|
||||
assert f'data-review-section="{name}"' in html
|
||||
assert f">{label}</button>" in html
|
||||
assert 'id="review-overview"' in html
|
||||
assert 'id="review-files-workspace"' in html
|
||||
assert 'id="review-feedback"' in html
|
||||
assert 'id="review-history-workspace"' in html
|
||||
assert '<script src="static/mobile-review-detail-nav.js"></script>' in html
|
||||
assert "createMobileReviewDetailNavigation({" in dashboard_js
|
||||
assert "mobileReviewDetailNavigation.reset();" in dashboard_js
|
||||
assert ".mobile-review-detail-nav" in css
|
||||
assert "position:sticky" in css
|
||||
assert "min-height:44px" in css
|
||||
assert "env(safe-area-inset-top)" in css
|
||||
assert "@media (min-width:601px)" in css
|
||||
assert "BASE + 'static/mobile-review-detail-nav.js'" in service_worker
|
||||
|
|
@ -1000,6 +1000,7 @@ def test_install_precaches_complete_subpath_scoped_app_shell():
|
|||
"/dashboard/static/issue-sheet.js",
|
||||
"/dashboard/static/mobile-issue-detail-nav.js",
|
||||
"/dashboard/static/mobile-update-detail-nav.js",
|
||||
"/dashboard/static/mobile-review-detail-nav.js",
|
||||
"/dashboard/static/checklist-conflict.js",
|
||||
"/dashboard/static/voice-transcript-store.js",
|
||||
"/dashboard/static/voice-issue-capture.js",
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user