Skip to content

LT-22692: Rebuild the Avalonia detail view once per burst of outside edits - #1238

Merged
thejambi merged 7 commits into
mainfrom
LT-22692-detail-refresh-parity
Nov 12, 2026
Merged

thejambi merged 7 commits into
mainfrom
LT-22692-detail-refresh-parity

Conversation

@thejambi

@thejambi thejambi commented Nov 9, 2026 •

Copy link
Copy Markdown
Contributor

Typing into the shown entry from another view, such as a column of the Entries list, no longer makes the new (Avalonia) detail view lag. It used to rebuild the whole detail view on every keystroke; now the first change shows at once and the rest of the burst updates the view once, a quarter-second after typing stops. A single edit, Undo or Redo still shows immediately.

In Notebook, undoing an edit to a record's Text field now redraws the view instead of leaving the old text until F5, and so does a change to the top-level record while one of its subrecords is selected.

The rest of LT-22692 already behaves as the ticket asks: Undo and Redo update the view in place on the same record and keep the scroll position, F5 rebuilds on the same record, and F5 while typing commits the typed text. F5 and Styles OK reach the Avalonia view through #1236, already merged.

Reviewer notes

Builds on #1236 (LT-22811), merged. #1236 added RecordEditView.RefreshDisplay, which makes F5 and Styles OK recompose the Avalonia view (the ticket's F5 step), with tests. This branch has been rebased onto main since, so its diff is the LT-22692 work only.

Root cause. AvaloniaDetailRefreshController.PropChanged sent every relevant change from outside the view straight to a full recompose on the next UI tick. Its queue only merged changes raised in the same call stack, and each keystroke typed in another view is its own unit of work, so each one rebuilt the whole detail view. The WinForms DataTree updates the affected slice in place, which is why it does not lag.

Root cause, Notebook. RecordEditView.IsChangeWithinEntry counted a change only when its owner chain reached Clerk.CurrentObject. Notebook's Text row (obj field="Text" in NotebookParts.xml) shows a Text the record references, not owns, so a paragraph's owner chain (StTxtPara, StText, Text) never reaches the record. And Notebook shows the top-level record while a subrecord is current, so a change to the top-level record's own fields was judged against the subrecord and missed.

Deliberate rules.

  • The first outside change of a burst rebuilds at once; later ones wait until no further change has arrived for 250 ms and rebuild once. 250 ms is a judgement call, not a measured value: long enough to batch typing, short enough to feel immediate after a pause.
  • The wait is supplied by the host through a new optional deferUntilQuiet argument; without it the controller refreshes every change as before. RecordEditView supplies a restartable WinForms timer, created on first use, disposed with the controller, and always run on the UI thread (a change raised elsewhere re-posts the wait there).
  • A rebuild the view asks for itself (after its own commit, F5, Styles OK) is never held, and it clears any change still waiting, since it reads current state.
  • An open edit still holds every rebuild until it completes, including one that comes due when a burst goes quiet.
  • A change held for one entry does not rebuild the view once the user has moved to another entry; showing that entry already read current state. A change to the newly shown entry during the same burst still rebuilds.
  • A change is relevant when its owner chain reaches the record the view shows or any object one of its rows is bound to (DetailField.ObjectHvo). Both are captured when the view shows a record and cleared when it shows a message, clears for a missing record or a suspended load, or is torn down; the shown record is not resolved again on each change notification, so a notification raised mid-delete cannot walk off a deleted object's owner. While no record is remembered as shown, relevance falls back to the clerk's current record, so a record the clerk selects without a show still refreshes on an outside change.
  • Moving to another record forgets the shown record before the open edit settles, so that commit is judged against the record about to be shown and does not rebuild it a second time.

Tests. DetailEditGuardAndSchedulingTests gains six controller tests, each driving real LCModel units of work (one per keystroke): a burst rebuilds once at the start and once when quiet; a single change rebuilds once; a rebuild the view asks for mid-burst covers the waiting change; a burst that goes quiet during an open edit holds the rebuild; a burst that goes quiet after moving to another entry does not rebuild it; a change to the newly shown entry during a burst still rebuilds. For Notebook, two relevance tests on a real notebook record with a subrecord and a referenced Text (a paragraph counts only through the Text row's bound object; the top-level record's change counts against the shown record), and ShowingARecord_RemembersItAndItsRowObjects_ForRefreshRelevance checks the wiring through a real Lexicon Edit show. Through the live view in DetailObjectCommandExecutionTests: leaving an entry with an edit open builds the next entry once (it fails with a second build when the forgetting is removed); with no record remembered as shown, an outside change to the clerk's current entry brings it up; and the quiet wait runs only the last deferred action, once. xWorksTests: 1941 of 1943 passed (2 skipped).

Manual tests (done). On Sena 3 with the new UI, the ticket's acceptance steps using the toolbar Undo and Redo: an edit commits on clicking away; Undo and Redo revert and restore in place, keeping the scroll position and the record; F5 rebuilds on the same record with edits intact; F5 while typing in Gloss commits the typed text; Undo while typing first saves the text as its own step. Typing several characters into the entry's Gloss in the Entries list no longer lags, and the detail view follows. The dictionary preview at the top of the pane turns gray while it refreshes in the WinForms UI too. In Notebook Edit, undoing an edit to a record's Text redraws the view without F5, and so does a change to the top-level record's fields while a subrecord is selected.

Out of scope. Ctrl+Z and Ctrl+Y inside Avalonia fields are trapped by the text boxes' own undo; that is LT-22820. The toolbar Undo and Redo tooltips show the generic ContextHelp.xml text in both UIs; the specific label is on the Edit menu. The Entries list scrolling the selected last row to second-to-last after an edit is the browse view's own behaviour. A change to an object a row only names, such as renaming a semantic domain a sense lists, still does not redraw the view, since no row is bound to that object.

🤖 Generated with Claude Code


This change is Reviewable

@thejambi
thejambi added this pull request to stack #1239 November 9, 2026 19:35
@github-actions

github-actions Bot commented Nov 9, 2026 •

Copy link
Copy Markdown

NUnit Tests

    1 files  ± 0      1 suites  ±0   8m 47s ⏱️ - 6m 11s
7 254 tests +13  7 169 ✅ +13  85 💤 ±0  0 ❌ ±0 
7 263 runs  +13  7 178 ✅ +13  85 💤 ±0  0 ❌ ±0 

Results for commit c8bf2ff. ± Comparison against base commit facc384.

♻️ This comment has been updated with latest results.

@codecov-commenter

codecov-commenter commented Nov 9, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 76.59574% with 22 lines in your changes missing coverage. Please review.
✅ Project coverage is 40.84%. Comparing base (facc384) to head (c8bf2ff).

Files with missing lines Patch % Lines
...xWorks/Avalonia/Hosting/RecordEditView.Avalonia.cs 70.31% 11 Missing and 8 partials ⚠️
...valonia/Hosting/AvaloniaDetailRefreshController.cs 89.28% 0 Missing and 3 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1238      +/-   ##
==========================================
+ Coverage   40.83%   40.84%   +0.01%     
==========================================
  Files        1573     1573              
  Lines      357160   357249      +89     
  Branches    41812    41828      +16     
==========================================
+ Hits       145835   145911      +76     
- Misses     182019   182026       +7     
- Partials    29306    29312       +6     
Files with missing lines Coverage Δ
Src/xWorks/RecordEditView.cs 62.19% <100.00%> (+0.16%) ⬆️
...valonia/Hosting/AvaloniaDetailRefreshController.cs 83.72% <89.28%> (+7.25%) ⬆️
...xWorks/Avalonia/Hosting/RecordEditView.Avalonia.cs 64.29% <70.31%> (+0.57%) ⬆️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Base automatically changed from LT-22811-normal-style-font-size to main November 10, 2026 19:23
@thejambi
thejambi force-pushed the LT-22692-detail-refresh-parity branch from 43e4dbd to 8ce3916 Compare November 10, 2026 19:23
@thejambi
thejambi marked this pull request as ready for review November 11, 2026 12:52
@mark-sil

Copy link
Copy Markdown
Contributor

Notebook still builds twice when you leave a subrecord with an edit open

ForgetShownRecordIfLeaving compares the resolved record, so moving between two subrecords of the same top-level record (e.g. toolbar Next/Previous, which steps through AllRecords without taking focus from the field) keeps m_shownRecord. The settle-commit in ShowRecordOnIdle then changes a field owned by that top-level record, IsChangeWithinEntry counts it, and the controller queues a refresh that rebuilds the view a second time after the navigation's own show.

On main this didn't happen: relevance was checked against Clerk.CurrentObject, which is already the new subrecord at settle time, and the left subrecord's change doesn't reach it. So it's the same double build c58b3a1 fixed for entries, still open for Notebook's same-root case.

One option: forget the shown record whenever the clerk's current object differs from the one it was shown for, not only when the resolved root differs. A Notebook version of LeavingARecordWithAnOpenEdit_BuildsTheNextRecordOnce, moving between two subrecords of one record, would cover it.

@thejambi

Copy link
Copy Markdown
Contributor Author

Fixed!

@mark-sil mark-sil left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Zachary Burnham and others added 7 commits November 12, 2026 12:44
Typing into a field of the shown entry from another view, such as a
column of the Entries list, rebuilt the whole Avalonia detail view on
every keystroke, so typing lagged.

The first change made outside the view still rebuilds it at once, so a
single edit, Undo or Redo shows immediately. Further changes that
arrive within 250 ms of the last wait, and the view rebuilds once when
they stop. A rebuild the view asks for itself, after its own commit or
F5, is not held and covers any change still waiting, and an open edit
still holds the rebuild until it completes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The quiet-wait timer now always runs on the UI thread: a change raised
on another thread re-posts the wait there, as the rebuild queue already
does, so the held refresh cannot be lost.

A change held for one entry no longer rebuilds the view after the user
has moved to another entry, which showing that entry already covered.
A change to the newly shown entry during the same burst still rebuilds.

ScheduleOnUiThread's comment sits on ScheduleOnUiThread again.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The comment above the outside-burst test described the per-keystroke
rebuild it guards against rather than the behaviour it asserts.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The refresh controller's constructor set _deferUntilQuiet before
checking its required arguments; it now sits after _schedule and
_isRelevant, in parameter order.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
In the Avalonia Notebook view, undoing an edit to a record's Text field
reverted the data but left the view showing the old text until F5. A
change counted only when its owner chain reached the clerk's current
object, but the Text row shows a Text the record references, not owns,
and Notebook shows the top-level record even when a subrecord is
current.

The view now remembers the record it shows and the objects its rows
are bound to. A change counts when its owner chain reaches either, so a
paragraph of the referenced Text redraws the view, as does a change to
the top-level record while a subrecord is current.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Leaving a record with an edit still open built the next record's view
twice: the left record's commit was judged against the record still
remembered as shown, so it queued a refresh that rebuilt the next
record again. The view now forgets the shown record before settling
when the clerk has moved on, so that commit is judged against the
record about to be shown.

While no record is remembered as shown, refresh relevance falls back to
the clerk's current record, so a record the clerk selects without a
show still refreshes on an outside change. Clearing the view for a
missing record or a suspended load also forgets the shown record.

The quiet wait's tick is a named method with a test, and teardown drops
the pending action with the timer. Comments that described an
entry-only relevance rule now describe the rule as it is.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Notebook shows each subrecord inside its top-level record. Moving from
one subrecord to another of the same record kept that record as the
shown one, so the commit of the edit left open on the first subrecord
looked like a change to the record on display and queued a second
build of the view.

The view now remembers the clerk's current object it was shown for
and forgets the shown record whenever that object changes. The
commit is then judged against the subrecord about to be shown, which
it does not touch, so the view is built once.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@thejambi
thejambi force-pushed the LT-22692-detail-refresh-parity branch from 614f4cc to c8bf2ff Compare November 12, 2026 18:01
@thejambi
thejambi merged commit 32b05ef into main Nov 12, 2026
8 of 9 checks passed
@thejambi
thejambi deleted the LT-22692-detail-refresh-parity branch November 12, 2026 18:18
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.

3 participants