Skip to content

Fix the writing system drop down not tracking reversal fields - #1121

Open
mark-sil wants to merge 1 commit into
mainfrom
plugin-ws-focus-publish
Open

Fix the writing system drop down not tracking reversal fields#1121
mark-sil wants to merge 1 commit into
mainfrom
plugin-ws-focus-publish

Conversation

@mark-sil

@mark-silmark-sil commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

In the New lexical edit UI, clicking into a Reversal Entries form now moves the toolbar writing-system combo to that form's writing system, the same way every other text field already does. Before, the keyboard switched but the combo kept showing the previous field's writing system.

Why it was broken. Editors built by SliceFactory receive the host's writing-system focus callback. Plugin-built rows never did: SlicePluginBuildContext carried no callback, so ReversalIndexEntryPlugin substituted its own keyboard-only lambda and nothing published WritingSystemUnderCursorChanged. The fix hands the plugin the host's real callback. The question worth a reviewer's time is not "what breaks" (nothing changes for composer-built rows) but whether the callback enters at the right layer, which is covered below.

Where to look

  • RecordEditView.ShowAvaloniaEntry: two named arguments pass OnDetailWritingSystemFocused into Compose. This is the only production wiring, and the only touch on that file.
  • DetailComposer: optional trailing parameter on both Compose overloads and ComposeState, passed into the plugin build context. Parameter plumbing only.
  • SlicePluginBuildContext.WritingSystemFocused: the new seam plugins read.
  • ReversalIndexEntryPlugin: forwards the context callback and drops its private keyboard-only fallback, so keyboard activation and the publish come from one path.
  • DetailWritingSystemStateTests: the new test composes with the host handler, realizes the view headless, focuses the reversal editor, and asserts the toolbar's WritingSystemHvo property moved through Pub/Sub and WritingSystemListHandler.

Deliberately not here

Verification.build.ps1 -CommentHygiene -TokenHygiene green. Targeted test.ps1 over the ws-state, plugin inventory, and reversal composer fixtures: 11/11. Manual check in Sena 3 (New UI mode): combo follows the cursor into the Portuguese and English reversal forms. Full suites not run. Two pre-existing reversal composer tests were order-dependent (no headless platform init) and now pass in isolation.


Design: compose-time threading vs the ControlFactory seam

SliceFactory.CreateCustom already holds a SliceFactoryContext with the host callback when it invokes a plugin's factory, so the architecturally purest fix would pass that context through DetailField.ControlFactory. That changes the delegate type in DetailModel.cs, which PR #1111 rewrites heavily along with DetailComposer.cs. Compose-time threading keeps the #1111 overlap to parameter-list additions.

Consequences accepted for now: the callback lives in two places (Compose and the DataTree constructor), both fed by the same method group in RecordEditView; and further host services for plugins (menu, clipboard) would repeat the same plumbing. The render-time seam is the target once #1111 lands.

Decision: the plugin's keyboard-only fallback is gone

The plugin used to call WritingSystemKeyboards.Activate itself. The host callback already does that and also publishes, so keeping a fallback would double-activate. Without a host callback the plugin row now behaves exactly like every composer-built row (no keyboard switching). No production or preview host composes plugin rows without the callback; the only such callers are tests that do not assert on keyboard behavior.

Preflight review details

Eight-angle review with one verifier per candidate: nine candidates, seven confirmed, two refuted.

Fixed before commit:

  • Orphaned ctor <param> and consumer-narrating property summary on SlicePluginBuildContext.
  • Compose<param> trimmed to its value contract.
  • Headless platform init moved to DetailComposerTests.TestSetup (both reversal tests pass alone).
  • Test registers its reversal index and entry for teardown; teardown deletes all registered objects.
  • Window size aligned with sibling headless tests (the Form does not virtualize).
  • Editor located by its full stamped automation id via the new ReversalIndexEntryPlugin.DefaultAutomationId.

Accepted as is: the new test supplies the host handler to Compose itself; the host's two named arguments are covered by manual test.

Refuted: dual injection points drifting (no callback-less host composes plugin rows); dropping the fallback (parity with composer rows).

Contract changes: optional trailing parameters only, plus one new public constant. No Critical or Important findings.

CI-ready checklist
  • Commit messages follow .github/commit-guidelines.md.
  • The change is unit tested (new headless test; two existing tests hardened).
  • Builds and tests pass locally via build.ps1 and test.ps1.
  • AI-assisted work: pr-preflight ran before requesting review.
  • No AGENTS.md under the touched Src/** folders needs updating.
  • AI code reviewer comments considered (pending review).

🤖 Generated with Claude Code


This change is Reviewable

Avalonia text editors built by SliceFactory receive the host's
writing-system focus callback, so the toolbar ws combo follows the
cursor. Plugin-built rows never did: SlicePluginBuildContext carried
no callback, and ReversalIndexEntryPlugin substituted a keyboard-only
lambda, so focusing a reversal form switched the keyboard but
published nothing.
In FLEx's New lexical edit UI, clicking into a Reversal Entries form
left the toolbar writing-system combo showing whichever writing system
the previous field had, even though the keyboard switched. Users saw
Sena selected while typing into an English or Portuguese reversal
form.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown

NUnit Tests

1 files ±0 1 suites ±0 12m 42s ⏱️ +14s
6 003 tests +1 5 922 ✅ +1 81 💤 ±0 0 ❌ ±0 
6 012 runs +1 5 931 ✅ +1 81 💤 ±0 0 ❌ ±0 

Results for commit c5a9210. ± Comparison against base commit f2829d6.

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 38.55%. Comparing base (f2829d6) to head (c5a9210).

Additional details and impacted files
@@ Coverage Diff @@## main #1121 +/- ##
==========================================
- Coverage 38.56% 38.55% -0.01% 
==========================================
Files 1513 1513 Lines 350995 351003 +8 Branches 40350 40350 ==========================================
- Hits 135351 135334 -17 - Misses 186427 186449 +22 - Partials 29217 29220 +3 
Files with missing linesCoverage Δ
Src/xWorks/Avalonia/Composer/DetailComposer.cs67.56% <100.00%> (+0.06%)⬆️
...xWorks/Avalonia/Hosting/RecordEditView.Avalonia.cs57.21% <100.00%> (+0.14%)⬆️
...Works/Avalonia/Plugins/ReversalIndexEntryPlugin.cs55.45% <100.00%> (+0.90%)⬆️
Src/xWorks/Avalonia/Plugins/SlicePlugins.cs98.03% <100.00%> (+0.08%)⬆️

... and 5 files 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.

Sign up for freeto 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.

2 participants

@mark-sil@codecov-commenter