Merge pull request 'Open each mobile update at its newest unread activity' (#760) from timmy/759-newest-update-activity into main
This commit is contained in:
commit
7c86f28346
|
|
@ -385,6 +385,9 @@ textarea { resize: vertical; min-height: 120px; }
|
||||||
.update-sheet-header button { min-height:44px; }
|
.update-sheet-header button { min-height:44px; }
|
||||||
.update-triage-progress { margin:10px 0; padding:8px 10px; border:1px solid #2a496e; border-radius:10px; background:#10213a; }
|
.update-triage-progress { margin:10px 0; padding:8px 10px; border:1px solid #2a496e; border-radius:10px; background:#10213a; }
|
||||||
.update-sheet-content { overflow-wrap:anywhere; white-space:pre-wrap; }
|
.update-sheet-content { overflow-wrap:anywhere; white-space:pre-wrap; }
|
||||||
|
.update-new-activity { scroll-margin-block:18px 96px; border-color:#60a5fa; box-shadow:0 0 0 2px rgba(96,165,250,.25); }
|
||||||
|
.update-new-activity::before { content:'New activity'; display:inline-block; margin-bottom:6px; padding:2px 7px; border-radius:999px; background:#1d4ed8; color:#fff; font-size:12px; font-weight:700; }
|
||||||
|
.update-new-activity-jump { position:sticky; bottom:92px; z-index:4; min-height:44px; width:100%; margin:10px 0; border-color:#60a5fa; background:#17345a; }
|
||||||
.update-reply { display:grid; gap:8px; margin-top:16px; }
|
.update-reply { display:grid; gap:8px; margin-top:16px; }
|
||||||
.update-reply textarea { width:100%; min-height:112px; resize:vertical; }
|
.update-reply textarea { width:100%; min-height:112px; resize:vertical; }
|
||||||
.update-reply button { min-height:44px; width:100%; }
|
.update-reply button { min-height:44px; width:100%; }
|
||||||
|
|
|
||||||
|
|
@ -910,6 +910,11 @@
|
||||||
refresh: refreshMyWorkView,
|
refresh: refreshMyWorkView,
|
||||||
select: qs,
|
select: qs,
|
||||||
});
|
});
|
||||||
|
const updateReadPosition = createUpdateReadPosition({
|
||||||
|
panel: qs('#update-sheet .update-sheet-panel'),
|
||||||
|
jump: qs('#jump-update-new-activity'),
|
||||||
|
});
|
||||||
|
qs('#jump-update-new-activity').addEventListener('click', () => updateReadPosition.jump());
|
||||||
const notificationReader = createNotificationReader({
|
const notificationReader = createNotificationReader({
|
||||||
load: fetchNotificationDetail,
|
load: fetchNotificationDetail,
|
||||||
getScope: () => confirmedOwnerLogin,
|
getScope: () => confirmedOwnerLogin,
|
||||||
|
|
@ -924,6 +929,7 @@
|
||||||
}
|
}
|
||||||
selectedUpdate = item;
|
selectedUpdate = item;
|
||||||
selectedUpdateDetail = null;
|
selectedUpdateDetail = null;
|
||||||
|
updateReadPosition.open(String(item.notification_id));
|
||||||
updateMentions.dismiss();
|
updateMentions.dismiss();
|
||||||
qs('#update-sheet').classList.add('open');
|
qs('#update-sheet').classList.add('open');
|
||||||
qs('#update-sheet-key').textContent = item.key || '';
|
qs('#update-sheet-key').textContent = item.key || '';
|
||||||
|
|
@ -2984,6 +2990,8 @@
|
||||||
qs('#load-older-update-comments').hidden = !Number.isInteger(state?.older_page);
|
qs('#load-older-update-comments').hidden = !Number.isInteger(state?.older_page);
|
||||||
qs('#update-conversation-status').textContent = comments.length ?
|
qs('#update-conversation-status').textContent = comments.length ?
|
||||||
comments.length + ' of ' + Math.max(state.total || 0, comments.length) + ' messages loaded.' : 'No comments yet.';
|
comments.length + ' of ' + Math.max(state.total || 0, comments.length) + ' messages loaded.' : 'No comments yet.';
|
||||||
|
const newest = qs('#update-comments .issue-comment:last-child');
|
||||||
|
updateReadPosition.ready(String(selectedUpdate?.notification_id || ''), newest);
|
||||||
}
|
}
|
||||||
|
|
||||||
function renderPullConversation(state) {
|
function renderPullConversation(state) {
|
||||||
|
|
|
||||||
|
|
@ -760,6 +760,7 @@
|
||||||
</div>
|
</div>
|
||||||
<h2>Full conversation</h2>
|
<h2>Full conversation</h2>
|
||||||
<div id="update-comments"></div>
|
<div id="update-comments"></div>
|
||||||
|
<button class="update-new-activity-jump" id="jump-update-new-activity" type="button" hidden>Jump to new activity</button>
|
||||||
<button class="conversation-more" id="load-older-update-comments" type="button" hidden>Load older messages</button>
|
<button class="conversation-more" id="load-older-update-comments" type="button" hidden>Load older messages</button>
|
||||||
<div id="update-conversation-status" class="small" aria-live="assertive"></div>
|
<div id="update-conversation-status" class="small" aria-live="assertive"></div>
|
||||||
<details>
|
<details>
|
||||||
|
|
@ -1074,6 +1075,7 @@
|
||||||
<script src="static/mobile-work-entry.js"></script>
|
<script src="static/mobile-work-entry.js"></script>
|
||||||
<script src="static/mobile-queue-launcher.js"></script>
|
<script src="static/mobile-queue-launcher.js"></script>
|
||||||
<script src="static/update-triage-session.js"></script>
|
<script src="static/update-triage-session.js"></script>
|
||||||
|
<script src="static/update-read-position.js"></script>
|
||||||
<script src="static/update-triage-launcher.js"></script>
|
<script src="static/update-triage-launcher.js"></script>
|
||||||
<script src="static/agenda-session-launcher.js"></script>
|
<script src="static/agenda-session-launcher.js"></script>
|
||||||
<script src="static/mobile-launch.js"></script>
|
<script src="static/mobile-launch.js"></script>
|
||||||
|
|
|
||||||
|
|
@ -77,6 +77,7 @@ const SHELL = [
|
||||||
BASE + 'static/mobile-work-entry.js',
|
BASE + 'static/mobile-work-entry.js',
|
||||||
BASE + 'static/mobile-queue-launcher.js',
|
BASE + 'static/mobile-queue-launcher.js',
|
||||||
BASE + 'static/update-triage-session.js',
|
BASE + 'static/update-triage-session.js',
|
||||||
|
BASE + 'static/update-read-position.js',
|
||||||
BASE + 'static/update-triage-launcher.js',
|
BASE + 'static/update-triage-launcher.js',
|
||||||
BASE + 'static/agenda-session-launcher.js',
|
BASE + 'static/agenda-session-launcher.js',
|
||||||
BASE + 'static/mobile-launch.js',
|
BASE + 'static/mobile-launch.js',
|
||||||
|
|
|
||||||
49
frontend/update-read-position.js
Normal file
49
frontend/update-read-position.js
Normal file
|
|
@ -0,0 +1,49 @@
|
||||||
|
(function (root, factory) {
|
||||||
|
if (typeof module === 'object' && module.exports) module.exports = factory;
|
||||||
|
else root.createUpdateReadPosition = factory;
|
||||||
|
})(typeof self !== 'undefined' ? self : this, function createUpdateReadPosition(options) {
|
||||||
|
const panel = options.panel;
|
||||||
|
const jump = options.jump;
|
||||||
|
let identity = '';
|
||||||
|
let openingTop = 0;
|
||||||
|
let movedByUser = false;
|
||||||
|
let positioned = false;
|
||||||
|
let target = null;
|
||||||
|
|
||||||
|
function moveToTarget() {
|
||||||
|
if (!target) return false;
|
||||||
|
target.classList?.add('update-new-activity');
|
||||||
|
target.scrollIntoView({ block:'end' });
|
||||||
|
positioned = true;
|
||||||
|
jump.hidden = true;
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
|
panel.addEventListener('scroll', () => {
|
||||||
|
if (identity && !positioned && panel.scrollTop !== openingTop) movedByUser = true;
|
||||||
|
}, { passive:true });
|
||||||
|
|
||||||
|
return {
|
||||||
|
open(nextIdentity) {
|
||||||
|
identity = String(nextIdentity || '');
|
||||||
|
openingTop = panel.scrollTop;
|
||||||
|
movedByUser = false;
|
||||||
|
positioned = false;
|
||||||
|
target = null;
|
||||||
|
jump.hidden = true;
|
||||||
|
},
|
||||||
|
ready(readyIdentity, newest) {
|
||||||
|
if (!identity || String(readyIdentity || '') !== identity || positioned || !newest) return false;
|
||||||
|
target = newest;
|
||||||
|
target.classList?.add('update-new-activity');
|
||||||
|
if (movedByUser) {
|
||||||
|
jump.hidden = false;
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
return moveToTarget();
|
||||||
|
},
|
||||||
|
jump() {
|
||||||
|
return moveToTarget();
|
||||||
|
},
|
||||||
|
};
|
||||||
|
});
|
||||||
|
|
@ -847,6 +847,7 @@ def test_install_precaches_complete_subpath_scoped_app_shell():
|
||||||
"/dashboard/static/mobile-work-entry.js",
|
"/dashboard/static/mobile-work-entry.js",
|
||||||
"/dashboard/static/mobile-queue-launcher.js",
|
"/dashboard/static/mobile-queue-launcher.js",
|
||||||
"/dashboard/static/update-triage-session.js",
|
"/dashboard/static/update-triage-session.js",
|
||||||
|
"/dashboard/static/update-read-position.js",
|
||||||
"/dashboard/static/update-triage-launcher.js",
|
"/dashboard/static/update-triage-launcher.js",
|
||||||
"/dashboard/static/agenda-session-launcher.js",
|
"/dashboard/static/agenda-session-launcher.js",
|
||||||
"/dashboard/static/mobile-launch.js",
|
"/dashboard/static/mobile-launch.js",
|
||||||
|
|
|
||||||
87
tests/test_update_read_position.py
Normal file
87
tests/test_update_read_position.py
Normal file
|
|
@ -0,0 +1,87 @@
|
||||||
|
import json
|
||||||
|
import subprocess
|
||||||
|
from pathlib import Path
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
|
||||||
|
from tests.dashboard_bundle import dashboard
|
||||||
|
|
||||||
|
|
||||||
|
POSITIONER = Path(__file__).resolve().parents[1] / "frontend" / "update-read-position.js"
|
||||||
|
|
||||||
|
|
||||||
|
def run_positioner(script: str) -> dict:
|
||||||
|
source = f"const createPositioner = require({json.dumps(str(POSITIONER))});\n" + script
|
||||||
|
result = subprocess.run(["node", "-e", source], capture_output=True, text=True)
|
||||||
|
assert result.returncode == 0, result.stderr
|
||||||
|
return json.loads(result.stdout)
|
||||||
|
|
||||||
|
|
||||||
|
def test_new_update_scrolls_to_newest_activity_once_without_reusing_previous_offset():
|
||||||
|
result = run_positioner("""
|
||||||
|
const listeners = {};
|
||||||
|
const panel = {scrollTop:480, addEventListener:(name, fn)=>listeners[name]=fn};
|
||||||
|
const jump = {hidden:true, focus(){}};
|
||||||
|
const first = {calls:0, scrollIntoView(){this.calls += 1}, classList:{add(){}}};
|
||||||
|
const second = {calls:0, scrollIntoView(){this.calls += 1}, classList:{add(){}}};
|
||||||
|
const positioner = createPositioner({panel, jump});
|
||||||
|
positioner.open('101');
|
||||||
|
positioner.ready('101', first);
|
||||||
|
positioner.ready('101', first);
|
||||||
|
panel.scrollTop = 920;
|
||||||
|
positioner.open('102');
|
||||||
|
positioner.ready('102', second);
|
||||||
|
process.stdout.write(JSON.stringify({first:first.calls, second:second.calls, jumpHidden:jump.hidden}));
|
||||||
|
""")
|
||||||
|
|
||||||
|
assert result == {"first": 1, "second": 1, "jumpHidden": True}
|
||||||
|
|
||||||
|
|
||||||
|
def test_user_scroll_during_loading_preserves_position_until_explicit_jump():
|
||||||
|
result = run_positioner("""
|
||||||
|
const listeners = {};
|
||||||
|
const panel = {scrollTop:120, addEventListener:(name, fn)=>listeners[name]=fn};
|
||||||
|
const jump = {hidden:true, focus(){}};
|
||||||
|
const target = {calls:0, scrollIntoView(){this.calls += 1}, classList:{add(){}}};
|
||||||
|
const positioner = createPositioner({panel, jump});
|
||||||
|
positioner.open('201');
|
||||||
|
panel.scrollTop = 260;
|
||||||
|
listeners.scroll();
|
||||||
|
positioner.ready('201', target);
|
||||||
|
const before = {calls:target.calls, hidden:jump.hidden, top:panel.scrollTop};
|
||||||
|
positioner.jump();
|
||||||
|
process.stdout.write(JSON.stringify({before, after:{calls:target.calls, hidden:jump.hidden}}));
|
||||||
|
""")
|
||||||
|
|
||||||
|
assert result == {
|
||||||
|
"before": {"calls": 0, "hidden": False, "top": 260},
|
||||||
|
"after": {"calls": 1, "hidden": True},
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
|
def test_stale_or_prefetched_conversation_cannot_move_the_visible_update():
|
||||||
|
result = run_positioner("""
|
||||||
|
const panel = {scrollTop:0, addEventListener(){}};
|
||||||
|
const jump = {hidden:true};
|
||||||
|
const stale = {calls:0, scrollIntoView(){this.calls += 1}, classList:{add(){}}};
|
||||||
|
const current = {calls:0, scrollIntoView(){this.calls += 1}, classList:{add(){}}};
|
||||||
|
const positioner = createPositioner({panel, jump});
|
||||||
|
positioner.open('302');
|
||||||
|
positioner.ready('301', stale);
|
||||||
|
positioner.ready('302', current);
|
||||||
|
process.stdout.write(JSON.stringify({stale:stale.calls, current:current.calls}));
|
||||||
|
""")
|
||||||
|
|
||||||
|
assert result == {"stale": 0, "current": 1}
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.anyio
|
||||||
|
async def test_dashboard_mounts_mobile_new_activity_positioning_flow():
|
||||||
|
html = await dashboard()
|
||||||
|
|
||||||
|
assert '<script src="static/update-read-position.js"></script>' in html
|
||||||
|
assert 'id="jump-update-new-activity"' in html
|
||||||
|
assert "updateReadPosition.open(String(item.notification_id))" in html
|
||||||
|
assert "updateReadPosition.ready(String(selectedUpdate?.notification_id || ''), newest)" in html
|
||||||
|
assert ".update-new-activity" in html
|
||||||
|
assert "BASE + 'static/update-read-position.js'" in (POSITIONER.parent / "service-worker.js").read_text()
|
||||||
Loading…
Reference in New Issue
Block a user