15 commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
| a2b05c56c9 |
feat(makepad-table): opt-in capabilities feature, and raise the matrix_client defect
The two caveats from the dependency investigation. ## The capabilities feature Camera and location attachments are now available behind `features = ["capabilities"]`, which pulls `nigig-uikit` and supplies `UikitAttachmentProvider`. Measured: 89 crates by default, 275 with the feature on. That cost is real and it is inherent, not packaging waste. `camera_widget` imports `send_geocode_request` and `request_map_tile` from `nigig-core`, both of which call `spawn_async` — the shared Tokio runtime — and the first makes an HTTPS call to Nominatim. A camera that geocodes needs an async runtime and an HTTP client; there is no lighter honest version. It is affordable because it is opt-in, and because any app enabling it already depends on `nigig-core`, so that app's own tree grows by nothing. Everything touching `nigig-uikit` is in one module, so the boundary is a file rather than `#[cfg]` scattered through the widget. The provider holds no widgets of its own: the host owns the `CameraWidget` already in its tree and this asks it to open, because a provider that instantiated a second camera would fight the first for the device. A second request while one is outstanding is refused rather than overwriting. The table turns that refusal into `AttachmentUnavailable`, so the user is told the camera is busy instead of watching their first request vanish. File picking is deliberately declined here — `robius-file-picker` already ships unconditionally and costs nothing, and two paths for one job is one too many. Two CI gates, both verified to fail when they should: the opt-in build must keep compiling, and the default build must pull none of `tokio`, `reqwest`, `hyper`, `clap`, `csv`, `image`, `nigig-uikit` or `nigig-core`. The second checks the resolved `cargo tree` rather than the manifest, because feature unification can switch an optional dependency on from a sibling crate. Tests 99 default, 105 with the feature. Both clippy-clean. ## The matrix_client defect Raised in REVIEWS/MATRIX_CLIENT_FEATURE_GATE.md rather than fixed. It is not my crate, nothing depends on the broken combination, and a blind fix could change behaviour someone relies on. `matrix_client` declares `native = ["dep:tokio", "dep:reqwest", "dep:rusqlite"]` but its source gates on `#[cfg(not(target_arch = "wasm32"))]`. Two switches for the same modules, so on a native target with the feature off the modules compile and their dependencies do not — 19 errors, 26 ungated uses across 7 files. There is no CI job for the crate, which is why it rotted unnoticed. The note corrects an overstatement I made while arguing for the trait hook. I said fixing this would unblock wasm. It would not: `matrix_client` already builds clean for wasm32 with `--no-default-features`, and `nigig-core` has 8 wasm errors of its own (`crate::platform::spawn` missing) that have nothing to do with it. The only broken combination is native-target-with-feature-off, which nothing builds. I also said earlier that `matrix_client` was heavy — it is a 7-dependency local crate, not matrix-sdk. That was wrong and it inflated the case for the trait hook; the note records the measured numbers instead. |
|||
| 7737096858 |
refactor(makepad-table): adopt Robrix's image decode path
Replaces the hand-rolled try-PNG-then-JPEG with `pageflipnav/src/utils.rs::load_png_or_jpg`, the pattern the Robrix-derived app in this repo already uses. Two things it does better: - It sniffs the header with `imghdr` and calls the matching loader directly, so a JPEG does not decode-and-fail as a PNG first on every cold cache. - It still falls back to trying both when the sniff names something unexpected or nothing at all. `imghdr` is not perfect, and a mislabelled file is more useful decoded than refused. `imghdr` has no transitive dependencies — it reads a header and names a format. It is already a dependency of `pageflipnav` at the same version. The upstream version logs the failure and dumps the bad bytes to disk. That is right for a chat client receiving untrusted media and wrong here: this runs from the draw path for every attached cell, so a broken file would log once per frame. The caller already caches the failure and draws a labelled chip naming the file, which tells the user more than a log line would. Tests 94 -> 99. Verified by removing the sniff and by removing the fallback; each fails the ordering test. `TextOrImage`, the other candidate for reuse, is referenced in `room_screen.rs` but not defined anywhere in this checkout — it is upstream Robrix only, so there was no baseline here to adopt. |
|||
| 01ebeeb7fe |
feat(makepad-table): real image rendering with a resize anchor, and per-row heights
Two things: attached images are decoded and drawn rather than shown as a placeholder chip, and row heights become genuinely per-row. **Image rendering.** Decoding follows `pageflipnav/src/utils.rs`, the Robrix-derived app in this repo: try PNG, then JPEG, because a header sniff is not reliable enough to choose on its own. The decoded texture is cached per cell, and a cache entry of `None` records a file that could not be read so a broken path is attempted once rather than every frame — `draw_walk` runs at 60Hz and re-decoding a photo there would be the slowest thing in the widget by a wide margin. One `Image` widget repositioned per cell, matching `cell_editor` and `math_cell`, with the texture swapped from the cache. A pool would let several textures live at once but needs runtime template instantiation and a reuse policy; this is the same number of GPU uploads with far less machinery. On first successful decode the real pixel size is written back to the attachment, so the row is sized from the true aspect ratio instead of the placeholder guess. **The resize anchor.** A grab square at the image's bottom-right corner. Dragging it writes `ImageSizing::Fixed`, which pins the height so a later relayout cannot overrule what the user chose, and the row follows because `row_height_for` reads the same value. The floor is asserted at compile time against the anchor size: an image dragged smaller than its own grab handle could not be grabbed again, and the user would have to delete the attachment to recover it. The anchor is hit-tested before the cell, or dragging it would open the editor instead. Sizing lives on the attachment rather than in widget state, so it survives a column reorder along with the image. **Per-row heights.** `TableRow::height` holds a dragged override and `row_height_for` honours it. Item 5's row resize previously assigned `self.row_height`, which is table-wide — dragging one row's handle resized every row at once. `RowGeometry`, added with the attachments, now carries the consequence: rows below a resized one shift down. Tests 86 -> 94 (140 across the tree), all five crates clippy-clean. Verified by reintroducing four defects. One of those guards did not work first time and the gap was mine. Deleting the per-row override branch from `row_height_for` left the whole suite green: the tests checked that `TableRow::height` could be *stored*, and nothing checked it was ever *read*. Storing a value no one consults is exactly the shape of "the handle does nothing". `the_row_override_is_actually_consulted` now asserts the connection at both ends — that `row_height_for` reads `row.height`, and that the resize writes it rather than the table-wide field. |
|||
| 3c751f18bd |
feat(makepad-table): cell attachments and per-row heights (item 6)
The last of the nine. Items 1-5, 7, 8 and 9 shipped in |
|||
| bbdfe823f8 |
feat(makepad-table): row gutter, header select/resize, long-press menus, one input model
Items 1, 2, 3, 4, 5, 7 and 9 of the nine reported. Item 8 shipped separately
in
|
|||
| 1887b54efa |
fix(makepad-table): repaint when the cell editor takes focus (item 8)
The editing cell's text did not appear until the pointer moved. `begin_edit` cannot focus the editor directly. `set_key_focus` takes an `Area`, and the editor only has one once it has been drawn, so focus is deferred: `begin_edit` sets `needs_editor_focus` and the next `handle_event` calls `set_key_focus` on the now-valid area. That deferred step changed how the editor draws — the caret starts blinking and the focused colour states apply — but it never asked for another frame. The editor therefore kept painting its unfocused appearance until something unrelated triggered a redraw. Moving the mouse was that something, which is why the text appeared only after moving the pointer away. One `self.redraw(cx)` after focus is taken. Tests 62 -> 63; verified by removing the call, which fails the new test. This is item 8 of nine reported together. The other eight are new capability — a row-number gutter, header editing, resize handles, a cell context menu with attachments, and a touch path — and are being scoped separately rather than bundled into a bug fix. |
|||
| dffa140123 |
fix(makepad-table): measure text with the layouter instead of estimating it
Some checks failed
email.yml / fix(makepad-table): measure text with the layouter instead of estimating it (push) Failing after 0s
repo hygiene / hygiene (push) Has been cancelled
makepad-table / model (push) Successful in 20s
makepad-table / widget (push) Successful in 1m41s
makepad-table / hygiene (push) Successful in 6s
Right-aligned text still overflowed its column after the previous fix, because that fix kept the 7px-per-character estimate and only clamped the result. Real glyphs at this size average wider than 7px, so the estimate came out short, `pos.x + width - pad - estimated_width` placed the run too far right, and it ran past the cell edge. The clamp only guards the left side. No fixed per-character figure can work here: under-measuring overflows and over-measuring leaves a visible gap. `draw_cells` now measures with the same layouter that draws the run — `DrawText::layout(..).size_in_lpxs.width`, the pattern `glass_panel.rs` uses — and both the truncation and the alignment use that measurement, so the two cannot disagree. `align_text_x` takes a width rather than a string. `fit_text_to_cell` becomes `fit_text_measured`, taking a measuring closure and bisecting for the longest prefix that fits; laying out a string is not free, and a long value in a wide column would otherwise be measured once per character every frame. Tests 59 -> 62, and the existing ones were rewritten around an injected measurer. `varied()` gives different characters different widths, as a real font does, so the suite now fails on anything that assumes a constant width. Verified against four defects. Three of the guards did not work on the first attempt, and all three failures were mine: - **Estimating the alignment width — the actual reported bug — passed.** The source check asserted `size_in_lpxs` appeared *somewhere* in `draw_cells`, and the fitting call still mentioned it after the aligning call had been replaced with an estimate. It now counts both measurements and rejects any `chars().count() as f64 *` in the draw path. - **Removing the clamp passed.** Every test reached `align_text_x` through `fit_text_measured`, and once text has been shortened to fit, the clamp never fires. `alignment_clamps_a_run_wider_than_its_cell` calls it directly with an over-wide value. The clamp still earns its place: the fitted width and the aligned width are separate measurements of separate strings, and any disagreement is what it catches. - **An off-by-one in the bisection hung instead of failing.** `lo = mid - 1` stops the interval shrinking and the loop spins forever — a frozen frame, not a wrong pixel, and no assertion catches it without a timeout. The loop is now a bounded `for` capped at the number of steps a correct bisection can need, so the same mistake produces a wrong answer that a test can see. `the_fit_search_terminates_on_hostile_input` covers degenerate measurers. |
|||
| 2ba06f34c5 |
fix(makepad-table): stop overlong cell text spilling into the next column
Some checks failed
email.yml / fix(makepad-table): stop overlong cell text spilling into the next column (push) Failing after 0s
repo hygiene / hygiene (push) Has been cancelled
makepad-table / model (push) Has been cancelled
makepad-table / widget (push) Has been cancelled
makepad-table / hygiene (push) Has been cancelled
A right-aligned cell whose text is wider than its column drew over the left-aligned text in the column beside it. Two separate causes, one on each side of the cell. **Spilling left.** `align_text_x` computed `pos.x + width - pad.right - text_width` for right alignment and `pos.x + (width - text_width) * 0.5` for centre. Once `text_width` exceeds the column width both go *negative relative to the cell*, so the run started outside its own cell and reached backwards over the previous column. That is the reported collision: the right-aligned column's text sitting on top of its left-aligned neighbour. The result is now clamped to the padded left edge, so an overlong string starts where a left-aligned one would and runs forwards. **Spilling right.** Clamping alone only fixes the near side — the run would still continue past the cell's right edge into the following column, which is the same overlap seen from the other direction. `fit_text_to_cell` shortens anything wider than the padded width and appends an ellipsis. `DrawText` can do this itself via `text_overflow: Ellipsis`, but only through `draw_walk`, which needs a turtle; these cells are drawn absolutely with `draw_abs`. The truncation therefore uses the same 7px-per-character estimate the positioning already used, so one approximation is applied consistently rather than two that can disagree. When a real text measurer replaces it, both callers change together. Order matters in `draw_cells`: the text is truncated first and the *truncated* string is aligned. Aligning the original and drawing a shorter one positions the run by a width it no longer has, which puts the overlap back. Tests 51 -> 59. Verified by reintroducing four defects: removing the clamp fails the left-spill test, removing truncation fails three, aligning the full text fails the ordering test, and measuring bytes fails the multi-byte test. Two of those needed the tests fixed first, and both were mine: - The draw-path ordering had no coverage at all, because `draw_cells` needs a live `Cx`. It now reads the source for the order of the two calls. Blunt, but an accidental reordering is exactly the regression this invites. - `truncation_counts_characters_not_bytes` passed with `len()` substituted for `chars().count()`. The byte count only gates *whether* to truncate, and the case I wrote was one that should be truncated either way. The test now uses a multi-byte string that comfortably fits — which `len()` would wrongly shorten — so the assertion turns on the difference rather than merely being near it. |
|||
| 306504bc0b |
fix(makepad-table): pin the pressed-state fill, and put the caret after the text
Some checks failed
email.yml / fix(makepad-table): pin the pressed-state fill, and put the caret after the text (push) Failing after 0s
makepad-table / model (push) Has been cancelled
makepad-table / widget (push) Has been cancelled
makepad-table / hygiene (push) Has been cancelled
repo hygiene / hygiene (push) Has been cancelled
Two bugs in the table demo's cell editor, both reported after the previous
colour pass.
**White on click.** `TextInput`'s `draw_bg` blends through six fill states,
and the previous change pinned five of them. It missed `color_down`. The
widget's animator holds `down: 1.0` for as long as the pointer is pressed on
it, so clicking into a cell blended toward the theme's light fill and stayed
there until the pointer moved off and the state decayed — which is exactly
the reported "goes white when I click, correct once I move the mouse away".
It read like a hover state and was the press state.
`color_down` is now pinned, along with the six `color_2*` gradient partners.
The shader only mixes those when `color_2.x > -0.5` and the default is a
-1.0 sentinel, so they were inert — but the theme sets them, and anything
that later turned the gradient on would have pulled theme colours back in.
**Caret at the start.** `begin_edit` called `set_text` and nothing else.
`set_text` loads the value and leaves the cursor at index 0, so typing into a
cell that already had content inserted at the front. `move_cursor_text_end`
after the load puts it where every spreadsheet puts it, and where
`commit_edit` reading the whole buffer back already assumed the user was
working.
Tests 49 -> 51, and the existing state test was rewritten. It had been
checking a hand-written subset of states and passed the whole time
`color_down` was missing — a test that only covers the states someone
remembered is a test that misses the one they forgot. The fill states are now
their own exhaustive check.
Verified by reintroducing each defect: removing `color_down` fails the fill
test, removing the caret call fails the caret test, and swapping the caret
call before `set_text` fails it too.
That last case needed the test fixed first. The ordering guard compared
`body.find("set_text")` against `body.find("move_cursor_text_end")`, and
`find` returns the first match anywhere — including inside the doc comment
above the code, which mentions `set_text`. Swapping the two statements left
the comment in place, so the naive check still passed. It now compares the
first non-comment line containing each call.
|
|||
| 60117099ae |
fix(makepad-table): make an editing cell readable — white fill, dark ink, one border
Some checks failed
email.yml / fix(makepad-table): make an editing cell readable — white fill, dark ink, one border (push) Failing after 0s
repo hygiene / hygiene (push) Has been cancelled
makepad-table / model (push) Has been cancelled
makepad-table / widget (push) Has been cancelled
makepad-table / hygiene (push) Has been cancelled
Three reported problems, one cause. `TextInput` does not have a colour, it has a set of them, and `get_color` blends between them: `color_focus` on focus, `color_empty*` when the field is empty, `color_hover`, `color_down`, plus `border_color_2*` gradient partners for every border state. The cell editor set four properties — `color`, `color_focus`, `border_color`, `border_color_focus` — and left the rest to fall back to the active theme. The active theme is the dark one: `theme_desktop_dark::script_mod` runs after the light one in `widgets/src/lib.rs` and wins. So the moment a cell took focus the editor drew `theme.color_text` (near white) on `theme.color_inset_hover` (dark grey), and an emptied cell hit the `color_empty` path, which was also unset. Fixed by pinning every state rather than the ones that happened to be visible in one configuration: 1. **No border of its own.** `border_size: 0.0` and every border state transparent, including the six `border_color_2*` partners, which render as a faint grey edge even at zero width on some backends. The 2px green frame is `draw_select`, drawn over the same rect in `draw_walk` — the editor's border sat inside it and read as a doubled frame. The green selection is unchanged. 2. **Editing ink equals resting ink.** `#x1f2937` in all eight text states, the same colour a non-editing cell uses. An editing cell should look like a resting cell with a caret in it; the caret is the affordance and a colour change only costs contrast. `draw_cursor` is pinned too — `theme.color_text_cursor` is chosen for a dark inset and nearly vanishes on white, which would have left no cue at all. 3. **Plain white fill**, in all five fill states rather than just two. This is also the "background goes white and the text disappears" case: that was the fill switching to the pinned white while the text switched to the unpinned theme colour, so the two moved independently. `draw_selection` is pinned to a pale green that matches the frame, instead of the theme's blue, so selected text stays dark-on-light. The same latent bug was in all six of the invoicer's inputs — the five header fields and the search box — which set the same four properties. They are pinned the same way. Their placeholders stay grey deliberately: a placeholder is a prompt, not content. Tests 44 -> 49. The colours live in `script_mod!`, which is data this crate does not parse, and no test can ask a widget what it drew without a GPU — so these read the DSL source and assert the states are present. That is blunt, and worth having anyway, because the failure mode is precisely "compiles, runs, looks wrong only on screen". Verified by restoring the original four-property block: all five fail. |
|||
| 624d6b846f |
feat(makepad-table): document library for Phase 3, and correct every stale note
Some checks failed
email.yml / feat(makepad-table): document library for Phase 3, and correct every stale note (push) Failing after 0s
repo hygiene / hygiene (push) Has been cancelled
makepad-table / model (push) Has been cancelled
makepad-table / widget (push) Has been cancelled
makepad-table / hygiene (push) Has been cancelled
Two things: the model layer Invoicer UI Phase 3 needs, and a sweep of the documentation, which was still describing the crate as it was six phases ago. `DocumentLibrary` in `makepad-doc-model` — open, save, recents and search, in the model rather than the app so it is testable without a window. The UI over it is not built; the README says so rather than claiming the phase. Three things it gets right that are easy to get wrong: - A document number becomes a filename and is user-controlled text. `safe_file_stem` replaces anything outside `[A-Za-z0-9._-]`, so `../../etc/passwd` cannot steer a write out of its directory. Leading dots go too, and a fully-stripped name falls back to `untitled`. - An empty query matches everything, so clearing the search box restores the list instead of emptying it. All terms must match, so each word narrows. - Recents de-duplicate and move to the front. Without that, re-opening one file fills the list with it and evicts everything else. doc-model tests 22 -> 31. Verified by reintroducing four defects: dropping the filename sanitising fails 3, removing the recents de-duplication fails 1, and switching the search from all-terms to any-term fails 2. The empty-query guard is honestly untested and marked as such below. Two test expectations I wrote were wrong and the code was right, which is worth recording because both look like search bugs and are not. Searching "globex" returns the invoice *and* the receipt — both are addressed to Globex, and finding every document for a client is the point. Searching "inv-2024-001" also returns both, because the receipt's line item reads "Invoice INV-2024-001 — Brand identity + website": it is the payment for that invoice, and surfacing it is the useful answer. Documentation, all of which had drifted: - `src/table.rs` called itself a "Phase 1 + 2 scaffold" with "Phase 3+ (SCAFFOLD ONLY — emits actions, no UI yet)". All six phases are implemented; the header now summarises what each one does. - `src/lib.rs` said Phase 3+ was "scaffolded via TableAction emissions but not yet implemented", and did not export the Phase 6 types at all. `parse_solid_spec`, `wireframe_edges`, `project_isometric`, `SolidSpec`, `SolidSpecError` and `Point3` were public but unreachable from the crate root. - `TableAction::RowMenuRequested` / `ColMenuRequested` were documented as "Phase 3 will open a PopupMenu". The widget opens the menu itself; these are notifications, not requests. - Both demos logged "Phase 3 will open PopupMenu" and neither handled `ColumnMoved`, so a Phase 4 drag produced no output in either. - The invoicer's header described a toolbar of six buttons that does not exist and a context menu as pending. - The README's caveats section listed three "if the compiler complains" predictions from before the crate had ever been built. All three are settled — `KeyCode::Tab` is right, `TextInput` needs no `ComponentRef`, pdf-writer 0.15 compiles as written — so it now lists the five real remaining limitations instead. - The README's workspace tree omitted `examples/table_demo` entirely and described `table.rs` as 1145 lines; it is 3260. The "drop into makepad" instructions were quietly wrong after Phase 5 and are now corrected with verified line numbers. They say to register `Table` next to `chart`, which at the pinned revision is line 611 — but `MathView` registers at 617, and `Table`'s DSL body names `mod.widgets.MathView`. Following the old advice literally would register the widget six lines before the type it depends on. |
|||
| c5eefaaea4 |
feat(makepad-table): 3D cells (Phase 6)
Some checks failed
email.yml / feat(makepad-table): 3D cells (Phase 6) (push) Failing after 0s
repo hygiene / hygiene (push) Has been cancelled
makepad-table / model (push) Has been cancelled
makepad-table / widget (push) Has been cancelled
makepad-table / hygiene (push) Has been cancelled
The last of the README's table phases. `CellKind::Solid3d` reads a short
textual description of a solid from the cell and draws an isometric wireframe
of it:
cube 10 20 30 / cube 10 / sphere 5 / cylinder 3 12
Separators may be spaces or commas, names are case-insensitive, `box` and
`cyl` are aliases. Text in, geometry out — the cell stays a `String`, so a 3D
column saves, loads and round-trips exactly like every other column, and the
kind travels with the column through a drag-reorder.
A wireframe rather than a shaded render, deliberately. Shading needs a 3D
pass with its own camera, depth buffer and lighting shader; the CAD viewport
elsewhere in this repo spends about 2,500 lines on precisely that. A cell
forty pixels tall gains nothing from it. Edges projected isometrically and
stroked with `DrawVector` need no pass of their own, and isometric has no
camera to configure and cannot degenerate. Curved solids are drawn as rings,
not their full triangulation: a 40px cell cannot resolve hundreds of
triangles and stroking them would cost more than the rest of the table.
The projection scales to fit and centres, so a 1-unit and a 1000-unit cube
are drawn identically — without that a cell shows either a dot or nothing.
Degenerate inputs (no edges, an inset larger than the cell, a zero-size cell,
geometry that collapses to a point) return nothing rather than dividing by a
zero span, because `DrawVector` silently drops a path containing NaN and the
cell would just look empty.
Dimensions must be finite and positive, and a rejected spec draws its reason
in the cell — `[unknown shape: torus]`, `[cube wants 3 args, got 2]`. Same
principle as the LaTeX path: an empty cell and a broken one must not look
identical, or a typo reads as a rendering fault.
Tests 26 -> 44. Parsing: each shape, uniform and three-dimension boxes,
aliases, case, comma separators, and every rejection path including NaN and
infinity. Wireframes: a cube has twelve edges and eight corners, extents are
centred, sphere vertices lie on the radius. Projection: fits inside the cell,
is scale-invariant, is centred, stays finite under extreme aspect ratios, and
returns nothing when degenerate.
Verified by reintroducing three defects: dropping dimension validation fails
1 test, a fixed scale instead of scale-to-fit fails 2, and removing the
re-centring fails 1.
That last one is the interesting case, because on the first attempt it failed
*nothing*. Every primitive is built centred on the origin, so the midpoint of
its projection is already zero and subtracting it is a no-op — the centring
tests could not distinguish "centres the drawing" from "happens to be
centred". `an_off_centre_solid_is_still_centred_in_its_cell` translates a
cube well away from the origin first, and that one does fail. A guard that
cannot fail is decoration, and this one could not until it was checked.
|
|||
| 15ef8a0447 |
feat(makepad-table): LaTeX cells (Phase 5)
Some checks failed
email.yml / feat(makepad-table): LaTeX cells (Phase 5) (push) Failing after 0s
repo hygiene / hygiene (push) Has been cancelled
makepad-table / model (push) Has been cancelled
makepad-table / widget (push) Has been cancelled
makepad-table / hygiene (push) Has been cancelled
A column can now be declared as maths rather than text:
TableColumn { kind: CellKind::Latex, ..Default::default() }
Rendering goes through makepad's own `MathView`, which is already registered
with the script VM and already owns a glyph cache and the `makepad-latex-math`
parse/layout path. Reimplementing that inside the table would have been a
second copy of the same logic with none of the same testing.
The kind lives on the column, not the cell. Cells stay `String`, so a
`TableData` built before this existed keeps working and a table still
round-trips through plain text. A column of formulae is also the realistic
case — a spreadsheet does not mix prose and LaTeX down one column — and it
means the decision is made once per column rather than re-derived per cell
per frame. `CellKind` defaults to `Text`, so nothing changes for existing
callers except that the struct gained a field.
One `MathView` is repositioned over each maths cell in turn, the same pattern
`cell_editor` already uses. A widget per cell would allocate a glyph cache per
cell. The walk is `Size::Fit` rather than fixed to the cell, because
stretching a glyph run to fill a cell distorts the maths.
An expression that does not parse is not blanked. `MathView` draws its own
`[reason]` marker, so a mistyped formula is visible in the cell rather than
silently erasing the content — the failure mode that makes a formula column
untrustworthy.
Also extracted `align_text_x`, which was inline in `draw_cells`. It counts
characters rather than bytes; a multi-byte string measured with `len()` is
pushed off the cell entirely. The 7px-per-character approximation is
unchanged and still crude — correcting it needs a real text measurer, not a
different guess — but it is now in one place and under test, so replacing it
will be a visible diff rather than a silent shift.
Tests 21 -> 26. New: kinds default to Text, a Latex column keeps its kind
through a drag-reorder, left alignment ignores content, centre and right
alignment against the stated approximation, and character-vs-byte counting.
Verified by reintroducing two defects: measuring bytes fails the multi-byte
test, and resetting kind during a reorder fails the kind-preservation test.
The invoicer's six columns gained an explicit `kind`. That break was caught
by the `Invoicer and demo compile` step added with the workflow in the
previous commit, which is what it is there for.
|
|||
| ab17c72c55 |
feat(makepad-table): drag-reorder columns, and the first tests this crate has
Some checks failed
email.yml / feat(makepad-table): drag-reorder columns, and the first tests this crate has (push) Failing after 0s
repo hygiene / hygiene (push) Has been cancelled
makepad-table / model (push) Has been cancelled
makepad-table / widget (push) Has been cancelled
makepad-table / hygiene (push) Has been cancelled
Phase 4 of the README's table, plus the test infrastructure Phases 1-3 never
had. The crate had zero tests before this; it now has 21.
Drag-reorder:
- A press on a column header no longer commits to an action. It arms a drag
and resolves on release: travel more than 8px and it reorders, release
without travelling and it opens the column menu as before. Without that
ambiguity resolved, every menu open would jitter into a one-pixel drag.
The threshold matches `TouchTracker::MOVE_THRESHOLD` so a mouse and a
finger agree on what a drag is.
- While dragging, the carried column is tinted full-height and a 2px bar
marks the boundary it would land on. The bar is suppressed when the drop
is a no-op, so no bar means nothing will happen rather than a bar sitting
misleadingly at the source edge.
- `TableAction::ColumnMoved { from, to }` fires only when the index actually
changed, so a host persisting column order is not asked to write on every
wobble. An open cell editor is cancelled, because it addresses a cell by
index and the indices just moved underneath it.
`draw_drag: DrawVector` — declared, never used anywhere — is replaced by two
`DrawColor` layers. `DrawVector` is a full tessellator with path, vertex,
index and paint state; a translucent rectangle and a vertical bar do not
need any of it.
Testability, which needed a structural change rather than a test file:
`Table` derives `Script` and `Widget`, so it has no `Default` and cannot be
constructed without a live `Cx`. Nothing about it was unit-testable. The
logic worth testing does not need a widget, so it moved off it —
`ColumnGeometry` owns boundary and drop-position arithmetic, and a free
`reorder_columns` owns the move. `Table` forwards to both, and
`compute_layout` now goes through `ColumnGeometry` too, so there is one
implementation rather than two that can drift.
The 21 tests cover column geometry at even and uneven widths and at a
non-zero origin, drop-position resolution including the exact-midpoint case
and clamping outside the table, the index shift in both directions, no-op
drops, out-of-range refusal, cells travelling with their header, ragged
rows, a permutation property over repeated drags, and the Phase 3 menu's
geometry and hit-testing.
Verified by reintroducing three defects separately: removing the shift for
the removed source column fails 7 tests, dropping the no-op guard fails 1,
and moving headers without their cells fails 3.
Phase 3 was marked "scaffolds only" in the README and was in fact
substantially complete — menu state, open, hit-test, apply, and drawing all
present, with 15 row and column actions wired. Corrected to done, with its
geometry now under test.
Also adds `.forgejo/workflows/makepad-table.yml`, the first CI this tree has
had. Every step passes `--manifest-path` explicitly: the crate is excluded
from the root workspace, so `-p` from the repo root cannot reach it and
`--workspace` skips it — omitting the flag does not fail loudly, it silently
tests nothing. The workflow gates tests, clippy at `-D warnings` and fmt,
and asserts three invariants that would otherwise regress quietly: that the
exclusion still holds from both sides, that no manifest tracks a git branch
instead of pinning a revision, and that monetary fields stay integer.
Each gate was checked by breaking what it protects. The exclusion check
caught a defect in itself while being tested: a bare grep for the path also
matched the explanatory comment above the exclude list, so deleting the
entry and keeping the comment passed. It now anchors on the quoted entry.
Two pre-existing clippy warnings fixed so the new `-D warnings` gate starts
from zero.
|
|||
| 258fa3259e | Merge origin/main: resolve xref/document conflicts, add makepad_table |
