diff --git a/frontend/dashboard.css b/frontend/dashboard.css index c727ddb..41eacab 100644 --- a/frontend/dashboard.css +++ b/frontend/dashboard.css @@ -469,25 +469,26 @@ 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 { display:none; } +.mobile-issue-detail-nav, .mobile-pull-detail-nav { display:none; } @media (max-width:600px) { - .issue-sheet-panel { padding-top:max(12px,env(safe-area-inset-top)); } - .mobile-issue-detail-nav { + .issue-sheet-panel, .pull-sheet-panel { padding-top:max(12px,env(safe-area-inset-top)); } + .mobile-issue-detail-nav, .mobile-pull-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-issue-detail-nav button, .mobile-pull-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-issue-detail-nav button[aria-current="location"], .mobile-pull-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; } } -@media (min-width:601px) { .mobile-issue-detail-nav { display:none; } } +@media (min-width:601px) { .mobile-issue-detail-nav, .mobile-pull-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; } diff --git a/frontend/dashboard.js b/frontend/dashboard.js index 0eb92ff..fc372c4 100644 --- a/frontend/dashboard.js +++ b/frontend/dashboard.js @@ -29,6 +29,20 @@ prefersReducedMotion:() => window.matchMedia('(prefers-reduced-motion: reduce)').matches, }); mobileIssueDetailNavigation.start(); + const pullDetailPanel = qs('#pull-sheet .pull-sheet-panel'); + const mobilePullDetailNavigation = createMobileIssueDetailNavigation({ + root:pullDetailPanel, + buttons:Object.fromEntries(Array.from(document.querySelectorAll('[data-pull-section]')).map(button => [button.dataset.pullSection, button])), + targets:{ + overview:qs('#pull-overview'), + conversation:qs('#pull-conversation'), + reply:qs('#pull-comment'), + review:qs('#pull-review'), + }, + beforeNavigate:{review(target) { target.open = true; }}, + prefersReducedMotion:() => window.matchMedia('(prefers-reduced-motion: reduce)').matches, + }); + mobilePullDetailNavigation.start(); [ [qs('.app-menu'), qs('#app-menu-toggle')], [qs('.work-settings'), qs('#work-settings-toggle')], diff --git a/frontend/index.html b/frontend/index.html index 2e8eac5..3d3a878 100644 --- a/frontend/index.html +++ b/frontend/index.html @@ -1208,13 +1208,23 @@

Assigned pull request

-
Choose a pull request.
- -
-

Full conversation

- -
- + +
+
Choose a pull request.
+ +
+
+
+

Full conversation

+ +
+ +

Add comment

diff --git a/frontend/mobile-issue-detail-nav.js b/frontend/mobile-issue-detail-nav.js index 79dfcec..deba34e 100644 --- a/frontend/mobile-issue-detail-nav.js +++ b/frontend/mobile-issue-detail-nav.js @@ -17,6 +17,8 @@ function createMobileIssueDetailNavigation(options) { function navigate(name) { const target = targets[name]; if (!target) return false; + const prepare = options.beforeNavigate && options.beforeNavigate[name]; + if (prepare) prepare(target); if (name === 'actions' && options.planning) options.planning.open = true; target.scrollIntoView({ block: 'start', diff --git a/tests/test_mobile_pull_detail_navigation.py b/tests/test_mobile_pull_detail_navigation.py new file mode 100644 index 0000000..34922e6 --- /dev/null +++ b/tests/test_mobile_pull_detail_navigation.py @@ -0,0 +1,86 @@ +import json +import subprocess +from pathlib import Path + +FRONTEND = Path(__file__).resolve().parents[1] / "frontend" +CONTROLLER = FRONTEND / "mobile-issue-detail-nav.js" + + +def test_pull_navigation_prepares_review_before_scrolling_and_focuses_reply(): + script = f""" +const createNavigation = require({json.dumps(str(CONTROLLER))}); +class FakeElement {{ + constructor(name) {{ this.name=name; this.listeners={{}}; this.attributes={{}}; this.open=false; 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() {{ this.focuses += 1; }} + scrollIntoView(options) {{ this.scrolls.push({{...options, openWhenScrolled:this.open}}); }} +}} +const buttons = Object.fromEntries(['overview','conversation','reply','review'].map(name => [name,new FakeElement(name)])); +const targets = Object.fromEntries(['overview','conversation','reply','review'].map(name => [name,new FakeElement(name)])); +let prepared = []; +const navigation = createNavigation({{ + buttons, targets, + beforeNavigate:{{review(target) {{ prepared.push('review'); target.open=true; }}}}, + prefersReducedMotion:() => true, +}}); +navigation.start(); +navigation.navigate('reply'); +navigation.navigate('review'); +process.stdout.write(JSON.stringify({{ + replyFocuses:targets.reply.focuses, + reviewScrolls:targets.review.scrolls, + prepared, + current:Object.fromEntries(Object.entries(buttons).map(([name,button]) => [name,button.attributes['aria-current'] || null])), +}})); +""" + result = subprocess.run(["node", "-e", script], capture_output=True, text=True) + + assert result.returncode == 0, result.stderr + assert json.loads(result.stdout) == { + "replyFocuses": 1, + "reviewScrolls": [{"block": "start", "behavior": "auto", "openWhenScrolled": True}], + "prepared": ["review"], + "current": { + "overview": None, + "conversation": None, + "reply": None, + "review": "location", + }, + } + + +def test_pull_sheet_wires_four_workspace_destinations_to_existing_lazy_review(): + html = (FRONTEND / "index.html").read_text() + dashboard_js = (FRONTEND / "dashboard.js").read_text() + + assert '