Skip to content

Make covering workspaces modal - #536

Merged
max-sixty merged 11 commits into
mainfrom
codex/workspace-continuity
Sep 10, 2026
Merged

Make covering workspaces modal#536
max-sixty merged 11 commits into
mainfrom
codex/workspace-continuity

Conversation

@max-sixty

@max-sixty max-sixty commented Sep 10, 2026

Copy link
Copy Markdown
Owner

Covering workspaces currently obscure the document without disabling its controls, which lets focus and scrolling escape behind the visible surface. This change gives Threads, Asks, and Leaves one composed modality owner: Threads and Asks become modal only when responsive layout makes them cover the document, while Leaves is always modal because it navigates away from the page.

The shared owner applies native inertness, a full-background light-dismiss scrim, dialog semantics, focus containment and recovery, and active reading-scroll routing. It re-derives the inert boundary when a live version replaces the document. Workspace visibility remains separate from posture, so resizing between covering and beside modes preserves the focused subject, drafts, scroll position, and keyboard return route; a background popover is dismissed if that responsive change establishes a modal boundary beneath it. Global workspace addresses remain available inside the modal boundary, including native page-map and version layers opened above it. Asks and Leaves also gain pointer-accessible close controls and open on their first content destination.

The browser coverage exercises pointer and keyboard entry, same-address close and peer switching, responsive round trips, return-frame retirement, document replacement, scrim dismissal, document scroll isolation, focus recovery after DOM replacement, and the open chrome's accessibility tree.

This was written by Codex on behalf of max-sixty

@leaf-agent leaf-agent left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Three things to settle. Two of them show only in the nightly suite, and the PR gate skips test_render_*, so nothing on this branch's checks reports them.

A version arriving drops the boundary. enter() snapshots the background once and sets inert on that list, but the covered document is not a stable node: activateRevision in runtime/version.js runs document.querySelector("body > main").replaceWith(fresh), and prepareActivation is driven by the state projection, so it happens with no reader gesture. Measured on this branch at 500x640 with Threads covering — publish v2, push state — main.inert goes from true to false while .lf-panel keeps aria-modal="true" and coveringSurface() still returns the panel. The keyboard stays confined to the workspace while pointer and Tab reach the page again, and assistive tech is told a boundary that is no longer there. Either tray behaves the same way. CLAUDE.md's "The document starts state; the log changes it" makes a version arriving an ordinary transition, so the inert reading has to be re-derived when the document under it is replaced rather than captured once at entry.

The new scope puts navigation.back into the reference on every page. test_the_key_line_says_what_a_press_will_do passes at the merge base (203473a) and fails here. reference.js filters a scope's rows by live(row) only when readerIn(scope) — a scope the reader is merely near is listed whole — so COVERING_WORKSPACE's copy of BACK_OUT is presented whatever its when says, and presentedCommands then claims the id and takes Escape's row away from the page. On a 1200px window with Threads open beside the page, ? lists "Close the thread panel" under "In the covering workspace" alongside "Return from Threads panel": two rows for one press, in a scope that is not there. The inline suggestion below turns that test, and the PR's own 14 covering-workspace cases still pass with it applied.

test_selection_hints_do_not_name_page_content_behind_a_covering_panel still states the pre-inert contract. #139 named it as the open product call — "whether a covering panel should take the page out of reach entirely, or cover it and leave it working" — and this PR makes that call. test_render_aim.py was rewritten for exactly that consequence, and two of the other tests #139 listed picked up their Escape guards here, but the selection one did not: s at 700px with Threads open now produces zero .lf-target-hint elements. It is red at the merge base too, on its geometry assertion, so it does not turn ci from green — but what is now false is its premise rather than its numbers, and it is the test that records what "covering" means.

One more to confirm rather than fix: covers: () => key === "leaves" || trayCovers() makes Leaves modal at every width, while .lf-tray-panel is width: var(--lf-tray-w) with no backdrop. At 1400x900 the tray takes 300px, main is inert, html is overflow-y: hidden, and a wheel over the visible document does nothing — roughly 1100px of fully legible page is out of reach with nothing on screen to say so. If page-wide modality is what Leaves wants, it needs a scrim; if it is not, its covers() reading should ask the viewport the way the other two do.

Comment thread skills/leaf/assets/runtime/keyboard/page.js
Comment thread skills/leaf/assets/runtime/workspace-modality.js Outdated

@leaf-agent leaf-agent left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two findings, both measured in a browser on this head. The three from the last round are settled: a replacement document is re-inerted, COVERING_WORKSPACE no longer reaches the reference from a scope the reader is only near, and the selection-hint test now states the inert contract rather than the geometric one.

The banner is inert, but neither dimmed nor covered. background() in workspace-modality.js suspends every chromeRoot child except the surface and the scrim, so header.lf-banner goes inert along with the covered document — while the scrim that is supposed to say where the boundary runs stops below it: .lf-workspace-scrim is inset: var(--lf-banner-h) 0 0 at z-index: 8899, against the banner rule's z-index: 9000. Measured at 500x640 with Threads covering: the banner reads inert === true, opacity: 1, filter: none, the scrim's top edge is 88px and the banner's bottom edge is 88px, and .lf-threads-toggle still carries aria-expanded="true" and the open workspace's active tint. A pointer click at that toggle's centre leaves panel.open === true and aria-modal="true" exactly as they were.

So the one region of the background a reader can still see undimmed is the region holding the control that names the open workspace, and pressing it does nothing. The comment this PR deletes from panel-workspace.js named that press as how a covering panel closes, and test_workspaces_replace_each_other_and_name_the_open_one now presses Escape before each banner peer for the same reason — a covering workspace can no longer be replaced from the banner at all. The TODO item this PR retires asked that covered document controls stop being keyboard-reachable; the banner is not covered, so which way this resolves is a product call — extend the scrim and the boundary over the banner, or keep the workspace doors live above it. Either is defensible; a lit control that advertises the open workspace and answers nothing is the outcome I would rule out.

Comment thread skills/leaf/assets/runtime/trays.js Outdated

@leaf-agent leaf-agent left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The delta settles the last round: the scrim now stands over the banner with the surface above it, and pointer entry lands on the first row of either tray. One new finding, from the merged tree.

The address vocabulary stands down with the page, so a covering workspace can only be left by Escape. stack()'s covering branch keeps scopes rooted in the workspace plus RETURN, and Go to is page-rooted (skills/leaf/assets/runtime/keyboard/address.js, const GO declares no root), so the g prefix that arms it is suspended for as long as the posture holds. Measured on this head at the suite's default 1200px-wide viewport, with Leaves open from g Shift+l: body[data-lf-modal-workspace] reads lf-leaves, a second g leaves body[data-lf-goto] unset, and the key line reads ⏎ open it in a tab · esc back · ? more. Escape still closes, and the PR already took the Escape-first route for the banner peers in test_workspaces_replace_each_other_and_name_the_open_one, so standing the chords down in the covering posture is a defensible call — except that Leaves covers at every width, so g Shift+l's own Close the All leaves panel row is unreachable for the page's whole life rather than at one breakpoint, and the same holds for g Shift+m, g Shift+v, and the peer workspace addresses from inside any covering surface.

test_named_workspace_chords_toggle_their_panels states the contract this breaks — "Repeating a panel's complete address closes the panel it opened" — and it fails here on the Leaves iteration while passing at main's tip (7e7f3e05). It arrived in #530, after this branch's merge base, so the branch has never run it; test_render_* is nightly and the PR gate skips it, so nothing on this branch's checks reports it either. Whichever way the call goes, that test needs to state it: re-admit the address scope over a covering workspace, or press Escape first there the way the workspace-replacement case now does.

Comment thread skills/leaf/assets/runtime/keyboard/dispatch.js

@leaf-agent leaf-agent left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The delta settles the last round: g stands in the covering posture, the peer workspace addresses toggle from inside a modal surface, and test_named_workspace_chords_toggle_their_panels states a contract the branch now keeps. One finding on what the new route reaches, and one on what the test gave up for it.

The versions menu that g Shift+v now opens above a covering workspace has no keys of its own. stack() discards the remembered native layer whenever it is a popover the workspace does not contain (the const layer = ternary in keyboard/dispatch.js), and the covering filter under it keeps only scopes whose root is the workspace — so VERSIONS, rooted at versionMenu, is dropped while the reader is standing in it. The popover branch reaches the same result from the other side: with workspace set, aboveBoundary admits only workspace-rooted scopes. A modal dialog escapes this, which is why g Shift+m is fine — .lf-page-map-sheet is showModal(), so PAGE_MAP survives the filter.

Measured on this head at 500x800 with Leaves covering, g Shift+v: focus lands on .lf-version-row as the new case asserts, and the key line then reads esc back · d / u page down / up · ? more — the covering workspace's own scroll rows, offered to a reader who is inside the menu. Without a covering workspace the same press reads esc back · ⇥ leave forward · ? more. Tab there leaves the menu open and moves focus to .lf-edge outside it, where the plain case runs version.leave-forward and closes it. version.walk and the numbered rows are dropped by the same filter on a page with more than one version. Escape still light-dismisses, so nothing is trapped — but the line names presses belonging to the surface behind the one the reader is in, and the exit the menu declares is gone.

The module's new docstring covers the case that was reachable before: "A popover inside the workspace remains above that floor." This delta makes one outside it reachable — that is what adding [popover] to overlay() is for — and the floor still treats it as background. Either the layer reading admits a popover the reader has entered above the workspace, or g Shift+v is not one of the addresses that should stand in this posture; the new case asserts the surface opens, is focused and is not inert, so it reads as the first.

The toggle case gives up the beside posture to state the covering one. resized(page, 500, 800) makes all three panels cover, and the docstring narrows to a covering panel. At the default 1200x900 this case passes on this head — main.inert is false for Threads, false for Asks, true for Leaves — so the wider contract #530 wrote still holds and one case can carry both postures. Nothing else asserts the beside half; the covering half is already carried by test_a_closed_leaf_clears_itself_off_the_tray's new g Shift+l close and by the switching case added beside it.

Verified: test_render_navigation.py and test_render_controls.py in full on this head. The two failures there — test_the_register_is_the_only_way_a_key_enters_the_runtime and test_a_tall_local_comment_survives_its_panes_posture_and_return — both reproduce at main's tip (013f3c44), so neither is this branch's.

Comment thread skills/leaf/assets/runtime/keyboard/dispatch.js Outdated
Comment thread tests/test_render_navigation.py Outdated

@leaf-agent leaf-agent left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The delta settles the last round: the versions menu keeps its own scopes above a covering workspace, and the toggle case states both postures again. One new finding, from the same change.

A popover that was already open when the posture turns covering takes the keyboard from the workspace. Dropping the const layer = ternary makes stack() honour whatever currentNativeLayer reports, and that reading is "the newest open layer", not "a layer opened above the workspace". background() in workspace-modality.js exempts every [popover] child of chromeRoot through overlay(), so a menu opened before the workspace covered stays open, stays un-inert, and is still the newest layer when the posture flips.

Measured on this head: at 1400x900, g Shift+t (Threads beside, main.inert false), then g Shift+v, then resize to 500x800. Threads enters the covering posture and takes focus — document.activeElement is .lf-threads, .lf-panel carries aria-modal="true", main.inert is true — while the versions menu is still :popover-open behind it. The key line then reads navigation.return, version.later version.earlier, version.open-v1 version.open-v2, reference.open: the panel's own rows (thread.find, comment.write, page.down page.up, scroll.down scroll.up, navigation.address.open) are gone, because the new inLayer(scope) clause admits VERSIONS and its claims: allButTheReference takes the rest. One ArrowDown then moves document.activeElement to .lf-version-row, outside the surface that declares itself modal; the focusin recovery in workspace-modality.js exempts anything inside :popover-open, so nothing pulls the reader back. At ed9558f the same sequence leaves the line reading the panel's rows and keeps focus inside it.

Escape still unwinds — menu first, then panel — so nothing is trapped. What is wrong is the boundary: while that menu stands, aria-modal="true" is a claim the keyboard does not keep, and the resize round trip is one of the paths this PR's own description names as preserved.

The docstrings the delta writes state the narrower rule and it is the right one — "A native layer opened above the workspace keeps its own scopes above that floor" in dispatch.js, "A native layer opened above a covering workspace owns entries made inside it" in return-stack.js. currentNativeLayer cannot tell the two apart on its own. Either the layer reading distinguishes them — a layer the workspace does not contain counts only while it holds focus — or the modality owner takes the background's layers with the background, dismissing an open popover that is not above the workspace as enter() runs. The second also makes overlay()'s exemption true as written, since today it exempts a popover that was never opened above anything. The new case covers only the menu opened from inside the workspace, so whichever way it goes, the pre-existing-layer entry needs a case of its own.

Verified: test_render_navigation.py and test_render_controls.py in full on this head — controls is green, and navigation's two failures (test_the_register_is_the_only_way_a_key_enters_the_runtime, test_a_tall_local_comment_survives_its_panes_posture_and_return) are the same two that reproduce at main's tip.

Comment thread skills/leaf/assets/runtime/keyboard/dispatch.js

@leaf-agent leaf-agent left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The delta settles the last round: a popover standing when the posture turns covering is dismissed as enter() runs, and the new case pins it. One finding on what that dismissal cannot see.

A popover the reference put away is stood back up above the boundary. enter() dismisses the layers open at the instant it runs, and reference.js holds one across that instant: shortcutReferenceLayers = [...document.querySelectorAll(":popover-open")] is captured as ? opens the modal reference — showModal() clears every auto popover, which is what the stash exists to undo — and the closing branch calls layer.showPopover() on each. A posture change in between is invisible to both halves. The popover is already hidden when openNativePopovers() reads, so there is nothing to dismiss, and the restore stands it up afterwards without asking who owns the boundary by then.

Measured on this head: 1000x800, g Shift+t with Threads beside (main.inert false), focus a thread, g Shift+v, then ? ?. Resize to 500x800 — body[data-lf-modal-workspace] reads lf-threads, main.inert is true, .lf-panel carries aria-modal="true". Escape closes the reference and the versions menu is :popover-open again, with document.activeElement on .lf-version-row, outside the surface that declares itself modal; the key line reads version.later version.earlier · version.open-v1 version.open-v2 · version.activate, the panel's own rows gone and its claims with them. Escape still unwinds — menu first, focus back to .lf-thread, then the panel — so nothing is trapped; what is wrong is that aria-modal="true" again names a boundary the keyboard does not keep.

That is the state last round's finding described, reached through the one path that re-opens a layer rather than opening one, and it is why I have left the dispatch.js thread open rather than resolving it: currentNativeLayer still cannot tell a layer opened above the boundary from one standing beneath it, and the delta removes the entry-time producer of the second kind rather than the kind itself. Either that reading makes the distinction, or the restore consults the covering owner — a stashed layer the covering surface does not contain is one the modality owner would have dismissed, so skipping it there puts both halves under the rule the docstring already states.

Comment thread skills/leaf/assets/runtime/workspace-modality.js Outdated
@max-sixty
max-sixty merged commit 08b4de9 into main Sep 10, 2026
7 checks passed
@max-sixty
max-sixty deleted the codex/workspace-continuity branch September 10, 2026 11:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants