diff --git a/gently/ui/web/static/css/operate.css b/gently/ui/web/static/css/operate.css index 80c5890f..b79975c2 100644 --- a/gently/ui/web/static/css/operate.css +++ b/gently/ui/web/static/css/operate.css @@ -1074,3 +1074,22 @@ max-width: 52ch; line-height: 1.45; } + +/* The primary member of the selection is the one the instrument panes act on; + membership of the set is a lighter state. With one embryo selected the two + coincide, so the common case looks exactly as it did. */ +.rp-row.is-primary { + border-color: var(--accent); +} + +.rp-row.is-sel:not(.is-primary) { + border-color: color-mix(in srgb, var(--accent) 45%, transparent); + background: color-mix(in srgb, var(--accent) 6%, transparent); +} + +/* Disabled until something is selected, so it cannot be switched to an empty + target set. */ +#op-target-scope .op-segbtn:disabled { + opacity: 0.45; + cursor: default; +} diff --git a/gently/ui/web/static/js/operate.js b/gently/ui/web/static/js/operate.js index 093d6606..c8634002 100644 --- a/gently/ui/web/static/js/operate.js +++ b/gently/ui/web/static/js/operate.js @@ -94,6 +94,12 @@ const OperateManager = (function () { // ── emitters / run ────────────────────────────────────────────────────── let _acquiring = false; + // The selection set. `_selected` is the primary member of it. + let _targets = []; + // Which embryos a run targets. 'all' is the default and is what this pane + // has always done — a plain click must never silently narrow a timelapse + // from every subject to one, so the narrowing is a thing you say. + let _targetScope = 'all'; // Read back from the Light panel's device read, never remembered here. const ledIsOpen = () => (SharedState.get('light') || {}).led === 'Open'; let _galvo = 0.0, _piezo = 50.0; @@ -1004,14 +1010,52 @@ const OperateManager = (function () { } // ══ ACQUISITION PANE ════════════════════════════════════════════════════ - function selectEmbryo(id) { - _selected = id; + /** + * Select an embryo, and maintain the selection SET around it. + * + * `_selected` is the primary — the one the instrument panes act on, which + * is what it has always meant. `_targets` is every member. A plain click + * makes them the same, so single-embryo work is unchanged; the modifiers + * are the only way to grow the set, and growing it changes nothing until + * Acquisition is switched to "Selected". + * + * Selection is NOT role. Excluding an embryo from tonight's run by marking + * it a reference also changes expression_monitoring's scope and its + * photodose budget multiplier in the orchestrator's `_is_eligible` — those + * are facts about what the embryo is for, not about what is running. + */ + function selectEmbryo(id, mode) { + if (!id) return; + const order = _embryos.map(e => e.id); + + if (mode === 'toggle') { + const at = _targets.indexOf(id); + if (at >= 0 && _targets.length > 1) { + _targets.splice(at, 1); + // Deselecting the primary hands it to a remaining member rather + // than leaving the instrument panes pointed at nothing. + if (_selected === id) _selected = _targets[0]; + } else if (at < 0) { + _targets.push(id); + _selected = id; + } + } else if (mode === 'range' && _selected && order.includes(_selected)) { + const a = order.indexOf(_selected), b = order.indexOf(id); + _targets = order.slice(Math.min(a, b), Math.max(a, b) + 1); + _selected = id; + } else { + _targets = [id]; + _selected = id; + } + // Publish AFTER the local assignment: the subscription in wire() sees // _selected already equal and no-ops, so our own click renders exactly // once, as before. The set() is what makes the other copies of this // cursor follow. - SharedState.set('selectedEmbryoId', id); + SharedState.set('selectedEmbryoIds', _targets.slice()); + SharedState.set('selectedEmbryoId', _selected); publishRoster(); renderSpimTarget(); renderCalTarget(); renderSingle(); + renderTargetScope(); } // Shared embryo list, left of every instrument surface. Reads the canonical @@ -1142,10 +1186,46 @@ const OperateManager = (function () { * * Empty now, and the caller says so. */ - function subjectIds() { + function allSubjectIds() { return _embryos.filter(e => e.role !== 'calibration').map(e => e.id); } + /** The embryos a run will image, per the declared scope. */ + function subjectIds() { + if (_targetScope !== 'selected') return allSubjectIds(); + // Never a reference, whatever is selected: role still decides what an + // embryo is for. + const refs = new Set(_embryos.filter(e => e.role === 'calibration').map(e => e.id)); + return _targets.filter(id => !refs.has(id)); + } + + // The pane says which set it will use, and how big it is. The modifier keys + // that grow the set do not have to be discovered for this to be readable. + function renderTargetScope() { + const el = $('op-target-scope'); + if (!el) return; + const all = allSubjectIds().length; + const sel = subjectIds().length; + el.querySelectorAll('[data-scope]').forEach(b => { + const on = b.dataset.scope === _targetScope; + b.classList.toggle('is-on', on); + b.setAttribute('aria-pressed', String(on)); + }); + const a = el.querySelector('[data-scope="all"]'); + const s2 = el.querySelector('[data-scope="selected"]'); + if (a) a.textContent = `All subjects (${all})`; + if (s2) { + s2.textContent = `Selected (${_targetScope === 'selected' ? sel : _targets.length})`; + s2.disabled = !_targets.length; + } + } + + function setTargetScope(scope) { + _targetScope = scope === 'selected' ? 'selected' : 'all'; + renderTargetScope(); + renderSingle(); + } + // Every embryo marked as a reference means there is nothing to image. Say // so once, here, rather than at each mode — and say which state it is in, // because "no embryos" and "no subjects among your embryos" need @@ -1315,7 +1395,7 @@ const OperateManager = (function () { acquire: { onEnter() { renderRun(); }, onLeave() {}, - render() { publishRoster(); renderSingle(); }, + render() { publishRoster(); renderSingle(); renderTargetScope(); }, }, }; function stopBottom() { @@ -1430,11 +1510,17 @@ const OperateManager = (function () { function onEmbryosUpdate(p) { _embryos = (p && Array.isArray(p.embryos)) ? p.embryos.slice() : []; if (_selected && !_embryos.some(e => e.id === _selected)) _selected = null; + // Drop members that no longer exist, or the set silently targets ghosts + // — the same class of bug as the phantom roster row in #126. + const known = new Set(_embryos.map(e => e.id)); + _targets = _targets.filter(id => known.has(id)); // The embryo list is shared across all three panes; keep a live // selection whenever it is non-empty so SPIM/Acquire aren't a dead-end // ("No embryo selected") right after registering. The operator can still // switch by clicking a registered embryo (bottom) or a roster row. if (!_selected && _embryos.length) _selected = _embryos[0].id; + if (!_targets.length && _selected) _targets = [_selected]; + SharedState.set('selectedEmbryoIds', _targets.slice()); SharedState.set('selectedEmbryoId', _selected); // Render the shared rail even when the Operate view isn't the active tab, // so switching to it (or refreshing) shows the list immediately rather @@ -1477,9 +1563,10 @@ const OperateManager = (function () { // of them looking for buttons the panel no longer emits, is how a // select fires twice. // - // Everything below was collateral of that deletion: the slice that - // removed the roster's own listeners ran past them, and eight controls - // went dead — including the interlock banner's back-off button. + // Everything below was collateral of that deletion in the roster + // refactor: the slice that removed the roster's own listeners ran past + // them. Eight controls went dead, including the interlock banner's + // back-off button. Restored, and pinned by a test that counts them. const sp = $('op-spim-toggle'); if (sp) sp.addEventListener('click', toggleSpim); const cal = $('op-calibrate'); if (cal) cal.addEventListener('click', calibrateSelected); document.querySelectorAll('[data-gv]').forEach(b => @@ -1510,6 +1597,11 @@ const OperateManager = (function () { if (b) { _selectedLib = b.dataset.lib; loadLibrary(); } }); } + const scope = $('op-target-scope'); + if (scope) scope.addEventListener('click', e => { + const b = e.target.closest('[data-scope]'); + if (b && !b.disabled) setTargetScope(b.dataset.scope); + }); const start = $('op-run-start'); if (start) start.addEventListener('click', startRun); const pause = $('op-run-pause'); if (pause) pause.addEventListener('click', pauseRun); const stopb = $('op-run-stop'); if (stopb) stopb.addEventListener('click', stopRun); @@ -1617,7 +1709,7 @@ const OperateManager = (function () { // panels/roster.js renders SharedState.embryos and calls these. The // list, the selection and the endpoints stay here. roster: { - select: id => selectEmbryo(id), + select: (id, mode) => selectEmbryo(id, mode), remove: id => deleteEmbryo(id), centre: id => { const emb = _embryos.find(e => e.id === id); diff --git a/gently/ui/web/static/js/panels/roster.js b/gently/ui/web/static/js/panels/roster.js index 9e140b8a..d4310c72 100644 --- a/gently/ui/web/static/js/panels/roster.js +++ b/gently/ui/web/static/js/panels/roster.js @@ -105,12 +105,13 @@ const RosterPanel = (() => { function render() { const embryos = SharedState.get('embryos') || []; const selected = SharedState.get('selectedEmbryoId'); + const inSet = new Set(SharedState.get('selectedEmbryoIds') || []); mounts.forEach((opts, hostId) => { const host = document.getElementById(hostId); if (!host) return; host.innerHTML = embryos.length - ? embryos.map(e => row(e, selected, opts)).join('') + ? embryos.map(e => row(e, selected, inSet, opts)).join('') : empty(opts); wire(host); }); @@ -127,7 +128,7 @@ const RosterPanel = (() => { then register.${cta}`; } - function row(emb, selected, opts) { + function row(emb, selected, inSet, opts) { const xy = xyOf(emb); const role = (emb.role && emb.role !== 'unassigned') ? emb.role : 'test'; const isRef = role === 'calibration'; @@ -146,7 +147,13 @@ const RosterPanel = (() => { data-verb="${a.verb}" data-id="${esc(emb.id)}">${a.text}`; }).join(''); - return `
Embryo ${esc(labelOf(emb))}${fitBadge(emb, opts)} @@ -179,7 +186,14 @@ const RosterPanel = (() => { const goto = e.target.closest('[data-goto]'); if (goto) { if (v && v.goTo) v.goTo(goto.dataset.goto); return; } const row = e.target.closest('[data-embryo]'); - if (row && v && v.select) v.select(row.dataset.embryo); + if (!row || !v || !v.select) return; + // The interaction every file manager, DAW and photo library already + // teaches, so the set needs no chrome of its own. A plain click + // replaces, which keeps single-embryo work identical to before. + const mode = (e.metaKey || e.ctrlKey) ? 'toggle' + : e.shiftKey ? 'range' + : 'replace'; + v.select(row.dataset.embryo, mode); }; host.onkeydown = e => { if (e.key !== 'Enter' && e.key !== ' ') return; diff --git a/gently/ui/web/static/js/status-store.js b/gently/ui/web/static/js/status-store.js index 72f4adaa..06e481d6 100644 --- a/gently/ui/web/static/js/status-store.js +++ b/gently/ui/web/static/js/status-store.js @@ -88,6 +88,11 @@ const SharedState = (() => { const s = { stageXY: null, // {x, y} in um, or null if unknown selectedEmbryoId: null, + // The selection SET. `selectedEmbryoId` stays the primary — the one the + // instrument panes act on — and this is every member. One embryo + // selected means the two agree, which is the normal case and behaves + // exactly as it always did. + selectedEmbryoIds: [], sessionId: null, agentBusy: false, hasControl: false, diff --git a/gently/ui/web/templates/index.html b/gently/ui/web/templates/index.html index cb102363..3f9e830f 100644 --- a/gently/ui/web/templates/index.html +++ b/gently/ui/web/templates/index.html @@ -759,6 +759,17 @@

Device
What to run
+ +
+ + +
diff --git a/tests/test_operate_controls_are_wired.py b/tests/test_operate_controls_are_wired.py index 6103ea50..4c2c8301 100644 --- a/tests/test_operate_controls_are_wired.py +++ b/tests/test_operate_controls_are_wired.py @@ -50,6 +50,7 @@ "op-confirm": "$('op-confirm')", "op-clear": "$('op-clear')", "op-cam-toggle": "$('op-cam-toggle')", + "op-target-scope": "$('op-target-scope')", "[data-gv]": "[data-gv]", "[data-pz]": "[data-pz]", "[data-backoff]": "[data-backoff]", diff --git a/tests/test_target_selection.py b/tests/test_target_selection.py new file mode 100644 index 00000000..5d815c14 --- /dev/null +++ b/tests/test_target_selection.py @@ -0,0 +1,110 @@ +"""A run's targets are said, not inferred. + +The orchestrator and the route have always accepted an explicit `embryo_ids` +list — `orchestrator.start(embryo_ids=[...])` images exactly those. The UI never +offered it: `adaptive` and `library` sent every non-reference embryo, and there +was no way to say "just this one". + +The only workaround was to mark the others as references, and that is not a +selection mechanism. `role` decides what an embryo is *for*: +`expression_monitoring` scopes to `role == 'test'`, and the orchestrator's +`_is_eligible` reads `role.photodose_budget_multiplier` for the dose budget. So +excluding an embryo from tonight's run by demoting it also changed its +monitoring and its exposure allowance. + +Selection is now a set, and the run's scope is explicit. + +THE SAFETY PROPERTY, and the reason for the two-state control: targets must +never follow the selection implicitly. A plain click in the roster would +otherwise narrow a timelapse from every subject to one, silently. `all` is the +default and is what this pane has always done; narrowing is a thing you say. +""" + +from __future__ import annotations + +import re +from pathlib import Path + +JS = Path(__file__).resolve().parents[1] / "gently" / "ui" / "web" / "static" / "js" +INDEX = Path(__file__).resolve().parents[1] / "gently" / "ui" / "web" / "templates" / "index.html" + + +def _operate() -> str: + return (JS / "operate.js").read_text(encoding="utf-8") + + +def test_the_default_scope_is_every_subject() -> None: + """Anything else is a silent narrowing of an existing workflow.""" + src = _operate() + assert re.search(r"let _targetScope = 'all'", src), ( + "the default run scope is no longer 'all' — a plain click in the roster " + "would narrow a timelapse without saying so" + ) + + +def test_scope_selects_between_two_explicit_sets() -> None: + src = _operate() + body = src[src.index("function subjectIds()") :] + body = body[: body.index("\n }")] + assert "_targetScope !== 'selected'" in body + assert "allSubjectIds()" in body + + +def test_a_reference_is_never_a_target_however_it_is_selected() -> None: + """Role still decides what an embryo is for, whatever is highlighted.""" + src = _operate() + body = src[src.index("function subjectIds()") :] + body = body[: body.index("\n }")] + assert "'calibration'" in body, ( + "subjectIds no longer excludes references — selecting one would image it as a subject" + ) + + +def test_the_primary_and_the_set_are_both_published() -> None: + """Instrument panes act on the primary; runs act on the set.""" + src = _operate() + assert "SharedState.set('selectedEmbryoIds'" in src + assert "SharedState.set('selectedEmbryoId'" in src + store = (JS / "status-store.js").read_text(encoding="utf-8") + assert "selectedEmbryoIds" in store + + +def test_all_three_click_modes_exist() -> None: + panel = (JS / "panels" / "roster.js").read_text(encoding="utf-8") + assert "metaKey" in panel and "ctrlKey" in panel, "no toggle modifier" + assert "shiftKey" in panel, "no range modifier" + assert "'replace'" in panel, "a plain click must still replace the set" + + src = _operate() + sel = src[src.index("function selectEmbryo(id, mode)") :] + sel = sel[: sel.index("\n }")] + for mode in ("toggle", "range"): + assert f"'{mode}'" in sel, f"selectEmbryo does not handle {mode}" + + +def test_the_set_drops_embryos_that_no_longer_exist() -> None: + """A roster refresh must not leave the set targeting ghosts (#126's class).""" + src = _operate() + upd = src[src.index("function onEmbryosUpdate(p)") :] + upd = upd[: upd.index("\n }")] + assert "_targets = _targets.filter" in upd + + +def test_the_pane_states_which_set_it_will_use() -> None: + """The modifier keys need not be discovered for the scope to be readable.""" + html = INDEX.read_text(encoding="utf-8") + assert 'id="op-target-scope"' in html + assert 'data-scope="all"' in html and 'data-scope="selected"' in html + src = _operate() + body = src[src.index("function renderTargetScope()") :] + body = body[: body.index("\n }")] + # Counts, so "Selected" is never an unknown quantity. + assert "All subjects (${all})" in body + assert "Selected (${" in body + + +def test_selected_cannot_be_chosen_with_nothing_selected() -> None: + src = _operate() + body = src[src.index("function renderTargetScope()") :] + body = body[: body.index("\n }")] + assert "disabled = !_targets.length" in body