From feb0c4e0152ec23cac40ca0b19fdf3a6110a0ba9 Mon Sep 17 00:00:00 2001 From: Michael Johnson Date: Sat, 22 Aug 2026 21:54:36 +0100 Subject: [PATCH 1/2] Dashboard: a way out of the review pane MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review is a mode, not a fourth column: opening it stands every other pane down at every width. Below 760 px the tab bar takes you back, but above it the tab bar is hidden and a tab click was the only thing that ever called `show()` with another pane name — so an operator who clicked `N candidates — review` on a desk could leave only by resizing the window or reloading the page. `back to map` in the review pane head, and Escape, call `panes.show('map')`. Both are held off while a zone edit is live, as the floor picker and the revision list already are: an edit has no autosave, and leaving would strand it on a canvas nobody can see. A promotion returns to the map itself, the decision the pane exists for having been made — unless the announcement failed, where the note saying so is readable only in the pane that wrote it. ui_test.mjs holds the exit in place: the pane has the control, its handler names a pane that is not this one, Escape shares that handler, and an edit disables it. browser_check.mjs drives it at 1600 px, where the tab bar computes to `display: none`: the button returns to the map pane and the operations panes are displayed again, re-entering works, Escape does the same, and neither works mid-edit. 39/39 checks pass; the phone pass is unchanged. --- docs/fleet/README.md | 8 ++++ mote_fleet/server/ui/app.mjs | 33 ++++++++++++++-- mote_fleet/server/ui/index.html | 6 +++ mote_fleet/server/ui/review.mjs | 11 +++++- mote_fleet/server/ui/style.css | 5 ++- mote_fleet/test/browser_check.mjs | 63 +++++++++++++++++++++++++++++++ mote_fleet/test/ui_test.mjs | 36 ++++++++++++++++++ 7 files changed, 156 insertions(+), 6 deletions(-) diff --git a/docs/fleet/README.md b/docs/fleet/README.md index 8d48324..a93aa50 100644 --- a/docs/fleet/README.md +++ b/docs/fleet/README.md @@ -908,6 +908,14 @@ which appears whenever the floor on screen has something waiting. It shows: Everything except that button and the zone editor below is a read. Nothing you do here changes any floor until you promote. +**Leaving it** is `back to map` in the pane head, or Escape. Review is a mode +rather than a fourth column — it stands the operations panes down at every +width — and the tab bar exists only below 760 px, so on a desk the pane's own +head is the way out. Both are held off while a zone edit is up, as the revision +list is: an edit has no autosave, so `cancel` (or `save as candidate`) ends it +first. A promotion leaves for you, the decision it was opened for having been +made. + ### Naming the rooms on it, before you promote it `pixi run segment-map` finds the rooms of a map but cannot know what they are diff --git a/mote_fleet/server/ui/app.mjs b/mote_fleet/server/ui/app.mjs index 4a5ef3e..5a37e81 100644 --- a/mote_fleet/server/ui/app.mjs +++ b/mote_fleet/server/ui/app.mjs @@ -288,11 +288,34 @@ function onReviewJump() { if (state.mapKey) review.open(state.mapKey); } -// A promotion happened in the review pane: this pane's basemap is now a -// different map, so re-resolve it rather than keep drawing the old one. -function onPromoted() { +// Out of it again. Review stands every other pane down at every width, and the +// tab bar that would bring one back exists only below 760 px, so this is the +// desk's only exit. An edit in progress owns the pane — the same rule that +// disables the floor picker and the revision list — because leaving would put +// an unsaved edit on a canvas nobody can see. +function onReviewBack() { + if (!review.leavable()) return; + panes.show('map'); +} + +// Escape is the same exit. It is on the document rather than on the pane +// because the pane holds no focus of its own: the last thing clicked was a +// revision row, the canvas, or nothing at all. +function onKey(event) { + if (event.key !== 'Escape') return; + if (panes.current() !== 'review') return; + onReviewBack(); +} + +// A promotion happened in the review pane: the floor's canonical revision has +// changed, so re-resolve this pane's basemap rather than keep drawing the old +// one — and the decision the review pane exists for has been made, so hand the +// screen back to operations. Not when the announcement failed: the note saying +// so is readable only in the pane that wrote it. +function onPromoted(site, floor, revision, announced) { state.mapKey = null; scheduleRender(); + if (announced) panes.show('map'); } // -- rendering ----------------------------------------------------------- @@ -576,6 +599,7 @@ function bind() { reviewCanvas: 'review-canvas', reviewMapLabel: 'review-map-label', reviewPromote: 'review-promote', + reviewBack: 'review-back', reviewFit: 'review-fit', reviewNote: 'review-note', zonesEdit: 'zones-edit', @@ -620,6 +644,7 @@ export async function boot() { zoneSource: dom.reviewZoneSource, mapLabel: dom.reviewMapLabel, promote: dom.reviewPromote, + back: dom.reviewBack, fit: dom.reviewFit, note: dom.reviewNote, // The zone editor's own controls. It lives in this pane because it edits @@ -645,6 +670,8 @@ export async function boot() { }); dom.zone.addEventListener('change', onZone); dom.reviewJump.addEventListener('click', onReviewJump); + dom.reviewBack.addEventListener('click', onReviewBack); + document.addEventListener('keydown', onKey); dom.fit.addEventListener('click', () => { mapView.follow(null); dom.follow.checked = false; diff --git a/mote_fleet/server/ui/index.html b/mote_fleet/server/ui/index.html index 2749f03..49e87c1 100644 --- a/mote_fleet/server/ui/index.html +++ b/mote_fleet/server/ui/index.html @@ -63,6 +63,12 @@

map

is the map about to be promoted and not the one already published. -->
+ +

review

— diff --git a/mote_fleet/server/ui/review.mjs b/mote_fleet/server/ui/review.mjs index 525068d..01f5bb5 100644 --- a/mote_fleet/server/ui/review.mjs +++ b/mote_fleet/server/ui/review.mjs @@ -372,6 +372,14 @@ export class ReviewView { return Boolean(this.selected && this.map.map); } + // Whether the pane may be left. An edit is a mode on the selected revision + // with no autosave, so it holds the exit for the same reason it holds the + // floor picker and the revision list: leaving would strand it on a canvas + // nobody can see. `cancel` is how an edit ends. + leavable() { + return !this.editing; + } + // Editing is a mode on the selected revision, so while it is on, the things // that would swap that revision out from under it are disabled rather than // racing it. There is no autosave: an unsaved edit is lost to `cancel`, and @@ -384,6 +392,7 @@ export class ReviewView { this.dom.zoneSave.hidden = !this.editing; this.dom.zoneCancel.hidden = !this.editing; this.dom.floor.disabled = this.editing; + this.dom.back.disabled = this.editing; for (const row of this.dom.revisions.querySelectorAll('button')) { row.disabled = this.editing; } @@ -577,7 +586,7 @@ export class ReviewView { : `promoted, but not announced: ${body.detail}`, !body.announced, ); - this.onPromoted(site, floor, revision); + this.onPromoted(site, floor, revision, Boolean(body.announced)); } catch (error) { this.note(error.message, true); } diff --git a/mote_fleet/server/ui/style.css b/mote_fleet/server/ui/style.css index 9f39023..b60f997 100644 --- a/mote_fleet/server/ui/style.css +++ b/mote_fleet/server/ui/style.css @@ -792,9 +792,10 @@ main { } /* Editing holds the selected revision still, so the controls that would swap it - are disabled — and have to *look* disabled, or the pane reads as ignoring - clicks. */ + — or walk away from it, an edit having no autosave — are disabled, and have + to *look* disabled, or the pane reads as ignoring clicks. */ .zones-head button:disabled, +#review-back:disabled, .revision-row:disabled { opacity: 0.5; cursor: not-allowed; diff --git a/mote_fleet/test/browser_check.mjs b/mote_fleet/test/browser_check.mjs index c408231..c4a7526 100644 --- a/mote_fleet/test/browser_check.mjs +++ b/mote_fleet/test/browser_check.mjs @@ -283,6 +283,44 @@ try { `${candidateDrawn} painted pixels`, ); + // The way out, which above 760 px is the pane's own control and nothing else: + // the tab bar the phone leaves by is hidden here, and review stands every + // other pane down, so a pane with no exit of its own is a trap that ends in a + // window resize or a reload (what shipped with the pane, 2026-08-11). + const exits = await session.evaluate(`(() => { + const active = () => [...document.querySelectorAll('.pane')] + .filter(pane => pane.classList.contains('active')) + .map(pane => pane.dataset.pane).join(','); + const out = { width: innerWidth, tabs: getComputedStyle(document.querySelector('.panes')).display }; + document.getElementById('review-back').click(); + out.button = active(); + // The operations panes are only *displayed* again if the review mode rule + // has let go: the class is what that rule keys on, so both are read. + out.shown = getComputedStyle(document.querySelector('.map-pane')).display; + document.getElementById('review-jump').click(); + out.reopened = active(); + document.dispatchEvent(new KeyboardEvent('keydown', { key: 'Escape' })); + out.escape = active(); + return out; + })()`); + check( + 'the review pane can be left at desk width, by its button and by Escape', + exits.tabs === 'none' && + exits.button === 'map' && + exits.shown !== 'none' && + exits.reopened === 'review' && + exits.escape === 'map', + JSON.stringify(exits), + ); + + // Back in, for the rest of the review checks. + await session.evaluate(`document.getElementById('review-jump').click()`); + await settle( + session, + `document.getElementById('review-map-label').textContent`, + (label) => /\d{8}T\d{6}/.test(label), + ); + // The fixture's candidate is the published map mirrored, so a review pane // that fetched the canonical image — the defect this replaces — would draw a // perfectly convincing map. The URL is what separates the two. @@ -374,6 +412,24 @@ try { JSON.stringify({ ends: editing.ends, adds: editing.adds }), ); + // The way out is held with them. An edit has no autosave, so leaving would + // strand it on a canvas nobody can see; `cancel` is how an edit ends. + const held = await session.evaluate(`(() => { + document.dispatchEvent(new KeyboardEvent('keydown', { key: 'Escape' })); + document.getElementById('review-back').click(); + return { + disabled: document.getElementById('review-back').disabled, + pane: [...document.querySelectorAll('.pane')] + .filter(pane => pane.classList.contains('active')) + .map(pane => pane.dataset.pane).join(','), + }; + })()`); + check( + 'an edit in progress holds the exit, by button and by Escape', + held.disabled && held.pane === 'review', + JSON.stringify(held), + ); + // One list, one shape: the rows are the same rows, in the same place, the // same height, with the same cells — `edit zones` puts controls in them and // changes nothing else. Every part of that has been wrong at least once: a @@ -605,6 +661,13 @@ try { (note) => /is on \d{8}T\d{6}/.test(note), ); check('the first promotion on a floor goes through', /is on /.test(promoted), promoted); + + // And the pane it was made in stands down: the decision it exists for has + // been made, and the operations map is what an operator wants next. + const landed = await session.evaluate(`[...document.querySelectorAll('.pane')] + .filter(pane => pane.classList.contains('active')) + .map(pane => pane.dataset.pane).join(',')`); + check('a promotion hands the screen back to the operations map', landed === 'map', landed); } // -- the phone ---------------------------------------------------------- diff --git a/mote_fleet/test/ui_test.mjs b/mote_fleet/test/ui_test.mjs index f4d00e7..39f0a1e 100644 --- a/mote_fleet/test/ui_test.mjs +++ b/mote_fleet/test/ui_test.mjs @@ -937,6 +937,41 @@ test('review is a mode: opening it stands the operations panes down', () => { assert.match(css, /\.review-pane\.active\s*\{\s*display:\s*flex/); }); +test('the review pane has a way out, and it leads to the map', () => { + // Review stands every other pane down at every width, and the tab bar that + // would bring one back exists only below 760 px. So above it the pane's own + // control is the only exit there is: without one, an operator who opened + // review on a desk escaped by resizing the window or reloading the page. + const html = read('index.html'); + const review = html.slice( + html.indexOf('class="pane review-pane"'), + html.indexOf('class="pane detail-pane"'), + ); + assert.ok(review.includes('id="review-back"'), 'the review pane has no exit control'); + + // A control that leaves is only an exit if it names a pane that is *not* + // this one — `show('review')` on a button labelled `back` would look right + // in the markup and change nothing on screen. + const app = read('app.mjs'); + const leave = app.slice(app.indexOf('function onReviewBack(')); + assert.match(leave.slice(0, leave.indexOf('\n}')), /panes\.show\('map'\)/); + assert.match(app, /dom\.reviewBack\.addEventListener\('click', onReviewBack\)/); + // And Escape is the same exit, not a second one with its own rules. + const key = app.slice(app.indexOf('function onKey(')); + const body = key.slice(0, key.indexOf('\n}')); + assert.match(body, /event\.key !== 'Escape'/); + assert.match(body, /onReviewBack\(\)/); +}); + +test('an edit in progress holds the exit, as it holds the revision list', () => { + // There is no autosave: leaving mid-edit would strand it on a canvas nobody + // can see. `cancel` is how an edit ends. + const source = read('review.mjs'); + const controls = source.slice(source.indexOf('renderEditControls()')); + assert.match(controls.slice(0, controls.indexOf('\n }')), /this\.dom\.back\.disabled = this\.editing/); + assert.match(source, /leavable\(\) \{\s*return !this\.editing;/); +}); + test('the review pane has every element app.mjs binds to it', () => { const html = read('index.html'); for (const id of [ @@ -953,6 +988,7 @@ test('the review pane has every element app.mjs binds to it', () => { 'review-canvas', 'review-map-label', 'review-promote', + 'review-back', 'review-fit', 'review-note', ]) { From 078bbe7ea40f447e9c31bf471530892ea152c963 Mon Sep 17 00:00:00 2001 From: Michael Johnson Date: Sat, 22 Aug 2026 22:11:53 +0100 Subject: [PATCH 2/2] Trim the comments added with the review pane's exit What the code and the check names already said did not need saying beside them. What is left is what neither says: why 760 px decides the button exists, why a promotion stays put when the announcement failed, the mutation the `show('map')` assertion guards against, and why the browser check re-enters the pane mid-flow. --- mote_fleet/server/ui/app.mjs | 19 ++++++------------- mote_fleet/server/ui/index.html | 7 ++----- mote_fleet/server/ui/review.mjs | 4 ---- mote_fleet/server/ui/style.css | 4 ++-- mote_fleet/test/browser_check.mjs | 12 ++---------- mote_fleet/test/ui_test.mjs | 12 ++---------- 6 files changed, 14 insertions(+), 44 deletions(-) diff --git a/mote_fleet/server/ui/app.mjs b/mote_fleet/server/ui/app.mjs index 5a37e81..9292df4 100644 --- a/mote_fleet/server/ui/app.mjs +++ b/mote_fleet/server/ui/app.mjs @@ -288,30 +288,23 @@ function onReviewJump() { if (state.mapKey) review.open(state.mapKey); } -// Out of it again. Review stands every other pane down at every width, and the -// tab bar that would bring one back exists only below 760 px, so this is the -// desk's only exit. An edit in progress owns the pane — the same rule that -// disables the floor picker and the revision list — because leaving would put -// an unsaved edit on a canvas nobody can see. +// Out of it again, by the pane's own control or by Escape: above 760 px the tab +// bar is hidden, so these are the only exits. function onReviewBack() { if (!review.leavable()) return; panes.show('map'); } -// Escape is the same exit. It is on the document rather than on the pane -// because the pane holds no focus of its own: the last thing clicked was a -// revision row, the canvas, or nothing at all. function onKey(event) { if (event.key !== 'Escape') return; if (panes.current() !== 'review') return; onReviewBack(); } -// A promotion happened in the review pane: the floor's canonical revision has -// changed, so re-resolve this pane's basemap rather than keep drawing the old -// one — and the decision the review pane exists for has been made, so hand the -// screen back to operations. Not when the announcement failed: the note saying -// so is readable only in the pane that wrote it. +// A promotion happened in the review pane: this pane's basemap is now a +// different map, so re-resolve it rather than keep drawing the old one. The +// review is then over — except when the announcement failed, whose note is +// readable only in the pane that wrote it. function onPromoted(site, floor, revision, announced) { state.mapKey = null; scheduleRender(); diff --git a/mote_fleet/server/ui/index.html b/mote_fleet/server/ui/index.html index 49e87c1..2bf71e3 100644 --- a/mote_fleet/server/ui/index.html +++ b/mote_fleet/server/ui/index.html @@ -63,11 +63,8 @@

map

is the map about to be promoted and not the one already published. -->
- +

review

— diff --git a/mote_fleet/server/ui/review.mjs b/mote_fleet/server/ui/review.mjs index 01f5bb5..df09f01 100644 --- a/mote_fleet/server/ui/review.mjs +++ b/mote_fleet/server/ui/review.mjs @@ -372,10 +372,6 @@ export class ReviewView { return Boolean(this.selected && this.map.map); } - // Whether the pane may be left. An edit is a mode on the selected revision - // with no autosave, so it holds the exit for the same reason it holds the - // floor picker and the revision list: leaving would strand it on a canvas - // nobody can see. `cancel` is how an edit ends. leavable() { return !this.editing; } diff --git a/mote_fleet/server/ui/style.css b/mote_fleet/server/ui/style.css index b60f997..8ecd826 100644 --- a/mote_fleet/server/ui/style.css +++ b/mote_fleet/server/ui/style.css @@ -792,8 +792,8 @@ main { } /* Editing holds the selected revision still, so the controls that would swap it - — or walk away from it, an edit having no autosave — are disabled, and have - to *look* disabled, or the pane reads as ignoring clicks. */ + or leave it are disabled — and have to *look* disabled, or the pane reads as + ignoring clicks. */ .zones-head button:disabled, #review-back:disabled, .revision-row:disabled { diff --git a/mote_fleet/test/browser_check.mjs b/mote_fleet/test/browser_check.mjs index c4a7526..5be50b3 100644 --- a/mote_fleet/test/browser_check.mjs +++ b/mote_fleet/test/browser_check.mjs @@ -283,10 +283,8 @@ try { `${candidateDrawn} painted pixels`, ); - // The way out, which above 760 px is the pane's own control and nothing else: - // the tab bar the phone leaves by is hidden here, and review stands every - // other pane down, so a pane with no exit of its own is a trap that ends in a - // window resize or a reload (what shipped with the pane, 2026-08-11). + // The pane shipped with no exit above 760 px, where the tab bar is hidden: an + // operator who opened it left by resizing the window or reloading the page. const exits = await session.evaluate(`(() => { const active = () => [...document.querySelectorAll('.pane')] .filter(pane => pane.classList.contains('active')) @@ -294,8 +292,6 @@ try { const out = { width: innerWidth, tabs: getComputedStyle(document.querySelector('.panes')).display }; document.getElementById('review-back').click(); out.button = active(); - // The operations panes are only *displayed* again if the review mode rule - // has let go: the class is what that rule keys on, so both are read. out.shown = getComputedStyle(document.querySelector('.map-pane')).display; document.getElementById('review-jump').click(); out.reopened = active(); @@ -412,8 +408,6 @@ try { JSON.stringify({ ends: editing.ends, adds: editing.adds }), ); - // The way out is held with them. An edit has no autosave, so leaving would - // strand it on a canvas nobody can see; `cancel` is how an edit ends. const held = await session.evaluate(`(() => { document.dispatchEvent(new KeyboardEvent('keydown', { key: 'Escape' })); document.getElementById('review-back').click(); @@ -662,8 +656,6 @@ try { ); check('the first promotion on a floor goes through', /is on /.test(promoted), promoted); - // And the pane it was made in stands down: the decision it exists for has - // been made, and the operations map is what an operator wants next. const landed = await session.evaluate(`[...document.querySelectorAll('.pane')] .filter(pane => pane.classList.contains('active')) .map(pane => pane.dataset.pane).join(',')`); diff --git a/mote_fleet/test/ui_test.mjs b/mote_fleet/test/ui_test.mjs index 39f0a1e..3e97a66 100644 --- a/mote_fleet/test/ui_test.mjs +++ b/mote_fleet/test/ui_test.mjs @@ -938,10 +938,6 @@ test('review is a mode: opening it stands the operations panes down', () => { }); test('the review pane has a way out, and it leads to the map', () => { - // Review stands every other pane down at every width, and the tab bar that - // would bring one back exists only below 760 px. So above it the pane's own - // control is the only exit there is: without one, an operator who opened - // review on a desk escaped by resizing the window or reloading the page. const html = read('index.html'); const review = html.slice( html.indexOf('class="pane review-pane"'), @@ -949,14 +945,12 @@ test('the review pane has a way out, and it leads to the map', () => { ); assert.ok(review.includes('id="review-back"'), 'the review pane has no exit control'); - // A control that leaves is only an exit if it names a pane that is *not* - // this one — `show('review')` on a button labelled `back` would look right - // in the markup and change nothing on screen. + // An exit only if it names a pane that is *not* this one: `show('review')` + // on a button labelled `back` looks right in the markup and does nothing. const app = read('app.mjs'); const leave = app.slice(app.indexOf('function onReviewBack(')); assert.match(leave.slice(0, leave.indexOf('\n}')), /panes\.show\('map'\)/); assert.match(app, /dom\.reviewBack\.addEventListener\('click', onReviewBack\)/); - // And Escape is the same exit, not a second one with its own rules. const key = app.slice(app.indexOf('function onKey(')); const body = key.slice(0, key.indexOf('\n}')); assert.match(body, /event\.key !== 'Escape'/); @@ -964,8 +958,6 @@ test('the review pane has a way out, and it leads to the map', () => { }); test('an edit in progress holds the exit, as it holds the revision list', () => { - // There is no autosave: leaving mid-edit would strand it on a canvas nobody - // can see. `cancel` is how an edit ends. const source = read('review.mjs'); const controls = source.slice(source.indexOf('renderEditControls()')); assert.match(controls.slice(0, controls.indexOf('\n }')), /this\.dom\.back\.disabled = this\.editing/);