Skip to content

LT-22689: Drop the Senses banner and word item headers like legacy - #1201

Open
thejambi wants to merge 5 commits into
mainfrom
LT-22689-no-senses-banner
Open

thejambi wants to merge 5 commits into
mainfrom
LT-22689-no-senses-banner

Conversation

@thejambi

@thejambi thejambi commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Start here: Src/xWorks/Avalonia/Composer/DetailSummaryLines.cs -- every header line, one builder per class, each following that class's Summary and SummaryCollapsed parts.

Follow-up to #1184, from the tester's LT-22689 comment. In Lexicon Edit's Avalonia detail view, each sense is now its own section, with no "Senses" header over them, and item headers read as in the legacy view:

Item Expanded Collapsed
Sense Sense 1, Sense 1.1 Sense 1 - house - n (definition when there is one, else gloss; then grammatical info)
Etymology Etymology Etymology - language notes, gloss
Extended Note Extended Note Extended Note - type - discussion

The Etymologies and Extended Notes rows over their items are gone too; the legacy view has neither.

Does dropping those rows lose anything? One menu item: the Extended Notes row's Insert Extended Note. It stays on the sense's menu and each note's hotlinks, as in legacy, and both paths are tested. The question worth your time is whether each line matches its part.

Where to look

  • The grammatical-info branches per MSA kind -- the five GrammaticalInfo_* tests.
  • Indentation: summarized items sit at the list's level, subsenses under their sense -- Compose_Senses_HaveNoBanner_AndSubsensesIndentUnderTheirSense.
  • The header swapping its text and automation name on toggle, and wrapping a long line with the +/- box beside its first line -- CollapsibleHeader_ShowsItsCollapsedLine_OnlyWhileCollapsed, LongHeaderLine_WrapsWithinThePane_WithTheBoxBesideItsFirstLine.
  • Etymology and extended note headers (NormalSummary) drawing no heavy top-level rule, as senses (HeavySummary) do -- NormalWeightHeader_DrawsNoHeavyRule_AtTheTopLevel.
  • The menu tests rewritten for the removed rows -- InsertExtendedNote_FromTheSenseMenu_GoesLast.

Not here

  • Notebook subrecord headers keep their current wording; they move to LT-22858 with the subrecord fields.
  • A name read from outside the entry (part-of-speech abbreviation, note type) refreshes only when the entry is shown again -- the existing New-mode refresh rule (IsChangeWithinEntry).

Stack and verification. Merges into main, no dependency. Hygiene build clean; FwAvaloniaTests and xWorksTests pass (counts in the preflight section), with the UIA smoke fixtures excluded (an unrelated bulk-edit UIA test intermittently crashes the test host locally). Checked in FLEx, both states: sense, subsense, etymology and extended note headers, and both Insert Extended Note paths; a long collapsed definition wrapping to the pane.

Next: review the line mapping, or tell me to split the menu-test changes out.


Reading this a year from now -- start here

This PR is the second half of LT-22689. #1184 gave detail section headers a +/- box and remembered state; a tester then found two places where the Avalonia view still differed from the legacy one: a "Senses" header over all the senses, and sense headers worded "1 gloss" in both states. This branch fixes both and applies the same header wording to etymologies and extended notes. Notebook subrecord headers were in an earlier revision of this branch and were moved to LT-22858, because their collapsed text did not yet match legacy in testing. No working documents were kept on the branch; the reasoning is below.

Decisions, and why
  • The lines are built in C#, one builder per class, following the shipped parts (LexSense-Jt-Summary/SummaryCollapsed and LexExtendedNote-Jt-SummaryCollapsed in LexSenseParts.xml, the etymology parts in LexEntryParts.xml, and the MSA Basic parts in MorphologyParts.xml). Each <lit> goes through StringTable.Table.LocalizeLiteralValue, as the legacy XmlVc does, so translated literals carry over. A project that overrides those parts is not followed.
  • Summarized items sit at the sequence's own indent, as legacy sense headers sit at the entry's level; subsenses keep one extra level under their sense.
  • The collapsed line is carried on the field (DetailField.CollapsedLabel), and DetailSectionHeader swaps text and automation name as it toggles, so a toggle stays incremental and does not recompose the view. The text wraps to the pane width, as the WinForms summary does; a small panel keeps the +/- box beside the first line.
  • Headers carry their layout's weight (DetailField.IsNormalWeight): etymologies and extended notes open with a NormalSummary row, senses with a HeavySummary one, so only senses draw the heavy top-level rule. The WinForms view's thinner normal-weight line is not drawn; New mode has no rule style for it.
  • Notebook subrecords are left as they are on main. Their header wording, and the skip of the duplicate SubRecords header each subrecord's layout opens with, go to LT-22858, which also adds the subrecord's fields under that header.
Surprising findings
  • In legacy, a <seq>'s menu attribute applies only to its ghost (prompt) row. That is why the Extended Notes row's menu has no legacy counterpart once a sense has notes: legacy shows no row there at all, and Insert Extended Note lives on mnuDataTree-Sense and mnuDataTree-ExtendedNote-Hotlinks.
  • A sense with no grammatical info still ends its collapsed line with -: the legacy layout writes the separator unconditionally, and the legacy view shows it.
Evidence
Claim Test
Sense lines, definition preferred over gloss Sense_ShowsItsNumber_AndWhileCollapsed_TheGlossAndPartOfSpeech, Sense_WhileCollapsed_ShowsTheDefinitionInPlaceOfTheGloss
Hierarchical numbers in composed headers Compose_SenseHeaders_UseHierarchicalNumbers_AndTheGlossWhileCollapsed
No banners over etymologies and extended notes Compose_EtymologiesAndExtendedNotes_HaveNoBanner_AndCarryTheirSummaryLines
Insert Extended Note still reachable InsertExtendedNote_FromTheSenseMenu_GoesLast, InsertExtendedNote_FromTheFirstNotesHotlinks_GoesRightAfterIt
Header text swaps with state CollapsibleHeader_ShowsItsCollapsedLine_OnlyWhileCollapsed
A long header line wraps, box beside its first line LongHeaderLine_WrapsWithinThePane_WithTheBoxBesideItsFirstLine
Normal-weight headers draw no heavy rule NormalWeightHeader_DrawsNoHeavyRule_AtTheTopLevel, Compose_EtymologiesAndExtendedNotes_HaveNoBanner_AndCarryTheirSummaryLines

Mutation checks: disabling the header's text swap, the text wrapping, and the weight check each failed its test.

The menu execution tests' helper now targets the hidden command adapter for a menu the native authorities do not fully own, as RecordEditView does before building such a menu; without it, a sense-menu insert read as disabled in the test while it works in the product.

Preflight review details

Code Review Summary

Branch: LT-22689-no-senses-banner
Base: main (merge base d0d85cb)
Date: 2026-10-23
Review model: Claude Opus 5.5 (Claude Code)
Files changed: 10 (4 commits: two feature commits, one in-review comment fix, one
scope change taking Notebook subrecords out)

Overview

Follow-up to PR #1184 (LT-22689) from the tester's Jira comment: the Avalonia detail view
put one "Senses" section header over all of an entry's senses, and worded sense headers
as "1 gloss" in both states. The WinForms DataTree has no such banner (each sense header
is its own section) and words the header "Sense 1" while expanded and
"Sense 1 - gloss - n" while collapsed.

The branch removes the banner and gives sense, etymology and extended note headers the
expanded and collapsed lines that each class's Summary and SummaryCollapsed parts define.
A new DetailSummaryLines builds the lines, including the grammatical info of each MSA
kind; DetailSectionHeader swaps between the two lines as it toggles. The Etymologies and
Extended Notes rows over their items are removed too, as in the WinForms view.

Notebook subrecord headers were in scope until manual testing showed their collapsed text
did not match the WinForms view; at the author's direction they were taken out of this
branch and moved to LT-22858. Subrecords now compose as on main.

Ticket: LT-22689 (user-visible, testable from an install; key present).

Contract/API Changes

  • Added public DetailField.CollapsedLabel { get; set; } (FwAvalonia).
  • Changed public DetailSectionHeader constructor from (label, isExpanded, canToggle) to
    (label, collapsedLabel, isExpanded, canToggle); its only caller is DataTree.
  • Added public DetailSectionHeader.CurrentLabel.
  • No serialized format, settings, resource-key, native or COM changes.

Findings

Critical - Must address before merge

None.

Important - Should address before merge

  • Notebook subrecord headers' collapsed text did not match the WinForms view in
    manual testing.
    (fixed during review: subrecords taken out of DetailSummaryLines,
    and the item-summary-row skip that only subrecords used was removed; both move to
    LT-22858)

Minor - Consider

  • DetailComposer.cs Summary case comment used "legacy", which AGENTS.md bans in
    code comments. (fixed during review: reworded; the later scope change then restored
    that line to main's text, so the branch no longer touches it)
  • Collapsed header text can go stale when a referenced object outside the entry
    changes
    : the part-of-speech abbreviation and extended note type are read at compose
    time, and the detail view only recomposes for changes inside the shown record
    (RecordEditView.IsChangeWithinEntry).
    (author: accepted; every other New-mode row
    that shows another object's name follows the same refresh rule; note it in the PR)

Required Validation / Evidence

  • ./build.ps1 -BuildTests -SkipNative -CommentHygiene -TokenHygiene: clean after the
    scope change.
  • ./test.ps1 -NoBuild -TestProject FwAvaloniaTests: 936 total, 935 passed, 1 skipped
    (after the review follow-up).
  • ./test.ps1 -NoBuild -TestProject xWorksTests -TestFilter 'FullyQualifiedName!~UiaSmokeTests':
    1838 total, 1836 passed, 2 skipped. The UIA smoke fixtures were excluded because
    BulkEditBarUiaSmokeTests.FilterBar_RealizedWindow_ExposesTargetCombosThroughUIA
    intermittently crashes the test host on this machine; it passed alone on retry and does
    not touch the detail view.
  • Mutation check (earlier in the work): disabling the header's text swap failed its
    test.
  • Manual, by the author in FLEx, both states: sense, subsense, etymology and extended
    note headers; Insert Extended Note from the sense menu and from a note's hotlinks.

Positive Observations

  • Each header line follows the shipped Summary/SummaryCollapsed parts, with <lit> text
    localized through StringTable.LocalizeLiteralValue as the WinForms view does.
  • Insert Extended Note stays reachable from the sense menu and from each note's hotlinks,
    and both paths are tested.
  • The header keeps its automation name in step with the shown line.
  • The menu execution test helper now targets the command adapter for menus the native
    authorities do not fully own, as the host does.

Interview Notes

  • Wording: the author confirmed in FLEx that the sense lines match the WinForms view,
    including the trailing " - " when a sense has no grammatical info.
  • Extended Notes row: after a before/after explanation, the author confirmed removing the
    row is intended.
  • Stale referenced names: accepted as the existing New-mode refresh rule; noted in the PR.
  • Notebook subrecords: collapsed headers did not match the WinForms view; the author chose
    to defer Notebook headers to LT-22858 rather than diagnose them here.
  • Author does not understand: how a summarized item's header replaced its opening summary
    row. That mechanism (_itemSummaryHvo) was removed from this branch with the Notebook
    scope change, so it no longer applies here; it returns with LT-22858.
  • Nothing else flagged.

In-Review Quality Check

  • Comment reworded in DetailComposer.cs (commit 69ad548).
  • Scope change: DetailSummaryLines no longer covers notebook records; the
    _itemSummaryHvo skip, its comments and the two subrecord tests were removed. Hygiene
    build and both suites rerun clean.
  • Review follow-up (mark-sil, commit 15c93cc): header text wraps to the pane width with
    the +/- box beside its first line, and normal-weight headers draw no heavy rule. Both
    threads answered and resolved; the author checked long-definition wrapping in FLEx.

Suggested Review Focus

  • DetailSummaryLines: each line against its Summary/SummaryCollapsed part, especially
    the MSA grammatical-info branches.
  • DetailComposer.WalkSequence: item depth for summarized items (subsenses still
    indent under their sense) and the banner suppression.
  • Removed section rows (Senses, Etymologies, Extended Notes) and the menu tests updated
    for them.

🤖 Generated with Claude Code


This change is Reviewable

Zachary Burnham and others added 3 commits October 21, 2026 11:06
The WinForms DataTree shows no "Senses" row: each sense's own header is
the section, at the entry's level. The detail composer added a
collapsible "Senses" banner above them and indented every sense one
level under it.

The composer now emits no banner for any Senses sequence. An entry's
sense headers sit at the entry's own depth, and subsenses still indent
under their sense.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sense, etymology, extended note and notebook subrecord headers now show
the text the WinForms DataTree builds from each class's Summary and
SummaryCollapsed layouts. A sense reads "Sense 1" while expanded and
"Sense 1 - house - n" while collapsed, with the definition in place of
the gloss when there is one, and the grammatical info of its MSA.

A collapsible header takes an optional collapsed label and swaps between
the two as it toggles, keeping its automation name in step.

The Etymologies and Extended Notes rows over those items are gone, as is
the second SubRecords header under each subrecord; the WinForms view has
none of them. Insert Extended Note stays on the sense's menu and on each
note's hotlinks.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

NUnit Tests

    1 files  ± 0      1 suites  ±0   11m 4s ⏱️ + 2m 30s
6 832 tests +16  6 747 ✅ +16  85 💤 ±0  0 ❌ ±0 
6 841 runs  +16  6 756 ✅ +16  85 💤 ±0  0 ❌ ±0 

Results for commit 15c93cc. ± Comparison against base commit d0d85cb.

This pull request removes 5 and adds 21 tests. Note that renamed tests count towards both.
FwAvaloniaTests.LayoutImportCoverageTests ‑ ShippedLayouts_UnresolvedParts_AreExactlyTheLegacyUnresolvableSet
SIL.FieldWorks.FdoUi.DetailObjectInsertionTests ‑ IsOfferedOnEntry_AStemOrAffixAllomorph_FollowsTheFirstAllomorph
SIL.FieldWorks.XWorks.DetailComposerTests ‑ Compose_SenseHeaders_UseHierarchicalNumbering_WithGlossSummary
SIL.FieldWorks.XWorks.LexiconMenuAuthorityTests ‑ AllomorphInserts_AreOfferedByTheEntrysMorphTypeAndFirstAllomorph
SIL.FieldWorks.XWorks.LexiconMenuExecutionTests ‑ InsertExtendedNote_FromTheSectionRow_GoesLast
FwAvaloniaTests.DetailViewingParityTests ‑ CollapsibleHeader_ShowsItsCollapsedLine_OnlyWhileCollapsed
FwAvaloniaTests.DetailViewingParityTests ‑ LongHeaderLine_WrapsWithinThePane_WithTheBoxBesideItsFirstLine
FwAvaloniaTests.DetailViewingParityTests ‑ NormalWeightHeader_DrawsNoHeavyRule_AtTheTopLevel
FwAvaloniaTests.LayoutImportCoverageTests ‑ ShippedLayouts_UnresolvedParts_AreExactlyTheRefsWithNoConfigurationPart
SIL.FieldWorks.FdoUi.DetailObjectInsertionTests ‑ IsOfferedOnEntry_AnyOtherInsert_OnAnyEntry
SIL.FieldWorks.XWorks.DetailComposerTests ‑ Compose_SenseHeaders_UseHierarchicalNumbers_AndTheGlossWhileCollapsed
SIL.FieldWorks.XWorks.DetailComposerTests ‑ Compose_Senses_HaveNoBanner_AndSubsensesIndentUnderTheirSense
SIL.FieldWorks.XWorks.DetailSummaryLinesTests ‑ Compose_EtymologiesAndExtendedNotes_HaveNoBanner_AndCarryTheirSummaryLines
SIL.FieldWorks.XWorks.DetailSummaryLinesTests ‑ Etymology_WhileCollapsed_ShowsTheGloss_AndAnyLanguageNotesBeforeIt
SIL.FieldWorks.XWorks.DetailSummaryLinesTests ‑ ExtendedNote_WhileCollapsed_ShowsItsTypeAndDiscussion
…

♻️ This comment has been updated with latest results.

@codecov-commenter

codecov-commenter commented Oct 23, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 89.38053% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 39.91%. Comparing base (d0d85cb) to head (15c93cc).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
Src/xWorks/Avalonia/Composer/DetailSummaryLines.cs 85.00% 3 Missing and 6 partials ⚠️
...rc/Common/FwAvalonia/Detail/DetailSectionHeader.cs 94.28% 0 Missing and 2 partials ⚠️
Src/Common/FwAvalonia/Detail/DataTree.cs 75.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1201      +/-   ##
==========================================
+ Coverage   39.88%   39.91%   +0.02%     
==========================================
  Files        1542     1543       +1     
  Lines      355361   355395      +34     
  Branches    41320    41337      +17     
==========================================
+ Hits       141753   141861     +108     
+ Misses     184253   184182      -71     
+ Partials    29355    29352       -3     
Files with missing lines Coverage Δ
Src/Common/FwAvalonia/Detail/DetailModel.cs 78.84% <ø> (ø)
Src/xWorks/Avalonia/Composer/DetailComposer.cs 71.12% <100.00%> (+0.03%) ⬆️
Src/Common/FwAvalonia/Detail/DataTree.cs 93.06% <75.00%> (ø)
...rc/Common/FwAvalonia/Detail/DetailSectionHeader.cs 96.39% <94.28%> (-1.28%) ⬇️
Src/xWorks/Avalonia/Composer/DetailSummaryLines.cs 85.00% <85.00%> (ø)

... and 20 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.

Their collapsed header text did not match the WinForms view in testing,
so subrecords go back to composing as they do on main. The subrecord
summary lines, and the skip of the opening summary row that only
subrecords needed, move to LT-22858 with the subrecord fields.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@thejambi
thejambi marked this pull request as ready for review October 23, 2026 18:51
Comment thread Src/Common/FwAvalonia/Detail/DetailSectionHeader.cs
Comment thread Src/xWorks/Avalonia/Composer/DetailComposer.cs
A collapsed sense header with a sentence-long definition ran past the
right edge of the pane and was lost, since the header text could not
wrap and the detail view does not scroll sideways. Header text now wraps
to the pane width, as the WinForms summary does, with the +/- box beside
its first line.

Etymologies and extended notes open with a NormalSummary row, not a
HeavySummary one, so their headers no longer draw the heavy top-level
section rule that senses keep.

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.

3 participants