Repository navigation
feat(shell): wire the studio chrome to the seam - #8
Merged
Merged
Conversation
Its _ => None arm meant a new Route variant compiled as ungated. A capability-gated screen added later would then be reachable on a connection that cannot serve it, and the rail would hide an item the URL still opens. Every variant now states its gate.
The global key handler sat on the studio's .app element, so it only heard keys typed inside the connected chrome. Two places missed out: - ModalHost renders outside .app, so Esc typed into a modal input never closed the modal. - The Connection Manager has no .app, so ⌘, and Esc did nothing while disconnected. The mockup honours both in either state. ⌘, and Esc-closes-the-modal now live on a root wrapper in App. ⌘K, ⌘D and Esc-closes-the-palette/popover stay in StudioLayout, which owns those signals; Esc bubbles to the root after them. Disconnecting unmounts the focused .app and drops focus to <body>, outside every handler, so the root re-takes focus whenever the connection goes away. Checked in the running app: ⌘, and Esc while disconnected, Esc inside a focused modal input, and ⌘D then ⌘, with no click in between.
The Query and Streams badges were the literals "3" and "6". The rail now reads them from nav_badges(), which existed but had no caller. A zero count shows no badge, as the mockup does for an idle item. A read still in flight or a failed read shows none either: a badge is a hint, the screens it points at report their own load errors, and a number in its place would be one the server never sent.
The topbar had no breadcrumb. It now shows the view path, with the
mockup's labels ("Streams / CDC consumer", "Spatial map", ...), and
in the Explorer the selected collection after it. The database stays
out of it: the mockup moved that to the database chip.
Route::breadcrumb is an exhaustive match, so a new route must name its
crumb. An unknown Streams tab reads as CDC, the screen the Streams view
renders for it.
The Explorer's selection was a signal local to the Explorer view, so
the topbar could not see it. Selected moves to state::explorer and
Studio provides the signal. A pick now also survives leaving the
Explorer and coming back; before, it reset to the first collection.
The statusbar showed connection, database and role only. It now reads the server version and timezone through session_info(), re-read when the connection changes. A failed read says "session info unavailable" (error text on hover) rather than leaving a blank where the version was, which would read as a server with no version. SessionInfo drops database, role and read_only. ActiveConnection already carries all three from connect, and the chrome reads them there. Two copies would let the statusbar and the avatar popover show different roles once a real decoder fills both. One owner per fact. The mockup's "last query" latency is not shown: nothing produces one until the Query workbench runs through the seam.
The picker listed ActiveConnection.databases, a copy taken at connect. It now reads databases() each time it opens, so a database created since connect appears, and a failed read shows through AsyncView with Retry instead of an empty list. ActiveConnection drops its databases field: one owner for the list. The two copies had already drifted. local-nodedb-dev's profile lists 3 databases, while the free-standing mock::databases() fixture returned 5 others under a comment saying they matched. The mock now remembers which connection connect opened and answers databases() from that connection's profile, as a server answers per session. Switching connections switches the list. Before a connect it returns NotConnected. mock::databases() gives way to databases_for. Selecting a database still changes only the chrome's current_database. No seam method switches the server session's database yet.
"Switch to staging-cluster" and "Switch to prod-replica-eu" were two hardcoded items. The section now lists every connectable saved entry except the active one, from the registry list_connections() fills. On prod it offers local and staging; an offline entry never appears; a registry still loading offers nothing rather than a stale list. switch_targets is a plain function so the selection rule is tested without a renderer. Its tests read the registry through the seam, as the source scan in tests/seam_discipline.rs requires.
The pill was never visible. The stylesheet, ported from the mockup, hid .readonly-pill unless <body> carried a readonly class, which the mockup's JS toggled and nothing in the app sets. The topbar already renders the pill only for a read-only connection, so the CSS gate only ever hid it. The pill now displays whenever it is rendered. SSR tests saw the markup and passed; only the running window showed the pill missing on prod-replica-eu.
Three reads ran once and never again, so after a connection switch the chrome kept the previous connection's answers: - the notification feed (and registry) in App: the bell kept the old connection's feed - nav_badges() in the rail - collection_groups() in the Explorer sidebar, which also never re-read on a database pick, though collections belong to a database Each resource now reads the active connection (the sidebar also its current database), which subscribes it, so a connect, switch, database pick or disconnect re-runs it. The sidebar's existing selection check then drops a pick the new set lacks. The mock answers identically per connection, so this shows only as a re-read; checked in the running app that a switch and a database pick keep the selection and leave badges, bell and statusbar in place.
App constructed a new MockConnectionService on every render. The context kept the first instance, but use_resource re-runs with the closure from the latest render, so the feed reload read a fresh mock. Since the reload now re-runs on connection changes, a database pick brought back every notification the user had marked read.
Shortcuts were split between the App root (⌘, and Esc for the modal) and the studio's .app (⌘K, ⌘D, Esc for the palette and popover). A click focuses the nearest focusable ancestor, so a click inside a modal left focus on the root, and ⌘K then reached no handler. - Move the popover and palette signals from Studio to App, and handle every key on the root. .app is no longer focusable. - Return focus to the root when the modal and palette close, and on disconnect. Esc from a modal input unmounted the focused element and dropped focus to <body>, which also left the keys dead. - Close the palette and popover on disconnect, so a reconnect does not reopen them. - Ignore ⌘, while a modal is open: replacing it discarded typed fields. - Ignore keys while an IME composes text, so Esc cancels the candidate instead of the modal.
The feed, rail badges, statusbar and sidebar read the whole active connection to subscribe, so any write re-ran them: a database pick, or a failed switch. A pick reset the bell to Loading. App now provides a memo of the connection name, and those reads subscribe to it. - The database popover also subscribes, so a switch made from the palette while it is open re-reads the list instead of offering the previous server's databases. - The sidebar no longer re-reads on a database pick. collection_groups() takes no database, so the re-read returned the same set and suggested a per-database read that does not exist.
Resource::read keeps the previous result until a restarted read resolves. Right after a switch, the rail badges, statusbar version and timezone, sidebar and database list showed the last server's answer next to the new connection's name. The mock resolves at once, so this shows only with a backend that waits on the network. resource_state maps a resource to AsyncState and reads a pending re-run as Loading. The four per-connection reads use it, and the rail and statusbar now go through AsyncState like the other reads.
The selection lives at Studio level and outlived a switch. The Explorer keeps a pick while a collection by that name exists, so on a server where that name has another storage mode it kept the old mode and rendered the wrong viewer. A switch now clears the selection, and the Explorer picks the new server's default.
The Connection Manager, the switch popover and the palette each mapped a saved entry's profile to Credentials inline. A change to that rule had to land in three places. They now call SavedConnection::credentials, which keeps the rule: the profile's user, or a blank that the seam rejects with MissingUsername.
Replace would/could in comments added on this branch with the plain present: each describes what the code does, not a hypothetical.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Wires the studio chrome to the seam: rail badges, breadcrumb, statusbar, database picker, and the palette's connection list. Every shortcut moves to one handler at the app root. Still 100% mock.
-D warnings, nextest, all cleanSessionInfolosesdatabase,roleandread_only, andActiveConnectionlosesdatabases. The compiler flags every site. The palette and popover signals move fromStudiotoApp.The design choice to accept or reject
Each session fact has one owner.
Before this PR, role and current database lived on both
ActiveConnectionandSessionInfo. The database list lived on bothActiveConnection.databasesanddatabases(). The mock had already drifted:databases()returned 5 names, and local's profile lists 3.Now:
ActiveConnectionowns identity, capabilities, role and current database. They come from the connect result.session_info()owns server version and timezone, the facts the connect result does not carry.databases()owns the database list. The mock answers for the session the lastconnectopened, and returnsNotConnectedbefore one.If the real client learns the role or database from a session query instead of the connect result, the owner moves to
session_info(). Say so here if you know it does.Where to spend review time
1.
app.rs, one key handler and focus. A key reaches a handler only from a focused element inside it. A click focuses the nearest focusable ancestor. ⌘, and Esc sat on the app root, and ⌘K and ⌘D on the studio's.app. A click inside a modal focused the root, so ⌘K then did nothing.Now every key is handled on the root, and the palette and popover signals live in
App. The root takes focus back when the modal and palette close, and on disconnect. Esc from a modal input unmounted the focused element and dropped focus to<body>, outside every handler.Guards:
isComposingon that Esc.2. Per-connection re-reads. The notification feed, rail badges, statusbar, sidebar and database list re-read when the connection changes. They subscribe to a memo of the connection name that
Appprovides, not toactive. A database pick or a failed switch also writesactive, and neither changes what the server answers.resource_statereads a pending re-run asLoading.Resource::readkeeps the previous result until the new one lands, so right after a switch the chrome showed the last server's values. The mock answers at once, so no test covers that case.Appalso built a new mock on every render.use_resourcere-runs with the latest render's closure, so the feed reload read fresh mock state. A database pick brought back every notification the user had marked read. The backend is now built once.3. Explorer selection, now in
state/explorer.rs. It moves from the Explorer view toStudio, so the topbar breadcrumb can read it. A switch clears it, and the Explorer picks the new server's default. A collection with the same name on another server can have another storage mode, and the old pick kept the old viewer.One thing I want your call on
Does a NodeDB session have a current database? The picker lists
databases(), and a pick updates the chip and statusbar. Nothing reaches the server. No seam method switches the session's database, andcollection_groups()takes no database. With a real client, the Explorer keeps listing the session database's collections under the picked name.Options:
use_database(name)seam method.The sidebar no longer re-reads on a pick, because the re-read returned the same set.
Smaller changes
Route::required_capis an exhaustive match. A new route must state whether it is gated.data_grid.rskeys its columns the same way.<body>carried areadonlyclass, which the mockup's JS set and nothing in the app sets. The topbar renders the pill only for a read-only connection, so the CSS gate is gone.SavedConnection::credentialsreplaces three inline copies of the profile-to-credentials rule.