Grid, array editor and JSON: crash fix, scroll tearing, row JSON preview, selectable JSON colours - #92
Merged
Conversation
Double-clicking an array cell replaced the whole view with the error screen: effect_update_depth_exceeded, an effect reading and writing the same state. The array editor's seed effect assigned `items` and then read `items.length` on the next line to size the input list. That read made the effect depend on the state it had just written, and because every run assigns a brand new array the dependency fired again immediately - nothing in it ever converged. It is built into a local now and both the assignment and the length come off that. The cell editor had the same shape in its own seed effect: it wrote `maxLineLen` and read it back a line later to decide soft wrap. That one happened to settle, because the cell-identity guard above it returns before the write on any re-run, but it is the same mistake and it is measured into a local too.
Eleven elements came out as eleven separately bordered pills stacked on top of each other, each one looking like its own field to fill in, when the thing on screen is one list. It is one bordered surface with hairline separators now, and the values inside it are borderless until focused. The row controls faded in on hover. They moved nothing, but the row looked different every time the pointer crossed it, and they were out of reach of the keyboard entirely. They are always drawn now, just quiet, and come up to full contrast with the row. Rows are h-7 inside an h-8 row, which is the scale the rest of the app uses; they were running around 50px. The drag handle and the index sit back at lower contrast so the values lead. Save is bg-primary, the way ConfirmDialog and the update dialog paint their primary action - this was the one button in the app painted in the foreground colour. Every control in the dialog has a focus ring. One of the input's classes ended in a bare `hover:` with nothing after it.
The same array read three different ways depending on where you looked at it: a
text[] cell showed {Dhaka,Gazipur}, the jsonb column beside it showed
["Dhaka","Gazipur"], and double-clicking either put ["Dhaka","Gazipur"] in the
box to edit. The row you were reading and the value you were editing did not
look like the same thing. Array columns read as JSON now, which is the form the
editor already used. Writes still go out as the Postgres literal - that is what
the server wants, and it was never the right thing to read.
The switch thumb escaped its track at other zoom levels. Its travel was a flat
translate-x-4 that did not account for the track's own inset, so it sat 2px
from the left when off and 4px from the right when on, and the MCP one drew its
track with a 1px border - fixed, while the width, the thumb and the travel all
scale with the app zoom, so the three drifted apart as soon as the zoom left
100%. The tracks carry their inset as padding now, which puts the travel back
on the standard scale and makes it exactly the inner width minus the thumb, and
the border is an inset ring, which costs no layout at any zoom.
Reading a row across a wide table means scrolling sideways and holding the column names in your head to know what you are looking at. Alt+J puts the whole focused row in the dock as one JSON object, names attached, on one screen. It goes through the same rowToRecord the "Copy row as JSON" action uses, so the two agree and hidden columns stay hidden, and it reads through effectiveCellValue rather than the raw row so staged edits are in it - this is the row as it stands, which is what the grid above it is showing. Detached, like the other value-in-dock views: it is a view of a row, not of a cell, so the cursor moving must not re-point it at whatever cell it lands on. Alt+Space steps into it and Escape closes it, the same as for a cell.
The row JSON stayed on the row it was opened from. It is opened detached, so the cell-follow effect leaves it alone - which is right, since it must not be re-pointed at whatever cell the cursor lands on, but it left nothing moving it at all, and the highlighted row and the JSON under it disagreed about which row was being read. A second effect follows the row cursor specifically. Only the row is watched: stepping across columns within a row changes nothing about the row. dataVersion is watched with it, so a staged edit reaches the JSON the same moment it reaches the grid. The expanded row drew its data a step above the toolbar that controls it, so the JSON read larger than its own chrome and larger than the grid it came out of. Tree, raw and the loading line are all on the same rung now. Its context menu was in the code face at text-ui-sm. Those are menu labels, not data, and every other menu in the app draws them in the UI face at text-ui-2xs.
The tree drew at text-ui-sm, a step above the toolbar that controls it, so the data read larger than its own chrome. The raw pane beside it was moved down already; this is the view people actually use. Every key was followed by a floating colon - `id : "x"` rather than `id: "x"`. The row is an inline-flex with gap-1, and that gap falls between every child, including between a key and its own colon. The colon pulls back over the gap now, so it sits against the key and the spacing is left where it belongs, between the key and the value.
A control character has no glyph in any font, so the grid drew each one as a blank box. A value carrying one looked like a value that did not: the mojibake filename read as `â ¯` with a hole in the middle and nothing on screen saying what the hole was, or even that there was a character there at all. They are escaped now - \u0080 and the like - so what is in the cell is readable. ASCII, not a Control Pictures glyph (␀): whatever replaces them has to be certain to render, and those are missing from plenty of monospace faces, which would put the box straight back. Tab, newline and carriage return are deliberately left alone. They are ordinary in text columns, and escaping them would rewrite every multi-line value on screen to fix a problem those three do not have. Display only. Copy, export and the editors all still hand back the real value.
…itself A cell holding pretty-printed JSON read as `[ "Alex Smith", "Live test" ]`. fillText draws no line breaks, so each newline came out as nothing at all while the indentation around it was drawn in full - gaps where the structure used to be. The break and the whitespace either side of it fold to a single space now, which is what the copy-as-TSV path already did with the same values. The scroll blit copied the canvas onto itself, source and destination overlapping by everything except the scroll delta - which is the point of it. That is only safe if the engine snapshots the source first, and WebKitGTK rasterises this canvas on the CPU through Cairo, which copies in place: a band could be read after it had already been written over. What that looks like is a torn frame with rows repeated above the seam, only ever while scrolling. It goes through a scratch surface now - grown as needed, never shrunk, so a scroll allocates nothing after its first frame. Two flat surface copies instead of one, with no text in either, which is still nothing beside repainting 6,000 glyphs.
Alt+J had a binding and no way to reach it with the hand that is already holding the pointer - the same gap Preview cell had, so it sits next to it. It is the same question asked of the row instead of the cell.
Icon read the icon-set and icon-weight stores directly, and reading a store inside a component subscribes that component. The sidebar draws two or three icons a row, so opening a schema with 135 tables in it stood up several hundred store subscriptions, their teardown, and a derived weight per icon - for two values that are the same for every icon on screen and change only when the setting does. They come off one module-level subscription now: two in the process, however many icons are mounted. Nothing about what is drawn changes.
Every app theme rendered JSON in the same four colours. The --json-* tokens were declared once on `html` with a single light override, and not one of the 26 themes touched them, so a theme whose own palette read badly against that blue and green had nowhere to go. Six palettes now, in Settings → Appearance. Auto is what was there before and stays the default, following light and dark. Vivid, Ocean, GitHub and Monochrome are written twice, once for dark surfaces and once for light, because a palette tuned for one is unreadable on the other - the lightness has to move even though the hue does not. Solarized is written once on purpose: using the same accents on both backgrounds is the whole idea of it. It is a data attribute and nothing else. Every consumer already reads the --json-* custom properties, so the palette swaps without a single component re-rendering, and the swatch in the settings row is the palette itself rather than a description of it - which is why the selectors match any element carrying the attribute, not only the root.
Every switch and select carried a paragraph. The longest ran 257 characters and most were three or four lines, so Appearance alone was a long scroll of explanation with the controls spaced out between it, and the thing you came to change was rarely on screen with the thing next to it. They are one line each now: the longest description is 114 characters and the longest switch row 83, down from 257. What went is the justification - why the setting exists, what it is good for, which extension it also lives in. What stayed is what the control does and anything that is genuinely surprising: that turning image previews off stops the download rather than just the drawing, that the PIN has no reset, that search terms leave the machine. No dashes standing in for punctuation either; the only ellipses left are in search placeholders, where they are the convention.
broisnischal
changed the title
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.
14 commits. Build clean, 670 tests passing.
Crashes
Opening an array cell took down the whole view with
effect_update_depth_exceeded.ArrayCellEditor's seed effect assigneditemsand then readitems.lengthon the very next line to size the input list. That read makes the effect depend on state it writes, and since every run assigns a brand new array the dependency fires again immediately. No guard above it could stop the second run, so nothing converged.The cell editor had the same shape in its own seed effect (
maxLineLenwritten, then read back a line later). That one settled because the cell-identity guard returns before the write on a re-run, so it was not what crashed - but it is the same mistake and it is fixed too.Rendering
Scroll tearing. The blit copied the canvas onto itself, source and destination overlapping by everything except the scroll delta, which is the point of it. That is only safe if the engine snapshots the source first. WebKitGTK rasterises this canvas on the CPU through Cairo, which copies in place, so a band could be read after it had been written over: a torn frame with rows repeated above the seam, only ever while scrolling. It goes through a scratch surface now, grown as needed and never shrunk, so a scroll allocates nothing after its first frame.
Multi-line cells showed gaps.
fillTextdraws no line breaks, so a newline came out as nothing at all while the indentation around it was drawn in full - pretty-printed JSON read as[ "a", "b" ]. Breaks and the whitespace either side now fold to a single space, which is what the copy-as-TSV path already did.Control characters drew as an empty box. No font has a glyph for them, so a value carrying one was indistinguishable from one that did not. They escape as
\u0080now. ASCII, not a Control Pictures glyph - the replacement has to be certain to render, and those are missing from plenty of monospace faces.The switch thumb escaped its track at other zoom levels. Its travel was a flat
translate-x-4that ignored the track's own inset, and the MCP variant drew its track with a 1px border - fixed, while the width, thumb and travel all scale with the app zoom. Tracks carry their inset as padding now and the border is an inset ring, which costs no layout at any zoom.Features
Alt+J previews the whole row as JSON in the dock and follows the row cursor, through the same
rowToRecordthat "Copy row as JSON" uses, so the two agree and hidden columns stay hidden. Also on the right-click menu.Selectable JSON colours. The
--json-*tokens were declared once onhtmlwith a single light override, and not one of the 26 themes touched them - so every theme rendered JSON in the same four colours. Six palettes now, in Settings → Appearance. Four are written twice, once for dark surfaces and once for light, because a palette tuned for one is unreadable on the other. Solarized is written once on purpose: the same accents on both backgrounds is the whole idea of it.Design and performance
{Dhaka,Gazipur}in atext[]cell,["Dhaka","Gazipur"]in the jsonb column beside it, and["Dhaka","Gazipur"]in the edit box. Writes still go out as the Postgres literal; that was never the right thing to read.toTextno longer stringifies the cell value on every keystroke that misses its guard.Not in this PR
Two things I checked and deliberately left alone, because the data was wrong rather than the app:
attachmentfilenames are stored mojibake (a Latin-1 mis-decode of U+202F, confirmed in both directions), and the backend Unicode pipeline is clean - emoji and U+202F round-trip with correct byte and character counts.