Skip to content

LT-22858: Show notebook subrecord fields; drop dead prompt rows (LT-22844) - #1203

Draft
thejambi wants to merge 4 commits into
LT-22689-no-senses-bannerfrom
LT-22858-subrecord-fields
Draft

thejambi wants to merge 4 commits into
LT-22689-no-senses-bannerfrom
LT-22858-subrecord-fields

Conversation

@thejambi

@thejambi thejambi commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Start here: DetailComposer.WalkSublayout in Src/xWorks/Avalonia/Composer/DetailComposer.cs -- it lays a subrecord's own layout out under its header.

In Notebook > Record Edit in New mode, each subrecord now shows its fields under its header, chosen by its Type as in legacy, and nested subrecords repeat that one level further in (LT-22858). The header reads 1. Observation expanded and 1. Observation - <title> collapsed. The branch also fixes LT-22844: an empty list whose layout names no ghost field no longer shows a "Type to add ..." box that cannot be typed in.

Why did subrecords show nothing? The SubRecord layout brings the fields in with <sublayout name="Normal" layoutChoiceField="Type"/>, which the importer dropped as publishing-only; it is the only detail-view <sublayout>. What disappears with LT-22844? Only rows that could create nothing. Each affected list keeps its section header and that header's Insert command, and working prompts (sense and subsense gloss, a record's Text) stay.

Where to look

  • WalkSublayout: the Type choice (ResolveLayoutChoiceGuid) and in-place descent -- Compose_ASubrecord_ShowsItsFields_UnderItsHeader_ChosenByItsType, Compose_ASubrecordOfASubrecord_ShowsItsFieldsOneLevelFurtherIn.
  • The Summary case: a subrecord's header stands for its opening summary row and walks that row's children in its place (_itemSummaryHvo) -- Compose_ASubrecord_HasOneHeaderOfItsOwn_UnderTheSubrecordsSection.
  • AddGhostPrompt's early return -- Compose_AnEmptyListWithNoGhostField_HasNoPromptRow_UnlessTypingCanCreate, Compose_EmptyAllomorphSection_ShowsOnlyItsHeader_WithInsertOnItsMenu.
  • Edits land on the subrecord -- Edit_ASubrecordsTitle_UnderItsHeader_SavesToTheSubrecord.

Not here

  • Field Visibility and Move Field on a subrecord's rows target Normal without its Type choice, as on the top-level record.
  • About four nesting levels fit under the composer's depth cap; legacy has no cap.
  • Date of Event, Participants and the record-link fields stay unsupported.
  • LT-22844 reverses LT-22672: Compose the Allomorphs slice, and the shared detail-view fixes it surfaced #1135's watermarked row on an empty Allomorphs list; that row was never typeable.

Stack and verification. Based on #1201's branch; retarget to main once #1201 merges. Hygiene build clean; suites pass (counts in the preflight section). Checked in FLEx: subrecord fields, and header text in both states. Not yet: the LT-22844 checks in the preflight section.

Next: review WalkSublayout and the prompt rule; this leaves draft once the LT-22844 FLEx checks pass.


Reading this a year from now -- start here

LT-22858 is the subrecord half of the Notebook detail view in New mode: before it, each subrecord showed only a header with nothing under it. The subrecord header wording was first written for #1201 (LT-22689) and moved here, because in #1201's build a subrecord header had nothing under it and so could never collapse to show its collapsed line. LT-22844 (the dead "Type to add SubRecords" box) joined this branch during preflight, because composing subrecord layouts made that dead row appear inside every subrecord with no subrecords. No working documents were kept on the branch; the reasoning is below.

Decisions, and why
  • <sublayout> imports as its own node kind (ViewNodeKind.Sublayout), carrying the layout name in TargetLayout and the choice field in Field. Both properties already exist on every node, so the override applier, the JSON serializer and every node copy carry them without change.
  • The composer reuses the record's own layout choice. WalkSublayout resolves the choice with ResolveLayoutChoiceGuid and compiles with CompileForObject(..., choiceGuid, overrides), exactly as the shown record does, and walks the result at the current depth. The existing (object, layout) visited set stops runaway recursion.
  • The two per-compose memos keyed without the choice stay as they are. _itemMenuBindings and _itemRootLabelled only ever see sequence and atomic descents, which still compile with no choice; the comment over them now says so.
  • The subrecord header stands for its opening summary row. That row (HeavySummary) is the legacy item header, so the sequence's item header replaces it and its children (the <sublayout>) walk directly under the header rather than a level deeper.
  • LT-22844 drops only dead prompts. A prompt row is left out when its layout names no ghost field and nothing could be created from typing. Senses and subsenses keep their gloss prompt (the subsense one approved under LT-22690), and every authored ghost keeps its row.
Surprising findings
Evidence
Claim Test
The importer keeps <sublayout>; a nameless one drops with a warning SublayoutElement_ImportsAsANode_NamingItsLayoutAndChoiceField, SublayoutElement_WithoutAName_IsDroppedWithAWarning
Subrecord header lines Subrecord_ShowsItsNumberAndType_AndWhileCollapsed_ItsTitle
One header per subrecord Compose_ASubrecord_HasOneHeaderOfItsOwn_UnderTheSubrecordsSection
Fields chosen by Type, one level under the header Compose_ASubrecord_ShowsItsFields_UnderItsHeader_ChosenByItsType
Nesting Compose_ASubrecordOfASubrecord_ShowsItsFieldsOneLevelFurtherIn
Edits save to the subrecord Edit_ASubrecordsTitle_UnderItsHeader_SavesToTheSubrecord
No dead prompt rows; working ones stay Compose_AnEmptyListWithNoGhostField_HasNoPromptRow_UnlessTypingCanCreate, Compose_EmptyAllomorphSection_ShowsOnlyItsHeader_WithInsertOnItsMenu, Compose_ARecordWithNoSubrecords_KeepsTheSubrecordsHeader_WithNoPromptRow

Mutation checks: breaking the Type choice, the field indent, the importer, and the prompt rule each failed its tests.

Preflight review details

Code Review Summary

Branch: LT-22858-subrecord-fields
Base: LT-22689-no-senses-banner (PR #1201, at 15c93cc), which merges into main
Date: 2026-10-23
Review model: Claude Opus 5.5 (Claude Code)
Files changed: 8 against the base (2 commits, plus staged in-review changes); main is
one commit ahead of the base, so both branches want a rebase before merge

Overview

In Notebook > Record Edit in New mode, each subrecord showed only its header: the
SubRecord layout brings the subrecord's fields in with
<sublayout name="Normal" layoutChoiceField="Type"/>, which the layout importer dropped
as a publishing construct (LT-22858). The branch imports <sublayout> as a
ViewNodeKind.Sublayout node, and the composer lays out that layout for the subrecord's
Type in place, so the fields sit one level under the subrecord's header and nested
subrecords repeat the pattern.

It also carries the subrecord header wording moved out of #1201 ("1. Observation"
expanded, "1. Observation - title" collapsed), with the skip of the duplicate SubRecords
header. The collapsed text had looked wrong in #1201's build only because a header with
nothing under it cannot collapse; with the fields under it, it does, and it matches the
WinForms view.

During review the branch also took in LT-22844: an empty list whose layout names no ghost
field no longer shows a "Type to add ..." row that cannot be typed in, as in the WinForms
DataTree. Composing subrecord layouts had made that dead row appear inside every subrecord
with no subrecords.

Tickets: LT-22858 and LT-22844 (both user-visible, testable from an install; keys present
in the commits).

Contract/API Changes

  • Added ViewNodeKind.Sublayout (FwAvalonia public enum); TargetLayout names the layout
    and Field names the choice field.
  • <sublayout> in any imported layout now becomes a node instead of an Info diagnostic;
    one without a name drops with a Warning under the same sublayout-dropped code. All
    other shipped <sublayout> elements sit in jtview publishing layouts that no detail view
    embeds.
  • No serialized format, settings, resource-key, native or COM changes.

Findings

Critical - Must address before merge

None.

Important - Should address before merge

  • A dead "SubRecords" prompt row appeared inside every subrecord with no subrecords,
    now that subrecord layouts compose. (fixed during review: LT-22844 added -- AddGhostPrompt
    leaves out a prompt that names no ghost field and can create nothing; tests updated)

Minor - Consider

  • The revert brought back an unused using SIL.LCModel.Core.KernelInterfaces; in
    DetailSummaryLines.cs. (fixed during review: removed)
  • Field Visibility / Move Field on a subrecord's rows target the Normal layout
    without its Type choice
    (RecordEditView.TryLocateOverrideTarget compiles with no
    choice).
    (author: accepted; the top-level record already behaves this way, so this is
    not new; note it in the PR)
  • About four levels of subrecord nesting fit under the composer's depth cap of 12
    (each level uses about three indent levels; inferred from composed indents, not tested);
    the WinForms view has no limit.
    (author: accepted; note it in the PR)

Required Validation / Evidence

  • ./build.ps1 -BuildTests -SkipNative -CommentHygiene -TokenHygiene: clean after the
    in-review changes.
  • ./test.ps1 -NoBuild -TestProject xWorksTests -TestFilter 'FullyQualifiedName!~UiaSmokeTests':
    1844 total, 1842 passed, 2 skipped, with the LT-22844 change in place, after rebasing onto
    LT-22689: Drop the Senses banner and word item headers like legacy #1201's review follow-up.
  • Affected tests after restoring the LT-22844 rule: 64 of 64 passed (summary lines,
    allomorph section, prompt-row and Lexicon menu tests).
  • ./test.ps1 -NoBuild -TestProject FwAvaloniaTests: 937 total, 936 passed, 1 skipped,
    after the rebase.
  • Mutation checks: breaking the Type choice, the field indent, the importer, and the
    LT-22844 rule each failed their tests.
  • Manual, by the author in FLEx: subrecord fields under their headers, and the
    expanded and collapsed header text matching the WinForms view.
  • Manual, not yet done (LT-22844): an empty new notebook record and a subrecord with no
    subrecords show no "SubRecords" box; an entry with no allomorphs shows only the
    Allomorphs header with Insert Allomorph; a part of speech with empty sections shows only
    their headers; an entry with no senses and a sense with no subsenses still show their
    working gloss rows.

Positive Observations

  • The fix reuses the composer's existing pieces: the layout choice the root record already
    uses, in-place descent, and the (object, layout) recursion guard.
  • Every shipped detail list that loses its dead row keeps its section header, and that
    header carries the same Insert menu.
  • The working sense and subsense gloss rows, which name no ghost field but can create a
    sense, are kept and tested.

Interview Notes

  • Dead "SubRecords" row: the author chose to fix LT-22844 on this branch.
  • LT-22844 reverses a choice from LT-22672 (LT-22672: Compose the Allomorphs slice, and the shared detail-view fixes it surfaced #1135), whose test said an empty Allomorphs
    list "offers the watermarked add row rather than nothing". That row was never typeable;
    the WinForms view shows only the Allomorphs header. Both tests that asserted the dead row
    were rewritten.
  • Minors: unused using fixed; override targeting and nesting depth accepted, to be noted
    in the PR.
  • Author does not understand: how a subrecord's fields are chosen by its Type and placed
    one level under its header (WalkSublayout with ResolveLayoutChoiceGuid, and the
    Summary case walking the skipped row's children at the header's child depth).
  • Nothing else flagged.

In-Review Quality Check

  • DetailComposer.AddGhostPrompt: returns before adding a row when the layout names no
    ghost field and ResolveGhostCreation found nothing to create.
  • DetailSummaryLines.cs: unused using removed.
  • Tests: Compose_EmptyAllomorphSection_ShowsGhostAddRow became
    Compose_EmptyAllomorphSection_ShowsOnlyItsHeader_WithInsertOnItsMenu;
    Compose_GhostWithoutGhostField_IsNotEditable_SoTypingCannotCreateAndDiscard became
    Compose_AnEmptyListWithNoGhostField_HasNoPromptRow_UnlessTypingCanCreate; added
    Compose_ARecordWithNoSubrecords_KeepsTheSubrecordsHeader_WithNoPromptRow.
  • Committed as an LT-22844 commit and a small cleanup commit, after rebasing onto LT-22689: Drop the Senses banner and word item headers like legacy #1201's
    review follow-up (15c93cc). Hygiene build and the test runs above are on that tree.

Suggested Review Focus

  • DetailComposer.WalkSublayout, and the Summary case walking the skipped summary
    row's children at the header's child depth.
  • AddGhostPrompt's new early return, against the lists it removes rows from in the
    four New-mode tools (Lexicon Edit, its popup, Notebook Record Edit, Grammar's
    part-of-speech editor).
  • The importer's <sublayout> handling and its effect on the shipped-layout coverage
    report.

🤖 Generated with Claude Code


This change is Reviewable

Zachary Burnham and others added 4 commits October 23, 2026 18:54
A subrecord header reads "1. Observation" while expanded and
"1. Observation - title" while collapsed, as the record's Summary and
SummaryCollapsed parts define, and stands for the summary row the
subrecord's layout opens with, so no second SubRecords header follows.

This reverts commit 01de10d.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The SubRecord layout brings a subrecord's fields in with
<sublayout name="Normal" layoutChoiceField="Type"/>, which the layout
importer dropped as a publishing construct, so each subrecord showed only
its header. The importer now keeps it as a Sublayout node, and the
composer lays out that layout for the subrecord's Type in place, the way
the shown record's own layout is chosen. The fields sit one level under
the subrecord's header, and nested subrecords repeat the pattern.

A subrecord header with fields under it can now collapse, which is what
shows its collapsed line.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
An empty list whose layout names no ghost field showed a "Type to add
..." row that could not be typed in, labelled with the raw field name:
the SubRecords list on a new notebook record, and now on every subrecord
with no subrecords. The WinForms DataTree shows a prompt only where the
layout names a ghost field, so those rows are left out. Each affected
list keeps its section header and that header's Insert command, and the
sense and subsense gloss prompts, which can create a sense, stay.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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