mirror of
https://github.com/saphid/frame-control.git
synced 2026-10-06 03:00:18 +02:00
Offer Use now only where a reconnect goes, and keep newer edits and headsets apart
From the second independent review of this PR: - Use now sends a plain reconnect, which picks the first-ranked address that answers, but it was offered on every tested address listed above the one in use. The devices list now carries each address's rank on the current network (frame_devices.order_addresses), and only the address a reconnect would pick gets the button. - Saving one address, cancelling, then editing another: the first save's answer closed the second editor and lost what was typed. Each edit now has its own session, and a late answer leaves a newer one alone. - Switching headsets with the dialog open drew the address offer from the previous headset's status before it was cleared, so Add could save its IP to the new headset. The dialog now renders after the old status is cleared. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
1 parent
0520082586
commit
22bb9c6469
4 files changed
+44
-14
No files matched your search
+4
-3
@@ -78,9 +78,10 @@ can add, edit, reorder and remove them without leaving the page you're on. When
|
||||
the headset reports a LAN IP on the same network as this computer and that IP
|
||||
isn't saved, it offers to add it. The offer puts the address first in the list,
|
||||
so on that network it wins over the Tailscale name; away from home the Tailscale
|
||||
name still leads. A new or edited address is tested straight away. When a tested
|
||||
address ranks above the one in use, **Use now** reconnects through it; otherwise
|
||||
the change takes effect the next time Frame Control connects.
|
||||
name still leads. A new or edited address is tested straight away. When an address
|
||||
that would be tried before the one in use passes the test, it gets **Use now**,
|
||||
which reconnects; the reconnect picks the first address that answers, which is
|
||||
that one. Otherwise the change takes effect the next time Frame Control connects.
|
||||
|
||||
## Networks
|
||||
|
||||
|
||||
@@ -304,6 +304,19 @@ class Connecting(unittest.TestCase):
|
||||
self.assertEqual(self.link.active_device()["alias"], "frame-bare")
|
||||
self.assertEqual(self.routes[-1][0], "frame-bare")
|
||||
|
||||
def test_devices_view_ranks_addresses_for_the_current_network(self):
|
||||
# The page offers Use now only on the address a reconnect would pick: rank says which.
|
||||
d = self.device("frame.tail1234.ts.net")
|
||||
self.reg.add_address(d["id"], "192.168.1.40", kind="lan", first=True)
|
||||
self.reg.record_success(d["id"], "frame.tail1234.ts.net", "n-home", 6.0)
|
||||
self.link.state["network"] = {"id": "n-home", "name": "Home", "tailscale": {"up": True}}
|
||||
view = next(x for x in fl.devices_view(self.link)["devices"] if x["id"] == d["id"])
|
||||
self.assertEqual({a["host"]: a["rank"] for a in view["addresses"]},
|
||||
{"192.168.1.40": 1, "frame.tail1234.ts.net": 0}) # only Tailscale worked here so far
|
||||
self.reg.record_success(d["id"], "192.168.1.40", "n-home", 1.0)
|
||||
view = next(x for x in fl.devices_view(self.link)["devices"] if x["id"] == d["id"])
|
||||
self.assertEqual([a["rank"] for a in view["addresses"]], [0, 1]) # now the user's order decides
|
||||
|
||||
def test_removing_the_headset_frame_alias_named_doesnt_bring_it_back_bare(self):
|
||||
d = self.device("localhost")
|
||||
self.link.override = self.link.session_alias = "frame-t"
|
||||
|
||||
+8
-2
@@ -1017,6 +1017,8 @@ def devices_view(link):
|
||||
snap = link.reg.snapshot()
|
||||
active = link.active_device()
|
||||
names = {nid: link.reg.network_name(dict(n, id=nid)) for nid, n in snap["networks"].items()}
|
||||
net = link.state["network"] or {}
|
||||
tailscale_up = bool((net.get("tailscale") or {}).get("up"))
|
||||
devices = []
|
||||
bare = link.bare(link.session_alias) if link.session_alias and not link.reg.by_alias(link.session_alias) else None
|
||||
for extra in ([active] if active.get("transient") and not active.get("none") else []) + \
|
||||
@@ -1027,8 +1029,12 @@ def devices_view(link):
|
||||
view = {k: v for k, v in d.items() if k not in ("config_host", "addresses")}
|
||||
view["active"] = d["id"] == active["id"]
|
||||
view["pinned"] = frame_devices.pinned(d["id"])
|
||||
view["addresses"] = [dict(a, network_names=[names.get(n, "an unnamed network") for n in a["networks"]])
|
||||
for a in d["addresses"]]
|
||||
# rank: where the next connection on this network tries it (0 first), so the page can
|
||||
# tell which address a reconnect would pick.
|
||||
ranks = {a["host"]: i for i, (a, _) in
|
||||
enumerate(frame_devices.order_addresses(d["addresses"], net.get("id"), tailscale_up))}
|
||||
view["addresses"] = [dict(a, network_names=[names.get(n, "an unnamed network") for n in a["networks"]],
|
||||
rank=ranks[a["host"]]) for a in d["addresses"]]
|
||||
devices.append(view)
|
||||
return {"devices": devices, "active": active["id"], "network": link.state["network"],
|
||||
"networks": [dict(n, id=nid, display=names[nid]) for nid, n in snap["networks"].items()],
|
||||
|
||||
+19
-9
@@ -4324,7 +4324,7 @@ window.connBanner = renderBanner;
|
||||
|
||||
// The pill's dialog: how it's connected, and the headset's addresses, which can be added,
|
||||
// edited and reordered right here. The steps it took stay folded away while all is well.
|
||||
const cd = { editing: null, editDev: null, sig: "", phase: null };
|
||||
const cd = { editing: null, editDev: null, session: 0, sig: "", phase: null };
|
||||
function renderConnDlg() {
|
||||
const s = link.s;
|
||||
if (!s || !$("connDlg").open) return;
|
||||
@@ -4406,7 +4406,12 @@ function renderConnAddrs() {
|
||||
const refocus = cd.refocus ? [cd.refocus, "[data-ed]"]
|
||||
: held ? [held.dataset.host, ["[data-use]", "[data-mv='-1']", "[data-mv='1']", "[data-ed]", "[data-rm]"].find(q => document.activeElement.matches(q))] : null;
|
||||
cd.refocus = null;
|
||||
const usedAt = d.addresses.findIndex(a => a.host === inUse);
|
||||
// A reconnect picks the first-ranked address that answers: offer it when that's a tested
|
||||
// address other than the one in use.
|
||||
const used = d.addresses.find(a => a.host === inUse);
|
||||
const best = used && d.addresses.filter(a => (tested.get(a.host) || {}).ssh === "ok")
|
||||
.reduce((x, a) => !x || a.rank < x.rank ? a : x, null);
|
||||
const switchTo = best && best.rank < used.rank ? best.host : null;
|
||||
list.innerHTML = d.addresses.map((a, i) => {
|
||||
if (cd.editing === a.host && cd.editDev === d.id) return `<div class="item addr"><div class="addr-edit conn-edit">
|
||||
<input type="text" id="ceHost" value="${esc(a.host)}" maxlength="253" aria-label="Address">
|
||||
@@ -4420,7 +4425,7 @@ function renderConnAddrs() {
|
||||
return `<div class="item addr" data-host="${esc(a.host)}">
|
||||
<div class="grow"><div class="t">${esc(a.host)} <span class="tag">${esc(KIND_TAG[a.kind] || a.kind)}</span>${a.label ? ` <span class="sub">${esc(a.label)}</span>` : ""}</div>
|
||||
<div class="s">${result}</div></div>
|
||||
${t && t.ssh === "ok" && usedAt > i ? `<button class="small" data-use title="Reconnect through this address">Use now</button>` : ""}
|
||||
${a.host === switchTo ? `<button class="small" data-use title="Reconnect through this address">Use now</button>` : ""}
|
||||
<button class="small" data-mv="-1" title="Try earlier" aria-label="Move ${esc(a.host)} up"${i ? "" : " disabled"}>↑</button>
|
||||
<button class="small" data-mv="1" title="Try later" aria-label="Move ${esc(a.host)} down"${i < d.addresses.length - 1 ? "" : " disabled"}>↓</button>
|
||||
<button class="small" data-ed>Edit</button><button class="small danger" data-rm>Remove</button></div>`;
|
||||
@@ -4434,7 +4439,9 @@ function renderConnAddrs() {
|
||||
const host = row.dataset.host;
|
||||
row.querySelectorAll("[data-mv]").forEach(b => b.onclick = () =>
|
||||
devAction("Move " + host, { action: "address-move", id: d.id, host, delta: +b.dataset.mv }, b));
|
||||
row.querySelector("[data-ed]").onclick = () => { cd.editing = host; cd.editDev = d.id; renderConnAddrs(); $("ceHost").focus(); };
|
||||
row.querySelector("[data-ed]").onclick = () => {
|
||||
cd.editing = host; cd.editDev = d.id; cd.session++; renderConnAddrs(); $("ceHost").focus();
|
||||
};
|
||||
const use = row.querySelector("[data-use]");
|
||||
if (use) use.onclick = () => devAction(`Switch to ${host}`, { action: "retry", id: d.id }, use);
|
||||
row.querySelector("[data-rm]").onclick = e => {
|
||||
@@ -4443,7 +4450,7 @@ function renderConnAddrs() {
|
||||
};
|
||||
});
|
||||
if ($("ceSave")) {
|
||||
const host = cd.editing, a = d.addresses.find(x => x.host === host);
|
||||
const host = cd.editing, a = d.addresses.find(x => x.host === host), session = cd.session;
|
||||
let saving = false;
|
||||
const save = async e => {
|
||||
e.preventDefault();
|
||||
@@ -4453,14 +4460,16 @@ function renderConnAddrs() {
|
||||
saving = true;
|
||||
const res = await devAction(`Save ${host}`, { action: "address-update", id: d.id, host, newHost, kind: a.kind, label }, $("ceSave"));
|
||||
saving = false;
|
||||
if (!res) return; // refused: what was typed stays, to correct
|
||||
if (res) testQuietly(d.id);
|
||||
// Refused: what was typed stays, to correct. Cancelled or another address opened for
|
||||
// editing while this was on its way: leave that one alone.
|
||||
if (!res || cd.session !== session) return;
|
||||
cd.editing = null; cd.sig = ""; cd.refocus = newHost;
|
||||
renderConnAddrs();
|
||||
testQuietly(d.id);
|
||||
};
|
||||
$("ceSave").onclick = save;
|
||||
[$("ceHost"), $("ceLabel")].forEach(i => i.onkeydown = e => { if (e.key === "Enter") save(e); if (e.key === "Escape") { e.preventDefault(); $("ceCancel").click(); } });
|
||||
$("ceCancel").onclick = () => { cd.editing = null; cd.sig = ""; cd.refocus = host; renderConnAddrs(); };
|
||||
$("ceCancel").onclick = () => { cd.editing = null; cd.session++; cd.sig = ""; cd.refocus = host; renderConnAddrs(); };
|
||||
}
|
||||
}
|
||||
$("connAdd").onsubmit = async e => {
|
||||
@@ -4479,7 +4488,6 @@ function onConnection(s) {
|
||||
link.s = s; link.live = !s.local; link.lastData = Date.now();
|
||||
link.offset = Date.now() / 1000 - s.now;
|
||||
if (!link.live) return;
|
||||
renderPill(); renderConnDlg();
|
||||
const devId = s.device && s.device.id;
|
||||
window.connDevice = devId || null;
|
||||
window.connName = s.device ? s.device.name : null;
|
||||
@@ -4514,6 +4522,8 @@ function onConnection(s) {
|
||||
link.reload = true; // everything on the page was the other headset's
|
||||
$("battChip").hidden = true; closeDevPop();
|
||||
}
|
||||
// After the other headset's status is cleared: the dialog's offer reads it.
|
||||
renderPill(); renderConnDlg();
|
||||
if (s.phase !== link.phase || devId !== link.device) {
|
||||
if (s.phase === "connected") {
|
||||
log(`Connected to ${s.device.name} via ${viaText(s.via)} on ${netLabel(s)}`, "ok");
|
||||
|
||||
Reference in new issue
Block a user