diff --git a/docs/devices.md b/docs/devices.md index 847d8b7..ac8e9e5 100644 --- a/docs/devices.md +++ b/docs/devices.md @@ -76,8 +76,12 @@ services and checks `ALIAS.local` and `frame.local`. **The pill's dialog** lists the same addresses, with what each one answered, and 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, so at home the app connects directly rather -than over Tailscale. A new or edited address is tested straight away. +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 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 diff --git a/tests/test_devices.py b/tests/test_devices.py index 31f3c61..dde3bea 100644 --- a/tests/test_devices.py +++ b/tests/test_devices.py @@ -296,6 +296,20 @@ class Pins(Base): class Registry(Base): + def test_an_address_added_first_wins_on_its_own_network(self): + # The page's "Add 192.168.x.x" offer: the headset is reached over Tailscale, which has + # worked here before. The LAN address has to go ahead of it to be used at home. + d = self.reg.add_device("frame-4", hosts=["frame.tail1234.ts.net"]) + self.reg.record_success(d["id"], "frame.tail1234.ts.net", "n-home", 6.0) + self.reg.add_address(d["id"], "192.168.1.40", kind="lan", first=True) + self.reg.record_success(d["id"], "192.168.1.40", "n-home", 1.0) # Test now found it + addrs = self.reg.get(d["id"])["addresses"] + self.assertEqual([a["host"] for a in addrs], ["192.168.1.40", "frame.tail1234.ts.net"]) + at_home = [a["host"] for a, _ in fd.order_addresses(addrs, "n-home", True)] + self.assertEqual(at_home[0], "192.168.1.40") + away = [a["host"] for a, _ in fd.order_addresses(addrs, "n-cafe", True)] + self.assertEqual(away[0], "frame.tail1234.ts.net") # elsewhere Tailscale still leads + def test_address_editing(self): d = self.reg.add_device("frame-3", hosts=["192.168.1.40"]) a = self.reg.add_address(d["id"], "frame-3.local", label="mDNS") diff --git a/tests/test_link.py b/tests/test_link.py index 782306e..85d1802 100644 --- a/tests/test_link.py +++ b/tests/test_link.py @@ -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" diff --git a/ui/frame_devices.py b/ui/frame_devices.py index 8345302..ba6f5ac 100644 --- a/ui/frame_devices.py +++ b/ui/frame_devices.py @@ -623,7 +623,9 @@ class Registry: return a raise DeviceError(f"{host} isn't one of this headset's addresses") - def add_address(self, device_id, host, kind=None, label=""): + def add_address(self, device_id, host, kind=None, label="", first=False): + """Add an address at the end of the list, or at the front (first=True), where the + user's order makes it win over the others that work on the same network.""" with self._changing(): d = self._find(device_id) a = new_address(host, kind, label) @@ -631,7 +633,7 @@ class Registry: raise DeviceError(f"{a['host']} is already on the list") if len(d["addresses"]) >= 32: raise DeviceError("That's enough addresses for one headset") - d["addresses"].append(a) + d["addresses"].insert(0 if first else len(d["addresses"]), a) self.save() return copy.deepcopy(a) diff --git a/ui/frame_link.py b/ui/frame_link.py index 51979c6..46a01b5 100644 --- a/ui/frame_link.py +++ b/ui/frame_link.py @@ -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()], @@ -1111,7 +1117,8 @@ def devices_action(link, body, open_setup, busy=lambda: 0): raise frame_devices.DeviceError(f"Removed, but couldn't edit ~/.ssh/config: {e}") msg = f"Removed {d['name']}" + (f" and its '{d['alias']}' entry in ~/.ssh/config" if removed else "") elif action == "address-add": - a = reg.add_address(did, body.get("host"), body.get("kind") or None, body.get("label") or "") + a = reg.add_address(did, body.get("host"), body.get("kind") or None, body.get("label") or "", + first=body.get("first") is True) if is_active and link.state["phase"] == "failed": link.kick("retry") msg = f"Added {a['host']}" diff --git a/ui/index.html b/ui/index.html index 169c061..9ac7a9b 100644 --- a/ui/index.html +++ b/ui/index.html @@ -219,7 +219,7 @@ /* Every drop-down dark, like the fields; the panels' own rules below still size them. */ select { background: rgba(0,0,0,.28); color: var(--text); border: 1px solid transparent; border-radius: 3px; padding: 7px 8px; font: inherit; font-size: 13px; max-width: 100%; min-width: 0; } - select:focus { outline: none; border-color: var(--blue); } + select:focus-visible { outline: 2px solid var(--blue); outline-offset: 1px; } option { background: #1e2329; color: var(--text); } .row { display: flex; gap: 8px; align-items: center; flex-wrap: wrap; } @@ -294,6 +294,7 @@ padding: 16px; border: 1px solid rgba(255,255,255,.1); border-radius: 6px; background: #1e2329; color: var(--text); box-shadow: 0 20px 60px rgba(0,0,0,.6); -webkit-app-region: no-drag; } .pop .shelf-head { margin-bottom: 10px; } + .pop .shelf-head h2 { min-width: 0; overflow: hidden; text-overflow: ellipsis; } .pop .stats { margin-top: 12px; } .pop .chips { margin-top: 12px; } .batt .state { font-weight: 700; font-size: 12.5px; letter-spacing: 1.2px; text-transform: uppercase; } @@ -699,7 +700,7 @@ .detail-layout { display:flex; flex-direction:column; gap:24px; } .detail-buy { order:-1; width:100%; } .sources-dialog { padding:24px; } .sources-dialog .store-close { margin:-12px -12px -20px 0; } } - @media(prefers-reduced-motion:reduce) { html { scroll-behavior:auto; } .store-card, .source-toggle::after { transition:none; } .store-skeleton { animation:none; } } + @media(prefers-reduced-motion:reduce) { html, #scroll { scroll-behavior:auto; } .store-card, .source-toggle::after { transition:none; } .store-skeleton { animation:none; } } @@ -1787,9 +1788,8 @@ function battery(b, power) { $("battChip").innerHTML = pct == null ? "โ€”" : `${bolt}${pct}%`; $("battChip").title = `${label}. Click for the headset's details.`; - $("battChip").setAttribute("aria-label", `Battery ${pct}%, ${label}. Headset details`); - $("battChip").hidden = pct == null; - if (pct == null) closeDevPop(); + $("battChip").setAttribute("aria-label", pct == null ? "Headset details" : `Battery ${pct}%, ${label}. Headset details`); + $("battChip").hidden = false; // even without a battery reading: it's the way to the headset's details } // hidePopover throws where the menu isn't open (and doesn't exist in older browsers). function closeDevPop() { try { $("devPop").hidePopover(); } catch {} } @@ -1798,13 +1798,14 @@ $("devPop").addEventListener("beforetoggle", e => { if (e.newState !== "open") return; const r = $("battChip").getBoundingClientRect(); $("devPop").style.top = `${Math.round(r.bottom + 8)}px`; - $("devPop").style.right = `${Math.max(12, Math.round(innerWidth - r.right))}px`; + const width = Math.min(390, innerWidth - 24); // as .pop's CSS width + $("devPop").style.right = `${Math.max(12, Math.min(Math.round(innerWidth - r.right), innerWidth - width - 12))}px`; }); function render(s) { renderPerformance(s); battery(s.battery, s.power); - $("devPopTitle").textContent = window.connName || "Headset"; + $("devPopTitle").textContent = $("devPopTitle").title = window.connName || "Headset"; $("devName").textContent = `${s.hostname} ยท ${s.ip ?? "no IP"}`; $("updated").textContent = new Date(s.time * 1000).toLocaleTimeString(); const h = s.disk.home; @@ -4394,7 +4395,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, 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; @@ -4432,7 +4433,7 @@ const sameSubnet = (a, b) => !!a && !!b && a.split(".").slice(0, 3).join(".") == // saving it lets Frame Control connect directly at home rather than over Tailscale. function lanOffer(s, d) { const ip = state && state.ip, n = s.network || {}; - if (!d || d.transient || !ip || !PRIVATE_V4.test(ip) || !sameSubnet(ip, n.local_ip)) return null; + if (!d || d.transient || !ip || !PRIVATE_V4.test(ip) || ip === n.local_ip || !sameSubnet(ip, n.local_ip)) return null; if (d.addresses.some(a => a.host === ip) || (s.probes || []).some(p => p.ip === ip && p.kind !== "tailscale")) return null; const named = dv.data && dv.data.networks.find(x => x.id === n.id && x.name); return { ip, label: named ? named.name : n.ssid || "" }; @@ -4443,14 +4444,15 @@ function renderConnAddrs() { const offer = s && lanOffer(s, d); $("connOffer").hidden = !offer; if (offer) { - const key = offer.ip + offer.label; + const key = JSON.stringify([d.id, offer.ip, offer.label]); if ($("connOffer").dataset.key !== key) { $("connOffer").dataset.key = key; - $("connOffer").innerHTML = `
The Frame is at ${esc(offer.ip)} on this network. Add it for this - network? Frame Control will then connect to it directly whenever you're here.
+ $("connOffer").innerHTML = `
The Frame is at ${esc(offer.ip)} on this network. Add it, and + Frame Control tries it first here, reaching the headset directly.
`; + // First in the list: on this network it then wins over the address in use now. $("connOfferAdd").onclick = e => devAction(`Add ${offer.ip}`, { action: "address-add", id: d.id, host: offer.ip, - kind: "lan", label: offer.label }, e.currentTarget) + kind: "lan", label: offer.label, first: true }, e.currentTarget) .then(res => res && testQuietly(d.id)); } } else $("connOffer").dataset.key = ""; @@ -4465,13 +4467,24 @@ function renderConnAddrs() { const tested = new Map((((s.tests || {})[d.id] || {}).rows || []).map(r => [r.host, r])); const inUse = s.phase === "connected" && s.via ? s.via.host : null; const sig = JSON.stringify([d.id, d.addresses, [...probes.values(), ...tested.values()].map(p => [p.host, p.state, p.ssh, p.detail]), cd.editing, inUse]); - // Leave the rows alone when nothing has changed, and while an address is being edited. - if (sig === cd.sig || cd.editing && list.contains(document.activeElement) && document.activeElement.matches("input")) return; + // Leave the rows alone when nothing has changed, and while an address is being edited: what's + // typed stays put until it's saved or cancelled, wherever the focus is. + if (sig === cd.sig || cd.editing && $("ceHost") && cd.editDev === d.id && d.addresses.some(a => a.host === cd.editing)) return; cd.sig = sig; - const refocus = cd.refocus; // after an edit, back to that row's Edit button + // Rebuilding the rows would drop the keyboard focus: put it back on the same button of the + // same address (after an edit, on that row's Edit button). + const held = list.contains(document.activeElement) && document.activeElement.closest("[data-host]"); + 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; + // 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) return `
+ if (cd.editing === a.host && cd.editDev === d.id) return `
`; @@ -4483,33 +4496,51 @@ function renderConnAddrs() { return `
${esc(a.host)} ${esc(KIND_TAG[a.kind] || a.kind)}${a.label ? ` ${esc(a.label)}` : ""}
${result}
+ ${a.host === switchTo ? `` : ""}
`; }).join("") || `
No addresses yet. Add the Frame's IP address below.
`; - if (refocus) list.querySelector(`[data-host="${CSS.escape(refocus)}"] [data-ed]`)?.focus(); + if (refocus) { + const row = list.querySelector(`[data-host="${CSS.escape(refocus[0])}"]`); + const again = row && refocus[1] && row.querySelector(refocus[1]); + if (row) (again && !again.disabled ? again : row.querySelector("[data-ed]")).focus(); + } list.querySelectorAll("[data-host]").forEach(row => { 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; 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 => { if (host === inUse && !confirm(`${host} is how Frame Control is reaching the headset now. Remove it?`)) return; devAction(`Remove ${host}`, { action: "address-remove", id: d.id, host }, e.currentTarget); }; }); if ($("ceSave")) { - const host = cd.editing, a = d.addresses.find(x => x.host === host); - const save = e => { + const host = cd.editing, a = d.addresses.find(x => x.host === host), session = cd.session; + let saving = false; + const save = async e => { e.preventDefault(); + if (saving) return; // a second Enter while the first is on its way const newHost = $("ceHost").value.trim(), label = $("ceLabel").value; - cd.editing = null; cd.refocus = newHost || host; - devAction(`Save ${host}`, { action: "address-update", id: d.id, host, newHost, kind: a.kind, label }, $("ceSave")) - .then(res => res && testQuietly(d.id)); + if (!newHost) return $("ceHost").focus(); + 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) 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(); }; $("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 => { @@ -4528,7 +4559,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; @@ -4563,6 +4593,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");