Skip to content

chore(deps): re-pin MoonUI to the visible-range fix - #359

Closed
guyverino wants to merge 1 commit into
mainfrom
chore/re-pin-moonui-visible-range
Closed

chore(deps): re-pin MoonUI to the visible-range fix#359
guyverino wants to merge 1 commit into
mainfrom
chore/re-pin-moonui-visible-range

Conversation

@guyverino

Copy link
Copy Markdown
Collaborator

What & why

Renaming a core in Settings → Connections was impossible: clicking the name field put the caret in and took it straight back out, and the field swallowed no keystrokes.

The cause was in MoonUI, not here. MoonVirtualList::on_visible_range called its observer from the closure it hands to uniform_list — and that closure has a second job: measure_item renders one row to obtain a row height, from request_layout and again from prepaint, before the real range exists. So the observer received a phantom 0..1 twice per frame ahead of every real range.

settings/connections/mod.rs::on_conn_visible_range acts on that range destructively — window.blur(), feed_open = None, picking = None for anything outside it. Entry 0 of that list is always a heading, never a core row, so every core row was evicted one frame after it gained focus. The regression arrived with #349 (virtualising the Connections list); the other consumer, the chart stack, survived it only because its handler is an idempotent assignment that the real range repaired later in the same frame.

Fixed upstream in MoonUI#56 (850de06d): UniformList gained its own on_visible_range channel, invoked once per prepaint with the range that actually goes to the item renderer. This PR is the re-pin — no terminal code changes, because the consumer here was already written against the range the list actually draws.

Notable decisions

  • Fixed in the fork rather than worked around here. A filter like "ignore a one-item range" would have been a heuristic, and the same trap was still waiting for the next consumer of that API; MoonVirtualList::on_visible_range had no documented contract at all before this.
  • The upstream fix also settles two adjacent cases the observer never distinguished: a list holding no rows now reports 0..0, while a list that holds rows but shows none of them (zero height, mid-collapse, not yet sized) stays silent — so a collapsed panel can no longer be mistaken for an empty one and blur the field the user is typing into.

Known limitations

  • MoonDataTable still observes from inside its item renderer and carries its own visible_range.len() <= 1 guard against the same phantom. That guard is now redundant and also swallows a legitimate one-row range. Deliberately left for a separate PR — the table is the most heavily used component here and deserves its own verification pass.
  • Chart-stack scene gating was checked while re-pinning: it does not depend on this observer (chart_tabs/mod.rs:1143-1155, windows.rs:131,592 gate scenes by tab and window activity), so the new silence at zero height leaves no scene running.

How to verify

cargo build -p moon-ui-gpui --bin moonterminal --target x86_64-pc-windows-msvc --all-targets
cargo clippy -p moon-ui-gpui --bin moonterminal --target x86_64-pc-windows-msvc --all-targets -- -D warnings
cargo test -p moon-core --target x86_64-pc-windows-msvc
cargo test -p moon-ui-gpui --target x86_64-pc-windows-msvc
  • Build: clean, all targets.
  • Clippy: the two -D warnings errors reproduce identically on a detached origin/main, and this diff contains no .rs at all — pre-existing debt, no new finding.
  • Tests: moon-core 1399 + 14 + 3 + 3 + 1 + 6 + 3 + 1 + 10 green; moon-ui-gpui 1206 units + 245 theme_contract green.
  • FireTest: not run (chart-smoke / order-cancel-lag).
  • By hand, on a build against the fix: Settings → Connections → the core name field keeps focus and accepts text.

Upstream side, for the record: MoonUI guardrails PASS (437 component tests), cargo test -p moon-gpui 180 green, and six new behavioural tests held by a new audit contract, each proven by mutation.

MoonUI 850de06d gives `UniformList` its own `on_visible_range` channel, so the
observer is no longer called from the closure that also measures a row. Before
it, `MoonVirtualList` reported a phantom `0..1` twice per frame ahead of every
real range, and the Connections tab acted on it: `on_conn_visible_range` evicts
state for rows outside the reported range, so it blurred the focused core-name
field one frame after the click and no core could be renamed.

No terminal code changes — the consumer was already written against the range
the list actually draws.
@guyverino

Copy link
Copy Markdown
Collaborator Author

Not needed: CI refreshes the MoonUI crates itself on every build (cargo update -p moon-gpui -p moon-gpui-platform -p moon-ui in build.yml), so the fix reaches every build without a hand-written re-pin. The committed lock pins third-party versions; the MoonUI revision in it is expected to trail.

@guyverino guyverino closed this Aug 28, 2026
@guyverino
guyverino deleted the chore/re-pin-moonui-visible-range branch August 28, 2026 08:22
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.

1 participant