Grid and cell-dock fixes: focus, Space preview, wrap hang, shortcuts - #90
Merged
Merged
Conversation
Escape closed the dock and stopped there. Focus was sitting on an element that was about to be removed, so it fell back to <body>, and the grid answered no arrow keys until a cell was clicked again. The same applied to the close button and to staging a change, which are the other two ways out. All three go through one `dismiss()` that closes and tells the owner, and DataTable puts focus back on the grid container the way the undo path already does.
Three fixes that all came out of using the grid. Space now opens the focused cell in the dock. Space is a printable character, so it fell through to type-to-edit and opened the editor with a space typed into it: the one keystroke on the grid that destroyed the cell it was aimed at. Enter and any other character still start an edit, Shift+Space still previews, and the shortcut list and context menu say Space now. Confirm dialogs focus their confirm button instead of parking focus on the dialog box. The box held focus so that Enter would not hit Cancel, but nothing on screen then looked focused - Ctrl+Shift+D opened Disconnect with no visible answer to "where am I". The confirm button is the default action and already says so with its ↵, so it is the honest place for focus. Enter activates it the ordinary way, and the Enter handler already steps aside for a focused button so it still fires once. Its ring is drawn on :focus as well as :focus-visible, because WebKitGTK does not reliably treat focus set in code as focus-visible. The oversize cell notice had two Load buttons for one action, one in the toolbar and one in the notice. The notice keeps it, since that is where the reason is, and the toolbar keeps the size chip as status. The notice copy said in three sentences what it says here in one.
Turning wrap on for a jsonb cell holding a file froze the app. The value is one line of 563,607 characters, and CodeMirror virtualises by line: rows off screen cost nothing, but a single line always lays out whole. Wrapping that one meant measuring every character into a few thousand visual rows in one frame. Unwrapped the same value is cheap, because the line is one row and the view draws the slice that is on screen. So past a longest-line cap the toggle is disabled rather than merely defaulted off, and says why on hover - turning it on *is* the hang, and Alt+Z was a way around the old default. The existing 2MB rule stays; it was measuring the wrong thing, since 500KB spread over 10,000 lines wraps fine and 500KB on one line does not. Syntax highlighting now stops at the same kind of boundary. A line past 20,000 characters gets no language, so no parse and no highlight - the number VS Code uses for editor.maxTokenizationLineLength, for the same reason. The longest line is measured once per cell, with indexOf rather than split, so measuring a value big enough to be the problem does not allocate a second copy of it.
Alt+Shift+F/S/C/R for filter, sort, columns and reset are three keys each and nobody remembers which letter goes with which menu. They are now Alt+A, Alt+S, Alt+C and Alt+R, which is the shape the toolbar already used for Alt+N. Filter takes A because Alt+F is spoken for; the other three keep their letter. Alt+Space steps into the cell preview. Space opens the dock and deliberately leaves the cursor on the grid, so arrows keep walking the table and the preview follows along - which left no way in other than the mouse. Escape still brings focus back out. Every one of the five was checked against the whole registry in shortcuts.js and the createHotkey bindings first; none of them collides.
broisnischal
changed the title
The page boots at opacity 0 and JavaScript reveals it, but the window is shown from Rust on its own timer. So anything that stopped the bundle before armRevealFailsafe() ran left a painted black rectangle on screen with no way out but killing the app: a throw at module scope, an import that never resolves, one of JavaScriptCore's SIGTRAPs. Two changes, either of which would have been enough on its own. armRevealFailsafe() now runs first in main.js. It used to sit behind applySettings, installZoomShortcuts and resetWebviewZoom, none of which the page needs in order to be visible, and a throw in any of them took the failsafe with it. index.html carries a floor that needs no JavaScript at all: a keyframe that ends at opacity 1 after 8s, well behind the 2.5s JS failsafe so a healthy boot always reveals first and this is never what shows the app. revealApp() sets data-revealed, which stops the animation and beats the inline opacity:0. Checked by deleting the bundle from a copy of the built page: the window still comes up, at opacity 1, with no app JavaScript on the page at all.
Alt+A opened the filter bar on whatever column happened to be first, so the filter you actually wanted meant picking your column out of a list that can be eighty long - while the cell you wanted to filter on was already under the cursor. It now seeds that column, and because the operator is chosen from the column's type the row comes up ready to type into rather than ready to configure. The first column is still the fallback when nothing is focused, and a stale name that is no longer in the table falls back the same way. DataTable had no public surface at all, so this adds one function to it rather than lifting the cell cursor into the shell.
Three fixes to the same idea: Load means show me the value. The in-cell Load button left the dock showing a different row. It re-read the dock only when the dock already happened to be on that cell, which is not where it usually is - the button is in the cell, clicking it does not move the cursor, and the dock follows the cursor. So the value loaded and the panel went on saying "not loaded" over some other row. The dock now goes to the cell that was loaded, because loading a cell is a request to see that cell. A value past the 8MB inline cap answered with a toast explaining that the value was too big and that Space would page through it. Load already was the request; answering it with instructions for a key the reader could press themselves is not an answer. It opens the dock on that cell, which reads it in pages. Soft wrap is remembered. It was decided per cell from the text - structured unwrapped, prose wrapped - so turning it on meant turning it on again at the next cell, and the next. The reader's answer is kept in localStorage and used from then on; the old guess is only the default until they give one. Wrap forced off because a line is too long to lay out is not a preference and is not saved as one.
The search bar had a Search button, and nothing happened until it was pressed. Typing is the search now: 400ms after the typing stops, which matters more here than in most boxes because one search is a query per table, ten at a time - on a 135-table schema, firing per keystroke is 135 round trips for a letter about to be followed by another. Two characters is the floor; one matches most of the database and is not a search yet. Changing a match option re-runs it too, since the options change what the query means. Enter still works and means "do not wait for the pause". Escape empties the box and the results with it, and only when there is something to clear, so an empty box lets Escape through to whatever else is listening. The button is a clear affordance instead, shown only when there is text. runSearch no longer refuses while a search is running. A newer search supersedes an older one rather than being dropped by it; the generation check already stops the old workers. The bar was a h-9 field inside py-2 padding, so it stood ~52px against the 36px sidebar header next to it and the two did not line up. It is a h-9 row holding a h-7 control now, which is the sidebar header's height and the same control height as the schema picker and filter box in it. The regex error moved to its own row, since a second child of a fixed-height flex row sits beside the field rather than under it.
Shift+Space opened the dock and then chased it with a focus call on the next tick. The editor inside the dock is lazy-loaded, so that call could land before it existed, or before the seed replaced its document and took the selection with it - it worked sometimes, which is worse than never. Focus is a parameter of the open now, set before `cellEditorOpen` so the panel has it the first time it seeds. The panel already knew how to wait for its own editor; that is the path autofocus used before it was turned off for the grid, and the caret rule moved in with it - end of a short value, top of a long one. Ctrl+F on the Find in database page focused nothing. Mod+F already means "search what this page is showing" and the objects page had its own branch; the search page now has the same one, through the same bindable the objects page uses. It selects as well as focuses, so Ctrl+F on a page that already has a query replaces it by typing instead of appending to it.
The changeset still described only the first commit, so merging with a release label would have written a changelog that mentioned the focus fix and none of the nine changes after it. It now lists all of them, grouped the way CHANGELOG.md groups things. The updater entries are deliberately absent: Retry, the 15s check timeout and the per-frame download progress already shipped in 2.1.2 and are in the changelog under that version.
The FK sub-view drew its own HTML table from three booleans lifted off the grid - row rules, column rules, zebra - so every style that is not plain solid lines came out as plain solid lines in it. Dotted, dashed, hairline, double, bordered, ledger, graph and bands all collapsed to the same table, sitting directly under a grid that was drawing something else. The rest of the style travels with the metrics now, and the panel translates it into what a CSS border can say: the grid's dash array picks dotted or dashed, double and strong are borrowed as-is, and groupEvery gets its heavier rule every Nth row. Dashes are given back some contrast, because so much of the line is missing that they read lighter than a solid rule of the same colour. Corner dots and column ticks do not cross over and are left out rather than approximated into something the grid never draws. The header's own bottom rule stays solid whatever the style: it separates the table from its labels, not one row from the next, and a dashed version of it reads as a missing row. Line numbers in the cell editor are now a toggle (Alt+L), remembered the same way soft wrap is. Wrap already hides them while it is on, since a gutter numbering logical lines against wrapped visual rows either disagrees with the count in the bar or lies, so the button is disabled there and says why. Hiding the numbers drops the fold arrows with them - a fold column with no numbers beside it is a stripe nothing explains.
The window comes up before the app is ready to be seen, so what was on screen for that stretch was the painted background and nothing else - a black rectangle that reads as a hang. The opacity gate moved off <html> and onto #app, which is what lets anything be drawn during the boot at all. The root keeps its painted background, so the anti-flash work is untouched; it is simply no longer invisible. Under the app sits a splash: the brand mark and an indeterminate bar, inline in index.html and depending on nothing the bundle provides, because it has to be able to draw when the bundle is what failed. The mark is picked in CSS off the `dark` class the head script has already set, so the splash needs no JavaScript either. The bar is indeterminate on purpose - startup has no progress to report and a bar that pretends to measure one is a lie told at every launch. The floor that reveals the app if nothing else does is now an inline classic script, which runs even when the module bundle is the thing that failed - the case it exists for. The CSS animation stays as a second layer for when the engine itself stops, but it is no longer the only one: a keyframe animation needs frames to advance, and a failsafe that depends on the thing that may have stopped is not one. Verified by deleting the bundle from the built page: the splash shows, then the app is revealed and the splash retires.
The window came up before the app was ready and showed a black rectangle for the gap. It now shows the Stroke mark and name, and nothing that moves: startup has no progress to report, so there is nothing a spinner could say that the mark does not already say by being on screen. The reveal itself was the reason that gap was seconds long. revealApp waited for the show IPC to come back and then for a requestAnimationFrame - a frame a hidden window never produces - so when the show did not land, the page stayed hidden and what eventually uncovered it was a timer. It sets the attribute synchronously now and asks the window to show itself afterwards, which is the right order anyway: the first thing on screen is the finished app rather than a page fading in. Both boot timers are gone with it, and nothing in the startup path waits on one any more. They existed to rescue a window stuck on black, and there is no such state left to rescue: the splash is what shows whenever the app is not ready, so the worst case is the mark staying up rather than an empty rectangle.
…er cost A table whose columns are all foreign keys is exactly where the dock earns its keep, and arrowing along one kept showing the relation the dock was opened on: the cursor said credits_credithistory and the dock said authtoken_token. Only the row was watched, so a sideways move changed nothing at all. It follows the cell now - moving onto a different foreign key is a request to see that one, and the relation, its label and the column all move with the cursor. A reverse relation hangs off the row rather than any one column, so that one still follows rows only. Because the dock follows the cursor it re-renders on every arrow key, and it was doing far too much per cell to afford that: a seven-argument class join, three calls to work out whether the row was a group edge, a four-interpolation inline style, and three event closures - on every cell, fifty rows at a time. None of it varied by cell. The class strings are built once. The geometry and the rule weight come off custom properties on the table, so a cell now carries no inline style at all. The group-edge test happens once per row instead of three times per cell. And the body has one delegated listener rather than three per cell, which the cells were already set up for - they name themselves in data-fk-cell. Also enlarges the boot splash mark and stacks it over the name.
Everything describing the value in the dock's bar was shrink-0 - the type badge, the row, NULL, read-only, the line and character counts, the size chip. Only the column name could give way, so once there were enough of them the row grew past its own width and pushed Stage and Close off the end of it. They are one shrinking group now: a narrow dock takes room from the description, and the buttons stay where they are. The row number counts up as the cursor moves and was set in proportional digits, so the step from row 9 to row 10 shifted everything after it. Tabular now, like the counts beside it. The column name truncates and had no title. It is the only place the value says what it is, and it is the first thing to be given up when the bar runs short, so the full name (with its type) is now reachable on hover.
The dock's seed effect converted the value to text at the top, above both of its guards. So every re-run paid for a full JSON.stringify of the cell and then threw the result away - twice for anything small enough to pretty-print, since that path stringifies once to measure and again to format. It re-runs a lot. The dock follows the cell cursor, so every arrow key is a run, and most of them hit the guard two lines later and return without using the text at all. It did the same work with the dock closed. The value is still read at the top, because that read is what registers the dependency, but it is only converted once the guards have agreed there is a new cell to seed.
The splash drew its name and a 72px hole where the mark should be. The two PNGs were served off the dev server and WebKitGTK reserved the box without painting any of it, which is why the name sat below centre instead of on it. They are data URIs now, so the splash makes no request at all. That is what it should always have been: the screen exists to be on show when other things have not come up yet, and a boot screen that depends on a fetch has the same problem it is there to cover. 160px of source for a 72px box, so it stays sharp on a HiDPI screen, and 15KB for the pair after resizing down from 1024px. Checked by loading the built page from disk with the bundle blocked: the mark decodes and draws with nothing available to fetch from.
The toggle was disabled whenever wrap was on, and said so on hover. The reasoning was that a gutter numbering logical lines disagrees with wrapped visual rows - but that is how every editor that wraps draws it: the number sits on the line's first visual row and the continuation rows carry none. Tying the two together only took away a choice that works.
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
1 participant
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.
Six things from using the grid. No
release:*label, so nothing ships until you say so.1. Escaping the dock lost grid focus
CellEditorPanelclosed by settingopen = falseand nothing else, so focus sat on an element it was about to remove and fell through to<body>. Escape, the close button and staging a change all had the hole. All three now route through onedismiss()that firesonclose, andDataTablerefocuses the grid the way the undo path already does.2. Space previews a cell
Space is a printable character, so it fell through to type-to-edit and opened the editor with a space typed into it - the one keystroke on the grid that destroyed the cell it was aimed at. Space now previews. Enter and any other character still start an edit. Shift+Space still works, and the shortcut list and context menu say Space.
3. Space no longer steals focus, Alt+Space steps in
The preview keeps the cursor on the grid so arrows keep walking the table and the dock follows along. That left no way in except the mouse, so
Alt+Spacemoves focus into the editor. Escape brings it back.4. Soft wrap hung the app on a long line
A jsonb cell holding a file is one line of 563,607 characters. CodeMirror virtualises by line: rows off screen cost nothing, but a single line always lays out whole, so wrapping that one measured every character into thousands of visual rows in a single frame.
Unwrapped it is cheap, because the line is one row and the view draws the on-screen slice. So past a longest-line cap the toggle is disabled rather than defaulted off and says why on hover, since turning it on is the hang and Alt+Z was a way around the old default.
The existing 2MB rule stays, but it was measuring the wrong thing: 500KB over 10,000 lines wraps fine, 500KB on one line does not.
Syntax highlighting stops at the same kind of boundary. A line past 20,000 characters gets no language, so no parse and no highlight. That is the number VS Code uses for
editor.maxTokenizationLineLength, for the same reason.The longest line is measured once per cell with
indexOf, notsplit, so measuring a value big enough to be the problem does not allocate a second copy of it.5. Dialogs showed nothing focused
Confirm dialogs parked focus on the dialog box so Enter would not hit Cancel, which meant
Ctrl+Shift+Dopened Disconnect with no visible answer to "where am I". Focus now goes to the confirm button - the default action, which already says so with its ↵. Enter activates it normally and the Enter handler already steps aside for a focused button, so it still fires once. The ring is drawn on:focusas well as:focus-visible, because WebKitGTK does not reliably treat focus set in code as focus-visible.6. View shortcuts are one modifier
Filter takes A because Alt+F is spoken for. All five new bindings were checked against the full registry in
shortcuts.jsand everycreateHotkeycall; none collides.Also: the oversize cell notice had two Load buttons for one action. The notice keeps it, since that is where the reason is, and the toolbar keeps the size chip as status.
Checks
npm run buildclean,vitest661 passed across 43 files.