feat: add mobile pull workspace navigation (Closes #965)
This commit is contained in:
parent
762614b2d3
commit
311396cfac
|
|
@ -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-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 { display:flex; align-items:center; justify-content:space-between; gap:10px; }
|
||||||
.issue-sheet-header button { min-height:44px; }
|
.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) {
|
@media (max-width:600px) {
|
||||||
.issue-sheet-panel { padding-top:max(12px,env(safe-area-inset-top)); }
|
.issue-sheet-panel, .pull-sheet-panel { padding-top:max(12px,env(safe-area-inset-top)); }
|
||||||
.mobile-issue-detail-nav {
|
.mobile-issue-detail-nav, .mobile-pull-detail-nav {
|
||||||
position:sticky; top:env(safe-area-inset-top); z-index:6;
|
position:sticky; top:env(safe-area-inset-top); z-index:6;
|
||||||
display:grid; grid-template-columns:repeat(4,minmax(0,1fr)); gap:4px;
|
display:grid; grid-template-columns:repeat(4,minmax(0,1fr)); gap:4px;
|
||||||
margin:8px -6px 12px; padding:6px;
|
margin:8px -6px 12px; padding:6px;
|
||||||
background:rgba(11,21,38,.98); border-block:1px solid #2a496e;
|
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;
|
min-width:0; min-height:44px; padding:4px; overflow-wrap:anywhere;
|
||||||
border-color:transparent; font-size:12px;
|
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;
|
border-color:#60a5fa; background:#17365a; color:#fff;
|
||||||
}
|
}
|
||||||
#issue-overview, #issue-conversation, #issue-comment, #issue-planning { scroll-margin-top:72px; }
|
#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 { 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[hidden] { display:none; }
|
||||||
.completed-filed-actions button { min-height:44px; min-width:0; }
|
.completed-filed-actions button { min-height:44px; min-width:0; }
|
||||||
|
|
|
||||||
|
|
@ -29,6 +29,20 @@
|
||||||
prefersReducedMotion:() => window.matchMedia('(prefers-reduced-motion: reduce)').matches,
|
prefersReducedMotion:() => window.matchMedia('(prefers-reduced-motion: reduce)').matches,
|
||||||
});
|
});
|
||||||
mobileIssueDetailNavigation.start();
|
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('.app-menu'), qs('#app-menu-toggle')],
|
||||||
[qs('.work-settings'), qs('#work-settings-toggle')],
|
[qs('.work-settings'), qs('#work-settings-toggle')],
|
||||||
|
|
|
||||||
|
|
@ -1208,13 +1208,23 @@
|
||||||
<div><div class="small" id="pull-sheet-key"></div><h3 id="pull-sheet-title">Assigned pull request</h3></div>
|
<div><div class="small" id="pull-sheet-key"></div><h3 id="pull-sheet-title">Assigned pull request</h3></div>
|
||||||
<button id="close-pull-sheet" type="button">Close</button>
|
<button id="close-pull-sheet" type="button">Close</button>
|
||||||
</div>
|
</div>
|
||||||
<div id="pull-sheet-status" class="small" aria-live="polite">Choose a pull request.</div>
|
<nav class="mobile-detail-nav mobile-pull-detail-nav" aria-label="Pull request sections">
|
||||||
<button class="pull-retry" id="retry-pull-load" type="button" hidden>Retry loading pull request</button>
|
<button type="button" data-pull-section="overview">Overview</button>
|
||||||
<div class="pull-sheet-content markdown-content" id="pull-sheet-body"></div>
|
<button type="button" data-pull-section="conversation">Conversation</button>
|
||||||
<h2>Full conversation</h2><div id="pull-comments"></div>
|
<button type="button" data-pull-section="reply">Reply</button>
|
||||||
<button class="conversation-more" id="load-older-pull-comments" type="button" hidden>Load older messages</button>
|
<button type="button" data-pull-section="review">Review & merge</button>
|
||||||
<div id="pull-conversation-status" class="small" aria-live="assertive"></div>
|
</nav>
|
||||||
<button class="conversation-actions-retry" id="retry-pull-comment-actions" type="button" hidden>Retry comment tools</button>
|
<div id="pull-overview">
|
||||||
|
<div id="pull-sheet-status" class="small" aria-live="polite">Choose a pull request.</div>
|
||||||
|
<button class="pull-retry" id="retry-pull-load" type="button" hidden>Retry loading pull request</button>
|
||||||
|
<div class="pull-sheet-content markdown-content" id="pull-sheet-body"></div>
|
||||||
|
</div>
|
||||||
|
<section id="pull-conversation" tabindex="-1">
|
||||||
|
<h2>Full conversation</h2><div id="pull-comments"></div>
|
||||||
|
<button class="conversation-more" id="load-older-pull-comments" type="button" hidden>Load older messages</button>
|
||||||
|
<div id="pull-conversation-status" class="small" aria-live="assertive"></div>
|
||||||
|
<button class="conversation-actions-retry" id="retry-pull-comment-actions" type="button" hidden>Retry comment tools</button>
|
||||||
|
</section>
|
||||||
<section class="pull-comment-composer" aria-labelledby="pull-comment-title">
|
<section class="pull-comment-composer" aria-labelledby="pull-comment-title">
|
||||||
<h2 id="pull-comment-title">Add comment</h2>
|
<h2 id="pull-comment-title">Add comment</h2>
|
||||||
<textarea id="pull-comment" maxlength="10000" placeholder="Write a comment"></textarea>
|
<textarea id="pull-comment" maxlength="10000" placeholder="Write a comment"></textarea>
|
||||||
|
|
|
||||||
|
|
@ -17,6 +17,8 @@ function createMobileIssueDetailNavigation(options) {
|
||||||
function navigate(name) {
|
function navigate(name) {
|
||||||
const target = targets[name];
|
const target = targets[name];
|
||||||
if (!target) return false;
|
if (!target) return false;
|
||||||
|
const prepare = options.beforeNavigate && options.beforeNavigate[name];
|
||||||
|
if (prepare) prepare(target);
|
||||||
if (name === 'actions' && options.planning) options.planning.open = true;
|
if (name === 'actions' && options.planning) options.planning.open = true;
|
||||||
target.scrollIntoView({
|
target.scrollIntoView({
|
||||||
block: 'start',
|
block: 'start',
|
||||||
|
|
|
||||||
113
tests/test_mobile_pull_detail_navigation.py
Normal file
113
tests/test_mobile_pull_detail_navigation.py
Normal file
|
|
@ -0,0 +1,113 @@
|
||||||
|
import json
|
||||||
|
import subprocess
|
||||||
|
from pathlib import Path
|
||||||
|
|
||||||
|
from playwright.sync_api import sync_playwright
|
||||||
|
|
||||||
|
|
||||||
|
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 '<nav class="mobile-detail-nav mobile-pull-detail-nav"' in html
|
||||||
|
assert 'aria-label="Pull request sections"' in html
|
||||||
|
for name, label in (
|
||||||
|
("overview", "Overview"),
|
||||||
|
("conversation", "Conversation"),
|
||||||
|
("reply", "Reply"),
|
||||||
|
("review", "Review & merge"),
|
||||||
|
):
|
||||||
|
assert f'data-pull-section="{name}"' in html
|
||||||
|
assert f">{label}</button>" in html
|
||||||
|
assert 'id="pull-overview"' in html
|
||||||
|
assert 'id="pull-conversation"' in html
|
||||||
|
assert 'id="pull-comment"' in html
|
||||||
|
assert 'id="pull-review"' in html
|
||||||
|
assert "document.querySelectorAll('[data-pull-section]')" in dashboard_js
|
||||||
|
assert "beforeNavigate:{review(target) { target.open = true; }}" in dashboard_js
|
||||||
|
|
||||||
|
|
||||||
|
def test_pull_workspace_rail_is_phone_contained_and_desktop_hidden():
|
||||||
|
html = (FRONTEND / "index.html").read_text()
|
||||||
|
css = (FRONTEND / "dashboard.css").read_text()
|
||||||
|
rendered = html.replace(
|
||||||
|
'<link rel="stylesheet" href="static/dashboard.css" />',
|
||||||
|
f"<style>{css}</style>",
|
||||||
|
)
|
||||||
|
|
||||||
|
with sync_playwright() as playwright:
|
||||||
|
browser = playwright.chromium.launch()
|
||||||
|
page = browser.new_page(viewport={"width": 320, "height": 568})
|
||||||
|
page.set_content(rendered, wait_until="domcontentloaded")
|
||||||
|
page.locator("#pull-sheet").evaluate("node => node.classList.add('open')")
|
||||||
|
for width, height in ((320, 568), (390, 844)):
|
||||||
|
page.set_viewport_size({"width": width, "height": height})
|
||||||
|
phone = page.locator(".mobile-pull-detail-nav").evaluate(
|
||||||
|
"""nav => ({
|
||||||
|
display:getComputedStyle(nav).display,
|
||||||
|
navWidth:nav.getBoundingClientRect().width,
|
||||||
|
panelWidth:nav.closest('.pull-sheet-panel').getBoundingClientRect().width,
|
||||||
|
panelOverflow:nav.closest('.pull-sheet-panel').scrollWidth > nav.closest('.pull-sheet-panel').clientWidth,
|
||||||
|
heights:Array.from(nav.querySelectorAll('button')).map(button => button.getBoundingClientRect().height),
|
||||||
|
position:getComputedStyle(nav).position,
|
||||||
|
})"""
|
||||||
|
)
|
||||||
|
assert phone["display"] == "grid"
|
||||||
|
assert phone["position"] == "sticky"
|
||||||
|
assert phone["navWidth"] <= phone["panelWidth"]
|
||||||
|
assert phone["panelOverflow"] is False
|
||||||
|
assert min(phone["heights"]) >= 44
|
||||||
|
|
||||||
|
page.set_viewport_size({"width": 900, "height": 700})
|
||||||
|
assert page.locator(".mobile-pull-detail-nav").evaluate("nav => getComputedStyle(nav).display") == "none"
|
||||||
|
browser.close()
|
||||||
Loading…
Reference in New Issue
Block a user