From 1a0e95aa1642c04b18ea383dac3e50669dc3fdc2 Mon Sep 17 00:00:00 2001 From: Zachary Burnham Date: Tue, 13 Oct 2026 12:13:16 -0400 Subject: [PATCH 1/4] LT-22689: Give detail section headers a +/- box and lasting state Collapsible section headers in the Avalonia detail view show a +/- box, drawn from new tokens, in place of a text triangle. Header text and the "Field Options" link take the slice label size, and header text takes a colour token of its own instead of the button theme's link blue. A toggle inserts or removes only the rows under the header. The header itself stays, so it keeps its focus, its place in the view, and the second click of a double-click. On a 311-row test view a toggle went from about 2.7 s to under 0.1 s. Expansion state is keyed per section by the object's GUID, not its per-session HVO, so it survives restarts and sense reorders. The WinForms DataTree keys by object alone, which links an entry's sections together on return; that is not copied. Inserting into a collapsed section expands it, and headers expose the UI Automation ExpandCollapse pattern. The LT-22688 tab order is unchanged: Tab still skips headers. Co-Authored-By: Claude Opus 5.5 --- Src/Common/FwAvalonia/Detail/DataTree.cs | 252 +++++++++----- Src/Common/FwAvalonia/Detail/DetailModel.cs | 13 + .../FwAvalonia/Detail/DetailSectionHeader.cs | 180 ++++++++++ .../FwAvalonia/Detail/DetailVisibility.cs | 60 +++- Src/Common/FwAvalonia/DetailHostControl.cs | 18 +- Src/Common/FwAvalonia/FwAvaloniaDensity.cs | 37 +++ .../DetailViewingParityTests.cs | 313 ++++++++++++++++++ .../FwAvaloniaTests/DetailVisibilityTests.cs | 70 ++++ .../FwAvaloniaTests/ViewDefinitionTests.cs | 23 ++ .../Tokens/DataTree/DataTreeTokens.axaml | 14 + .../Tokens/FwColorTokens.axaml | 17 + .../Avalonia/Composer/DetailComposer.cs | 17 +- .../Hosting/RecordEditView.Avalonia.cs | 25 +- .../Composer/DetailExpansionComposeTests.cs | 94 ++++++ .../DetailCommandAdapterHardeningTests.cs | 16 + 15 files changed, 1030 insertions(+), 119 deletions(-) create mode 100644 Src/Common/FwAvalonia/Detail/DetailSectionHeader.cs create mode 100644 Src/xWorks/xWorksTests/Avalonia/Composer/DetailExpansionComposeTests.cs diff --git a/Src/Common/FwAvalonia/Detail/DataTree.cs b/Src/Common/FwAvalonia/Detail/DataTree.cs index 1e9d05085b..98e628872b 100644 --- a/Src/Common/FwAvalonia/Detail/DataTree.cs +++ b/Src/Common/FwAvalonia/Detail/DataTree.cs @@ -40,35 +40,50 @@ public sealed class DataTree : UserControl, IDetailPopupSink, IDisposable private readonly Action _labelColumnWidthChanged; private TextBlock _validationBlock; - // Fields render into this Form's Items, rebuilt each toggle from the model's - // visible-field - // subsequence -- a virtualizing panel only realizes on-screen containers, so visibility - // can't be cached. + // Each shown field's row is one of this Form's Items, in field order. A collapse or + // expand removes or inserts only the rows it hides or shows. private readonly Form _form; - // Session-recorded expansion overrides, keyed by header stable id; a toggle writes here - // (and - // through _expansionChanged for persistence) before RebuildItems() re-reads it. + // Session-recorded expansion overrides, keyed by DetailField.ExpansionKey; a toggle + // writes here (and through _expansionChanged for persistence) before ApplyVisibility() + // re-reads it. private readonly Dictionary _expansionState = new Dictionary(); + // The built row of each field, by field index; null while a collapsed header hides it. + private readonly ShownRow[] _rows; + + // The indices of the collapsible headers that own at least one row. + private readonly HashSet _toggleableHeaders = new HashSet(); + private readonly Func _getExpansionState; private readonly Action _expansionChanged; private readonly Action _menuRequested; private readonly Action _linkRequested; - // The vector rows of the shown fields, and the one whose item is current (at most one - // per view), so re-show continuity can read and restore that selection cheaply. - private readonly List _vectors = new List(); - // The editors the shown rows hold that attach handlers of their own, disposed when a - // rebuild replaces them or the view itself is disposed. - private readonly List _editors = new List(); private bool _disposed; + // The parts of one shown row the view itself acts on after building it. + private sealed class ShownRow + { + public Control Item; + // An editor that attaches handlers of its own, disposed when its row is removed. + public IDisposable Editor; + public FwReferenceVectorField Vector; + // The label, whose wrap cap tracks the label column width, and the width beside it + // that the cap leaves out. + public TextBlock Label; + public double LabelReserved; + public DetailSectionHeader Header; + } + + private IEnumerable ShownRows => _rows.Where(row => row != null); + /// The reference-vector row that has a current item; null when none /// does. internal FwReferenceVectorField SelectedVector { get; private set; } - /// The reference-vector rows of the shown fields. - internal IReadOnlyList VectorRows => _vectors; + /// The reference-vector rows of the shown fields, in field order. + internal IReadOnlyList VectorRows + => ShownRows.Where(row => row.Vector != null).Select(row => row.Vector).ToList(); /// True when the rebuild that follows should give keyboard focus to the vector /// row's current item. @@ -84,11 +99,9 @@ public sealed class DataTree : UserControl, IDetailPopupSink, IDisposable // Computed once per view (not per field/row) from the widest WS abbreviation across the // whole model, so every multi-WS field's gutter lines up at the same adaptive width. private readonly double _wsAbbrevColumnWidth; - // The live label column width, and the labels whose wrap cap tracks it. Capping from - // the token instead would ignore both a host-persisted width and a splitter drag. + // The live label column width, which each shown row's label wrap cap tracks. Capping + // from the token instead would ignore both a host-persisted width and a splitter drag. private double _labelColumnWidth; - private readonly List<(TextBlock Label, double Reserved)> _labelBlocks = - new List<(TextBlock Label, double Reserved)>(); /// /// Optional expansion-state hooks: supplies the @@ -110,6 +123,9 @@ public DataTree(DetailModel model, IDetailEditContext editContext = null, Action labelColumnWidthChanged = null) { Model = model ?? throw new ArgumentNullException(nameof(model)); + _rows = new ShownRow[Model.Fields.Count]; + foreach (var range in DetailVisibility.GetCollapsibleRanges(Model.Fields)) + _toggleableHeaders.Add(range.HeaderIndex); _editContext = editContext; _writingSystemFocused = writingSystemFocused; _getExpansionState = getExpansionState; @@ -183,7 +199,7 @@ public DataTree(DetailModel model, IDetailEditContext editContext = null, ApplyLabelColumnWidth(w.Value, notifyHost: true); }; - RebuildItems(); + ApplyVisibility(); // Viewing parity (11.x): the whole detail view scrolls, like legacy DataTree's AutoScroll panel. // Equal row height read-only vs editable (layout parity): the field container is @@ -291,8 +307,12 @@ private void ApplyLabelColumnWidth(double columnWidth, bool notifyHost) // splitter, which is on top and swallows clicks on it. FormItem honors only an // absolute width. _form.LabelWidth = new GridLength(columnWidth + FwAvaloniaDensity.SplitterWidth); - foreach (var entry in _labelBlocks) - entry.Label.MaxWidth = Math.Max(0, columnWidth - entry.Reserved); + // Runs on every layout pass, so it walks the rows without allocating. + foreach (var row in _rows) + { + if (row?.Label != null) + row.Label.MaxWidth = Math.Max(0, columnWidth - row.LabelReserved); + } if (notifyHost) // Reports the label column itself, not the form's label+splitter span. _labelColumnWidthChanged?.Invoke(columnWidth); @@ -352,10 +372,8 @@ public void Dispose() if (_disposed) return; _disposed = true; - var editors = _editors.ToArray(); - _editors.Clear(); - foreach (var editor in editors) - editor.Dispose(); + foreach (var row in ShownRows) + row.Editor?.Dispose(); } /// @@ -377,7 +395,8 @@ public void Dispose() /// view's own completion, which is what discards the text, only an external refresh. /// public bool HasUnsubmittedText - => _editors.OfType().Any(editor => editor.HasUnstagedText); + => ShownRows.Select(row => row.Editor).OfType() + .Any(editor => editor.HasUnstagedText); /// /// Raised when the view goes idle again -- the click finished and no picker it @@ -425,7 +444,7 @@ private void OnCancel() _validationBlock.IsVisible = false; // Text that was typed but never offered goes with the cancelled session. Left in // the editor, the next focus change would stage the very edit just cancelled. - foreach (var vector in _vectors) + foreach (var vector in VectorRows) vector.DiscardUnstagedText(); _editContext.Cancel(); RaiseOrDeferEditCompleted(); @@ -440,6 +459,9 @@ private void OnCancel() // rebuild destroys the anchor and the picker vanishes mid-choice. private int _openPickers; private bool _editCompletedHeld; + // Set once this view commits or cancels an edit of its own. The re-show that follows + // replaces the view, so the flag never outlives that edit. + private bool _completedOwnEdit; /// /// Raises the completion the host re-shows on -- unless a pointer gesture is in flight, @@ -448,6 +470,7 @@ private void OnCancel() /// private void RaiseOrDeferEditCompleted() { + _completedOwnEdit = true; if (IsInteractionInFlight) { _editCompletedHeld = true; @@ -499,49 +522,114 @@ private void DeliverWhenIdle() InteractionCompleted?.Invoke(this, EventArgs.Empty); } - // Viewing parity (11.x): a header owns every more-indented row up to the next row at its - // own indent or shallower; a row stays visible only while every owning header is - // expanded. - private void RebuildItems() + /// + /// Brings the shown rows in line with the recorded expansion state: builds and inserts + /// each row that became visible, removes each row that became hidden, and sets each shown + /// header's +/- box. Every other row is left as it is, so a toggle costs only the rows + /// it shows or hides. A header owns every more-indented row up to the next row at its + /// own indent or shallower, and a row is visible only while every owning header is + /// expanded. + /// + private void ApplyVisibility() { - var replaced = _editors.ToArray(); - _editors.Clear(); - _form.Items.Clear(); + var visible = DetailVisibility.ComputeVisibility(Model.Fields, GetRecordedExpansion); + var removed = new List(); + var position = 0; + for (var i = 0; i < _rows.Length; i++) + { + var row = _rows[i]; + if (!visible[i]) + { + if (row != null) + { + _form.Items.RemoveAt(position); + _rows[i] = null; + removed.Add(row); + } + continue; + } + + if (row == null) + { + row = BuildRow(i, Model.Fields[i]); + _rows[i] = row; + _form.Items.Insert(position, row.Item); + } + else if (row.Header != null) + { + row.Header.IsExpanded = IsExpanded(Model.Fields[i]); + } + position++; + } + // Only once the form has let go of them, so a focus loss their removal raises still // reaches their own handlers and stages what they hold. - foreach (var editor in replaced) - editor.Dispose(); - _vectors.Clear(); - _labelBlocks.Clear(); - SelectedVector = null; - var visible = DetailVisibility.ComputeVisibility(Model.Fields, GetRecordedExpansion); - for (var i = 0; i < Model.Fields.Count; i++) + foreach (var row in removed) { - if (visible[i]) - _form.Items.Add(BuildItem(i, Model.Fields[i])); + row.Editor?.Dispose(); + if (row.Vector != null && ReferenceEquals(SelectedVector, row.Vector)) + SelectedVector = null; } } // A header's recorded expansion state prefers this session's own toggles over the // host-supplied persisted state, so a toggle applies immediately rather than // waiting on the host's round-trip. - private bool? GetRecordedExpansion(string stableId) => - _expansionState.TryGetValue(stableId, out var v) ? (bool?)v : _getExpansionState?.Invoke(stableId); + private bool? GetRecordedExpansion(string expansionKey) => + _expansionState.TryGetValue(expansionKey, out var v) ? (bool?)v : _getExpansionState?.Invoke(expansionKey); - private bool IsExpanded(DetailField field) => GetRecordedExpansion(field.StableId) ?? field.IsInitiallyExpanded; + private bool IsExpanded(DetailField field) => GetRecordedExpansion(field.ExpansionKey) ?? field.IsInitiallyExpanded; + + private void RecordExpansion(DetailField header, bool expanded) + { + _expansionState[header.ExpansionKey] = expanded; + _expansionChanged?.Invoke(header.ExpansionKey, expanded); + } - // Wraps one field's content as a Form item: FormItem.Label carries the label cell - // (value-side - // only fields) or the item goes NoLabel and spans full width (headers). - private Control BuildItem(int index, DetailField field) + // The header itself stays, so it keeps its focus, its place in the view, and any + // click that follows this one. + private void ToggleSection(DetailField field, DetailSectionHeader header) { - var fieldContent = AddField(index, field); + if (!header.CanToggle) + return; + RecordExpansion(field, !IsExpanded(field)); + ApplyVisibility(); + } + + /// + /// Expands every collapsed section that hides a row of an object absent from the view + /// , recording each through the expansion hooks, so an object + /// just inserted into a collapsed section is shown. Does nothing when + /// is null, or completed an edit of its own: an object that + /// edit created was typed where the user could see it, so a section hiding it now was + /// collapsed on purpose. + /// + public void RevealObjectsAddedSince(DataTree previous) + { + if (previous == null || previous._completedOwnEdit) + return; + var headers = DetailVisibility.GetHeadersHidingNewObjects(previous.Model.Fields, Model.Fields, + GetRecordedExpansion); + if (headers.Count == 0) + return; + foreach (var header in headers) + RecordExpansion(header, true); + ApplyVisibility(); + } + + // Builds one field's row. Its Form item carries the label cell in FormItem.Label + // (value-side only fields), or goes NoLabel and spans full width (headers). + private ShownRow BuildRow(int index, DetailField field) + { + var row = new ShownRow(); + var fieldContent = AddField(index, field, row); var content = ApplyRule(fieldContent.Content, index); if (fieldContent.Label != null) FormItem.SetLabel(content, fieldContent.Label); else FormItem.SetNoLabel(content, true); - return content; + row.Item = content; + return row; } // The 1px inter-slice rule renders as a per-item bottom border; the last field @@ -569,7 +657,7 @@ private struct FieldContent public Control Label; } - private FieldContent AddField(int row, DetailField field) + private FieldContent AddField(int row, DetailField field, ShownRow shown) { var automationId = string.IsNullOrEmpty(field.AutomationId) ? field.StableId : field.AutomationId; var indent = new Thickness(field.Indent * 12, 0, 0, 0); @@ -579,52 +667,28 @@ private FieldContent AddField(int row, DetailField field) // the legacy tree's section rows. if (field.Kind == DetailFieldKind.Header) { + var headerLabel = field.Label ?? field.Field ?? string.Empty; Control header; if (field.IsCollapsible) { - // Legacy SliceTreeNode +/- box equivalent: the header toggles its nested rows. - var button = new Button - { - Content = (IsExpanded(field) ? "\u25bc " : "\u25b6 ") + (field.Label ?? field.Field ?? string.Empty), - FontWeight = FontWeight.Bold, - Background = FwAvaloniaDensity.TransparentBrush, - BorderThickness = new Thickness(0), - Padding = new Thickness(0), - Margin = new Thickness(indent.Left, 4, 0, FwAvaloniaDensity.FieldSpacing), - // Semi's Button ControlTheme centres by default; a section header hugs - // the - // left edge like the legacy row. - HorizontalAlignment = HorizontalAlignment.Left, - HorizontalContentAlignment = HorizontalAlignment.Left - }; - // A toggle flips the recorded state and rebuilds the Form's Items from the - // new - // visible-field subsequence -- there is no realized row to show/hide - // directly. - button.Click += (s, e) => - { - var next = !IsExpanded(field); - _expansionState[field.StableId] = next; - _expansionChanged?.Invoke(field.StableId, next); - RebuildItems(); - }; + var button = new DetailSectionHeader(headerLabel, IsExpanded(field), + _toggleableHeaders.Contains(row)); + button.Click += (s, e) => ToggleSection(field, button); + shown.Header = button; header = button; // Chrome, not a field, same reasoning as the field-menu kebab (LT-22688). Avalonia.Input.KeyboardNavigation.SetIsTabStop(button, false); } else { - header = new TextBlock - { - Text = field.Label ?? field.Field ?? string.Empty, - FontWeight = FontWeight.Bold, - Margin = new Thickness(indent.Left, 4, 0, FwAvaloniaDensity.FieldSpacing), - // A null background only hit-tests the glyphs; the whole header area - // must take the right-click. - Background = FwAvaloniaDensity.TransparentBrush - }; + var label = DetailSectionHeader.CreateLabel(headerLabel); + // A null background only hit-tests the glyphs; the whole header area must + // take the right-click. + label.Background = FwAvaloniaDensity.TransparentBrush; + header = label; } + header.Margin = new Thickness(indent.Left, 4, 0, FwAvaloniaDensity.FieldSpacing); AutomationProperties.SetAutomationId(header, automationId); AutomationProperties.SetName(header, field.Label ?? string.Empty); // The header answers right-click with its slice menu; the hover @@ -703,13 +767,14 @@ private FieldContent AddField(int row, DetailField field) // take the right-click for the slice menu. Background = FwAvaloniaDensity.TransparentBrush }; - _labelBlocks.Add((labelBlock, labelReserved)); + shown.Label = labelBlock; + shown.LabelReserved = labelReserved; AutomationProperties.SetAutomationId(labelBlock, automationId + ".Label"); AutomationProperties.SetName(labelBlock, field.Label ?? field.Field ?? string.Empty); ToolTip.SetTip(labelBlock, field.Label ?? field.Field); // the label text is its own tip var editor = CreateEditor(field, automationId); if (editor is IDisposable disposable) - _editors.Add(disposable); + shown.Editor = disposable; editor.Margin = new Thickness(0, 0, 0, FwAvaloniaDensity.FieldSpacing); // Every reachable control in a row shares its TabIndex, so Avalonia visits them in @@ -718,7 +783,7 @@ private FieldContent AddField(int row, DetailField field) ApplyRowTabIndex(editor, row); if (editor is FwReferenceVectorField vector) { - _vectors.Add(vector); + shown.Vector = vector; vector.SelectionChanged += OnVectorSelectionChanged; } @@ -898,6 +963,9 @@ private Control CreateHotlinkStrip(DetailField field, string automationId, Thick { Content = FwAvaloniaStrings.FieldOptionsMenu, Foreground = HotlinkBrush, + // Set here, so the button theme's larger semibold text does not apply. + FontSize = FwAvaloniaDensity.HotlinkFontSize, + FontWeight = FontWeight.Normal, Background = FwAvaloniaDensity.TransparentBrush, BorderThickness = new Thickness(0), Padding = new Thickness(0), diff --git a/Src/Common/FwAvalonia/Detail/DetailModel.cs b/Src/Common/FwAvalonia/Detail/DetailModel.cs index 464b9238fd..e97190635b 100644 --- a/Src/Common/FwAvalonia/Detail/DetailModel.cs +++ b/Src/Common/FwAvalonia/Detail/DetailModel.cs @@ -1682,6 +1682,19 @@ public DetailField( /// Initial expansion state of a collapsible header (from the layout's expansion attr). public bool IsInitiallyExpanded { get; } + private string _expansionKey; + + /// + /// The key a collapsible header's expansion state is recorded and persisted under; + /// when none is set. Settable so a row can carry a key that names + /// its object by guid, which still identifies that object after the project is reopened. + /// + public string ExpansionKey + { + get => _expansionKey ?? StableId; + set => _expansionKey = value; + } + /// Legacy slice menu id (layout `menu=`) for right-click on the row/label (13.x). public string MenuId { get; } diff --git a/Src/Common/FwAvalonia/Detail/DetailSectionHeader.cs b/Src/Common/FwAvalonia/Detail/DetailSectionHeader.cs new file mode 100644 index 0000000000..9e5bef2133 --- /dev/null +++ b/Src/Common/FwAvalonia/Detail/DetailSectionHeader.cs @@ -0,0 +1,180 @@ +// Copyright (c) 2026 SIL International +// This software is licensed under the LGPL, version 2.1 or later +// (http://www.gnu.org/licenses/lgpl-2.1.html) + +using System; +using Avalonia; +using Avalonia.Automation; +using Avalonia.Automation.Peers; +using Avalonia.Automation.Provider; +using Avalonia.Controls; +using Avalonia.Controls.Shapes; +using Avalonia.Layout; +using Avalonia.Media; + +namespace SIL.FieldWorks.Common.FwAvalonia.Detail +{ + /// + /// A collapsible section header: a flat button showing a +/- box before the section label. + /// The box shows minus while the section is expanded and plus while it is collapsed. A + /// header with no rows under it cannot toggle; it keeps the box's space but draws no box, + /// so its label lines up with its siblings'. The owner applies the toggle on Click, which + /// the ExpandCollapse automation pattern raises too, and then sets + /// . + /// + public sealed class DetailSectionHeader : Button + { + private DetailSectionHeaderAutomationPeer _peer; + private bool _isExpanded; + + /// Creates a header showing . + public DetailSectionHeader(string label, bool isExpanded, bool canToggle) + { + CanToggle = canToggle; + _isExpanded = isExpanded; + Box = new DetailExpanderBox(isExpanded) { Opacity = canToggle ? 1 : 0 }; + Background = FwAvaloniaDensity.TransparentBrush; + BorderThickness = new Thickness(0); + Padding = new Thickness(0); + // Semi's Button theme centres its content; a section header hugs the left edge. + HorizontalAlignment = HorizontalAlignment.Left; + HorizontalContentAlignment = HorizontalAlignment.Left; + var text = CreateLabel(label); + text.VerticalAlignment = VerticalAlignment.Center; + Content = new StackPanel + { + Orientation = Orientation.Horizontal, + Spacing = FwAvaloniaDensity.TreeBoxGap, + Children = { Box, text } + }; + } + + /// + /// The section header label text, for a header that cannot collapse as well as inside + /// this one. Its size and colour are its own, so neither the detail view's TextBlock + /// style nor the button theme decides them. + /// + public static TextBlock CreateLabel(string label) => new TextBlock + { + Text = label ?? string.Empty, + FontSize = FwAvaloniaDensity.SectionHeaderFontSize, + Foreground = FwAvaloniaDensity.SectionHeaderBrush, + FontWeight = FontWeight.Bold + }; + + /// Themed as a plain . + protected override Type StyleKeyOverride => typeof(Button); + + /// False when the header has no rows to show or hide. + public bool CanToggle { get; } + + /// + /// Whether the rows under the header are shown. Setting it redraws the +/- box and tells + /// assistive technology the state changed; it does not show or hide any rows. + /// + public bool IsExpanded + { + get => _isExpanded; + set + { + if (_isExpanded == value) + return; + var old = ExpandCollapseState; + _isExpanded = value; + Box.IsExpanded = value; + _peer?.RaiseExpandCollapseStateChanged(old, ExpandCollapseState); + } + } + + /// The +/- box. + public DetailExpanderBox Box { get; } + + private ExpandCollapseState ExpandCollapseState + => !CanToggle ? ExpandCollapseState.LeafNode + : _isExpanded ? ExpandCollapseState.Expanded + : ExpandCollapseState.Collapsed; + + /// + protected override AutomationPeer OnCreateAutomationPeer() + => _peer = new DetailSectionHeaderAutomationPeer(this); + + private void RequestToggle() => OnClick(); + + private sealed class DetailSectionHeaderAutomationPeer : ButtonAutomationPeer, IExpandCollapseProvider + { + public DetailSectionHeaderAutomationPeer(DetailSectionHeader owner) + : base(owner) + { + } + + private DetailSectionHeader Header => (DetailSectionHeader)Owner; + + public ExpandCollapseState ExpandCollapseState => Header.ExpandCollapseState; + + public bool ShowsMenu => false; + + public void Expand() + { + if (Header.ExpandCollapseState == ExpandCollapseState.Collapsed) + Header.RequestToggle(); + } + + public void Collapse() + { + if (Header.ExpandCollapseState == ExpandCollapseState.Expanded) + Header.RequestToggle(); + } + + public void RaiseExpandCollapseStateChanged(ExpandCollapseState old, ExpandCollapseState current) + => RaisePropertyChangedEvent(ExpandCollapsePatternIdentifiers.ExpandCollapseStateProperty, + old, current); + } + } + + /// + /// The square +/- box of a : a thin outline holding a + /// minus bar, plus a vertical bar while collapsed. + /// + public sealed class DetailExpanderBox : Border + { + private readonly Rectangle _verticalBar; + + /// Creates a box drawing a minus when , else a + /// plus. + public DetailExpanderBox(bool isExpanded) + { + Width = FwAvaloniaDensity.TreeBoxSize; + Height = FwAvaloniaDensity.TreeBoxSize; + BorderThickness = FwAvaloniaDensity.HairlineBorderThickness; + BorderBrush = FwAvaloniaDensity.TreeBoxBorderBrush; + Background = FwAvaloniaDensity.TreeBoxBackgroundBrush; + VerticalAlignment = VerticalAlignment.Center; + _verticalBar = Bar(FwAvaloniaDensity.TreeBoxGlyphThickness, FwAvaloniaDensity.TreeBoxGlyphLength); + _verticalBar.IsVisible = !isExpanded; + Child = new Panel + { + Children = + { + Bar(FwAvaloniaDensity.TreeBoxGlyphLength, FwAvaloniaDensity.TreeBoxGlyphThickness), + _verticalBar + } + }; + } + + /// True when the box draws a minus, false when it draws a plus. + public bool IsExpanded + { + get => !_verticalBar.IsVisible; + set => _verticalBar.IsVisible = !value; + } + + private static Rectangle Bar(double width, double height) => new Rectangle + { + Width = width, + Height = height, + Fill = FwAvaloniaDensity.TreeBoxGlyphBrush, + HorizontalAlignment = HorizontalAlignment.Center, + VerticalAlignment = VerticalAlignment.Center + }; + } +} diff --git a/Src/Common/FwAvalonia/Detail/DetailVisibility.cs b/Src/Common/FwAvalonia/Detail/DetailVisibility.cs index 8d0b007d93..64d6f5fd0e 100644 --- a/Src/Common/FwAvalonia/Detail/DetailVisibility.cs +++ b/Src/Common/FwAvalonia/Detail/DetailVisibility.cs @@ -29,11 +29,10 @@ public DetailHeaderRange(int headerIndex, int start, int end) } /// - /// Pure, model-level computation of collapsible-header ownership and field visibility. A - /// virtualizing items panel only realizes on-screen containers, so (unlike the legacy - /// WinForms tree boxes, and unlike DataTree's current captured-control approach) visibility - /// cannot live on control references and must be derivable from the field list and expansion - /// state alone. + /// Pure, model-level computation of collapsible-header ownership and field visibility, + /// derived from the field list and expansion state alone. Needing no controls, it answers + /// for rows that have none, such as rows a collapsed header hides or an items panel has not + /// realized. /// public static class DetailVisibility { @@ -74,10 +73,9 @@ public static IReadOnlyList GetCollapsibleRanges(IReadOnlyLis /// visibility depends only on its ancestors' state, never its own. /// /// - /// Returns the recorded expansion state for a header's , - /// or null when none is recorded (falls back to ). + /// Returns the recorded expansion state for a header's + /// , or null when none is recorded (falls back to + /// ). /// May itself be null, meaning no state is ever recorded. /// public static IReadOnlyList ComputeVisibility( @@ -93,9 +91,7 @@ public static IReadOnlyList ComputeVisibility( foreach (var range in GetCollapsibleRanges(fields)) { - var header = fields[range.HeaderIndex]; - var expanded = getExpansionState?.Invoke(header.StableId) ?? header.IsInitiallyExpanded; - if (expanded) + if (IsExpanded(fields[range.HeaderIndex], getExpansionState)) continue; for (var i = range.Start; i < range.End; i++) visible[i] = false; @@ -103,6 +99,46 @@ public static IReadOnlyList ComputeVisibility( return visible; } + /// + /// The collapsed headers in that hide a row belonging to an + /// object no row of belonged to, in field order. Expanding + /// them shows an object that was just added to the record. Expansion state is read as in + /// . + /// + public static IReadOnlyList GetHeadersHidingNewObjects( + IReadOnlyList previous, + IReadOnlyList current, + Func getExpansionState) + { + if (previous == null || current == null) + return Array.Empty(); + + var known = new HashSet(); + foreach (var field in previous) + known.Add(field.ObjectHvo); + + var result = new List(); + foreach (var range in GetCollapsibleRanges(current)) + { + var header = current[range.HeaderIndex]; + if (IsExpanded(header, getExpansionState)) + continue; + for (var i = range.Start; i < range.End; i++) + { + var hvo = current[i].ObjectHvo; + if (hvo != 0 && !known.Contains(hvo)) + { + result.Add(header); + break; + } + } + } + return result; + } + + private static bool IsExpanded(DetailField header, Func getExpansionState) + => getExpansionState?.Invoke(header.ExpansionKey) ?? header.IsInitiallyExpanded; + /// The subsequence of that keeps visible. public static IReadOnlyList GetVisibleFields( diff --git a/Src/Common/FwAvalonia/DetailHostControl.cs b/Src/Common/FwAvalonia/DetailHostControl.cs index 66e7f5fba6..4da33f0196 100644 --- a/Src/Common/FwAvalonia/DetailHostControl.cs +++ b/Src/Common/FwAvalonia/DetailHostControl.cs @@ -22,6 +22,9 @@ public sealed class DetailHostControl : AvaloniaHostControlBase // also survives across SESSIONS, mirroring legacy slice-splitter persistence. private double? _rememberedLabelColumnWidth; + // The root object of the detail last shown, or 0 when none was given. + private int _shownRootHvo; + /// public override bool IsDetailInteractionInFlight => CurrentContent is DataTree tree @@ -34,6 +37,14 @@ public DetailHostControl() AccessibleDescription = FwAvaloniaStrings.AvaloniaHostName; } + /// + /// Replaces the shown detail view with one for , carrying focus + /// and the label column width across. When names the same + /// object as the previous show, any collapsed section hiding an object that was not in + /// the previous view is expanded, so an object inserted into a collapsed section is + /// shown; a re-show that follows the previous view's own edit expands nothing. A + /// of 0 never matches. + /// public void ShowDetail(DetailModel detail, IDetailEditContext editContext = null, Action writingSystemFocused = null, Func getExpansionState = null, @@ -42,7 +53,8 @@ public void ShowDetail(DetailModel detail, IDetailEditContext editContext = null Action linkRequested = null, IFwClipboard clipboard = null, Func getLabelColumnWidth = null, - Action labelColumnWidthChanged = null) + Action labelColumnWidthChanged = null, + int rootHvo = 0) { if (detail == null) throw new ArgumentNullException(nameof(detail)); // Splitter position persists per-HOST across re-shows: this long-lived host owns @@ -62,6 +74,10 @@ public void ShowDetail(DetailModel detail, IDetailEditContext editContext = null view.EditCompleted += (s, e) => RaiseDetailEditCompleted(); view.InteractionCompleted += (s, e) => RaiseDetailInteractionCompleted(); + if (rootHvo != 0 && rootHvo == _shownRootHvo && CurrentContent is DataTree previous) + view.RevealObjectsAddedSince(previous); + _shownRootHvo = rootHvo; + // Continuity across the re-show, applied once the new view lays out. Focus leaves the // old view first: detaching a focused editor makes Avalonia refocus and scroll away. var focusMemento = DetailFocusMemory.Capture(CurrentContent); diff --git a/Src/Common/FwAvalonia/FwAvaloniaDensity.cs b/Src/Common/FwAvalonia/FwAvaloniaDensity.cs index 7ba9c13a5e..119af1d181 100644 --- a/Src/Common/FwAvalonia/FwAvaloniaDensity.cs +++ b/Src/Common/FwAvalonia/FwAvaloniaDensity.cs @@ -116,6 +116,17 @@ public static class FwAvaloniaDensity /// Slice label size: legacy 10pt (Slice.cs m_fontLabel). public const double LabelFontSize = 13.0; + /// Section header text size, collapsible or not: the same 10pt as a slice + /// label. + public const double SectionHeaderFontSize = LabelFontSize; + + /// Section header text colour, collapsible or not. + public static Avalonia.Media.IBrush SectionHeaderBrush => FwThemeResources.RequireBrush(GeneratedTokenKeys.FwSectionHeaderBrush); + + /// Size of the command links under a section header ("Field Options"): the same + /// 10pt as a slice label. + public const double HotlinkFontSize = LabelFontSize; + /// Writing-system abbreviation colour: #404040, measured from the legacy /// baseline. public static Avalonia.Media.IBrush WsAbbrevBrush => FwThemeResources.RequireBrush(GeneratedTokenKeys.FwWsAbbrevBrush); @@ -195,6 +206,32 @@ public static class FwAvaloniaDensity /// public static double TreeIndentPerLevel => FwThemeResources.RequireDouble(GeneratedTokenKeys.DataTree_TreeIndentPerLevel); + /// Outer size of the square +/- box before a collapsible section header's + /// label. + public static double TreeBoxSize => FwThemeResources.RequireDouble(GeneratedTokenKeys.DataTree_TreeBoxSize); + + /// Length of the +/- box's minus bar and of the plus sign's vertical + /// bar. + public static double TreeBoxGlyphLength => FwThemeResources.RequireDouble(GeneratedTokenKeys.DataTree_TreeBoxGlyphLength); + + /// Stroke width of the +/- box's bars. + public static double TreeBoxGlyphThickness => FwThemeResources.RequireDouble(GeneratedTokenKeys.DataTree_TreeBoxGlyphThickness); + + /// Horizontal gap between the +/- box and the header label. + public static double TreeBoxGap => FwThemeResources.RequireDouble(GeneratedTokenKeys.DataTree_TreeBoxGap); + + /// The +/- box outline. + public static Avalonia.Media.IBrush TreeBoxBorderBrush => FwThemeResources.RequireBrush(GeneratedTokenKeys.FwTreeBoxBorderBrush); + + /// The +/- box's minus and plus bars. + public static Avalonia.Media.IBrush TreeBoxGlyphBrush => FwThemeResources.RequireBrush(GeneratedTokenKeys.FwTreeBoxGlyphBrush); + + /// + /// The +/- box fill. Resolves Semi's base-surface role (SemiColorBackground0) directly, + /// so the box is opaque on the detail surface it sits on. + /// + public static Avalonia.Media.IBrush TreeBoxBackgroundBrush => FwThemeResources.RequireBrush("SemiColorBackground0"); + /// /// Compact width of the collapsed dropdown chooser (POS picker and similar) so the /// collapsed control reads as a field-sized box rather than shrinking to its current diff --git a/Src/Common/FwAvalonia/FwAvaloniaTests/DetailViewingParityTests.cs b/Src/Common/FwAvalonia/FwAvaloniaTests/DetailViewingParityTests.cs index b09bf2f16d..412bd7c689 100644 --- a/Src/Common/FwAvalonia/FwAvaloniaTests/DetailViewingParityTests.cs +++ b/Src/Common/FwAvalonia/FwAvaloniaTests/DetailViewingParityTests.cs @@ -6,11 +6,15 @@ using System.Linq; using Avalonia; using Avalonia.Automation; +using Avalonia.Automation.Peers; +using Avalonia.Automation.Provider; using Avalonia.Controls; +using Avalonia.Headless; using Avalonia.Headless.NUnit; using Avalonia.Interactivity; using Avalonia.Threading; using Avalonia.VisualTree; +using FwAvaloniaTests.VisualChecks; using NUnit.Framework; using SIL.FieldWorks.Common.FwAvalonia.Detail; using SIL.FieldWorks.Common.FwAvalonia.ViewDefinition; @@ -251,6 +255,315 @@ public void ExpansionState_PersistsThroughTheSuppliedStore_AndAppliesOnRebuild() Is.False, "the persisted collapse state applies to the rebuilt view"); } + // Re-queried after every toggle: a toggle rebuilds the rows, header included. + private static DetailSectionHeader SectionHeader(Control view, string id) + => view.GetVisualDescendants().OfType() + .First(h => AutomationProperties.GetAutomationId(h) == id); + + private static bool TextBoxPresent(Control view, string idPrefix) => view.HasDescendant( + t => (AutomationProperties.GetAutomationId(t) ?? "").StartsWith(idPrefix)); + + private static void ClickHeader(Button button) + { + button.RaiseEvent(new RoutedEventArgs(Button.ClickEvent)); + Dispatcher.UIThread.RunJobs(); + } + + private static DataTree ShowWithStore(DetailModel model, Dictionary store) + { + var view = new DataTree(model, null, null, + key => store.TryGetValue(key, out var e) ? e : (bool?)null, + (key, e) => store[key] = e); + new Window { Content = view, Width = 480, Height = 300 }.Show(); + Dispatcher.UIThread.RunJobs(); + return view; + } + + [AvaloniaTest] + public void CollapsibleHeader_DrawsAPlusMinusBox_ThatFlipsWithTheToggle() + { + var view = Show(Header("h1", "Sense 1", 0), Text("g1", "Gloss", 1), + Header("h2", "Variants", 0)); + var window = (Window)TopLevel.GetTopLevel(view); + DialogSnapshot.Capture(window, "Detail-SectionExpander-01-expanded"); + + var header = SectionHeader(view, "h1"); + Assert.That(header.Box.Opacity, Is.EqualTo(1), "a header with rows under it shows its box"); + Assert.That(header.Box.IsExpanded, Is.True, "an expanded section shows a minus"); + Assert.That(header.GetVisualDescendants().OfType().Select(t => t.Text), + Is.EqualTo(new[] { "Sense 1" }), "the label is the header's only text, with no glyph"); + + ClickHeader(header); + + var collapsed = SectionHeader(view, "h1"); + Assert.That(collapsed.IsExpanded, Is.False); + Assert.That(collapsed.Box.IsExpanded, Is.False, "a collapsed section shows a plus"); + Assert.That(TextBoxPresent(view, "g1"), Is.False); + DialogSnapshot.Capture(window, "Detail-SectionExpander-02-collapsed"); + + ClickHeader(collapsed); + Assert.That(SectionHeader(view, "h1").Box.IsExpanded, Is.True); + Assert.That(TextBoxPresent(view, "g1"), Is.True); + } + + // LT-22689: the header and link text sizes and the header colour are set on the text + // itself, so neither the view's TextBlock style nor the button theme decides them. + [AvaloniaTest] + public void SectionHeaderAndFieldOptionsText_TakeTheirOwnSizeAndColour() + { + var collapsible = new DetailField("hdr", "Allomorphs", null, null, DetailFieldKind.Header, + EditorClassification.GroupingNone, null, null, HostRouting.Inherit, null, null, null, + isEditable: false, indent: 0, isCollapsible: true, hotlinksId: "mnu-hotlinks"); + var plain = new DetailField("plain", "Grammatical Info.", null, null, DetailFieldKind.Header, + EditorClassification.GroupingNone, null, null, HostRouting.Inherit, null, null, null, + isEditable: false, indent: 0, isCollapsible: false); + var view = new DataTree(new DetailModel("LexEntry", "Normal", + new List { collapsible, Text("f1", "Form", 1), plain }, new List()), + menuRequested: request => { }); + var window = new Window { Content = view, Width = 480, Height = 300 }; + window.Show(); + Dispatcher.UIThread.RunJobs(); + DialogSnapshot.Capture(window, "Detail-SectionExpander-03-header-and-link-text"); + + TextBlock TextOf(string text) => view.GetVisualDescendants().OfType().First(t => t.Text == text); + var headerSize = SIL.FieldWorks.Common.FwAvalonia.FwAvaloniaDensity.SectionHeaderFontSize; + var linkSize = SIL.FieldWorks.Common.FwAvalonia.FwAvaloniaDensity.HotlinkFontSize; + + Assert.That(TextOf("Allomorphs").FontSize, Is.EqualTo(headerSize), "a collapsible header"); + Assert.That(TextOf("Grammatical Info.").FontSize, Is.EqualTo(headerSize), + "a header that cannot collapse matches one that can"); + var headerInk = SIL.FieldWorks.Common.FwAvalonia.FwAvaloniaDensity.SectionHeaderBrush; + Assert.That(TextOf("Allomorphs").Foreground, Is.SameAs(headerInk), + "a collapsible header is not the button theme's blue"); + Assert.That(TextOf("Grammatical Info.").Foreground, Is.SameAs(headerInk)); + var link = TextOf(SIL.FieldWorks.Common.FwAvalonia.FwAvaloniaStrings.FieldOptionsMenu); + Assert.That(link.FontSize, Is.EqualTo(linkSize)); + Assert.That(link.FontWeight, Is.EqualTo(Avalonia.Media.FontWeight.Normal), + "the link is not the button theme's semibold"); + } + + [AvaloniaTest] + public void HeaderWithNothingUnderIt_DrawsNoBox_AndIgnoresClicks() + { + var store = new Dictionary(); + var view = ShowWithStore(new DetailModel("LexEntry", "Normal", + new List { Header("empty", "Variants", 0), Header("h2", "Allomorphs", 0), Text("a1", "Form", 1) }, + new List()), store); + + var empty = SectionHeader(view, "empty"); + Assert.That(empty.CanToggle, Is.False); + Assert.That(empty.Box.Opacity, Is.EqualTo(0), "nothing to collapse, so no box is drawn"); + Assert.That(empty.Box.Bounds.Width, Is.GreaterThan(0), + "the blank box keeps its width, so the label lines up with its siblings'"); + + ClickHeader(empty); + Assert.That(store, Is.Empty, "a header with nothing under it records no state"); + Assert.That(SectionHeader(view, "h2").CanToggle, Is.True); + } + + [AvaloniaTest] + public void Toggling_KeepsFocusOnTheHeader() + { + var view = Show(Header("h1", "Sense 1", 0), Text("g1", "Gloss", 1)); + var header = SectionHeader(view, "h1"); + header.Focus(); + Dispatcher.UIThread.RunJobs(); + Assert.That(header.IsFocused, Is.True, "precondition: the header has focus"); + + ClickHeader(header); + + Assert.That(SectionHeader(view, "h1"), Is.SameAs(header), + "a toggle adds or removes only the rows under the header, never the header itself"); + Assert.That(header.IsFocused, Is.True); + } + + // LT-22689: each click of a double-click toggles, as each click of the +/- box does in + // the WinForms DataTree, so a double-click expands and collapses again. + [AvaloniaTest] + public void DoubleClickingAHeader_TogglesItTwice() + { + var view = Show(Header("h1", "Sense 1", 0), Text("g1", "Gloss", 1)); + var window = (Window)TopLevel.GetTopLevel(view); + var point = SectionHeader(view, "h1").TranslatePoint(new Point(4, 4), window); + Assert.That(point, Is.Not.Null, "precondition: the header is laid out"); + + window.MouseDown(point.Value, Avalonia.Input.MouseButton.Left); + window.MouseUp(point.Value, Avalonia.Input.MouseButton.Left); + Assert.That(SectionHeader(view, "h1").IsExpanded, Is.False, "the first click collapses"); + window.MouseDown(point.Value, Avalonia.Input.MouseButton.Left); + window.MouseUp(point.Value, Avalonia.Input.MouseButton.Left); + Dispatcher.UIThread.RunJobs(); + + Assert.That(SectionHeader(view, "h1").IsExpanded, Is.True, "the second click expands again"); + Assert.That(TextBoxPresent(view, "g1"), Is.True); + } + + // LT-22689: a toggle rebuilds every row, which must not scroll the view; the header the + // user clicked stays where it was. + [AvaloniaTest] + public void Toggling_KeepsTheHeaderWhereItWasInTheViewport() + { + var fields = new List(); + for (var i = 0; i < 30; i++) + fields.Add(Text($"above{i}", $"Above {i}", 0)); + fields.Add(Header("h1", "Sense 9", 0)); + for (var i = 0; i < 10; i++) + fields.Add(Text($"gloss{i}", $"Gloss {i}", 1)); + for (var i = 0; i < 30; i++) + fields.Add(Text($"below{i}", $"Below {i}", 0)); + var view = Show(fields.ToArray()); + var scroller = view.GetVisualDescendants().OfType() + .First(s => AutomationProperties.GetAutomationId(s) == "DataTree.Scroll"); + double TopOf(Control control) => control.TranslatePoint(new Point(0, 0), scroller)?.Y ?? double.NaN; + void Settle() + { + view.UpdateLayout(); + Dispatcher.UIThread.RunJobs(); + } + + scroller.Offset = new Vector(0, scroller.Offset.Y + TopOf(SectionHeader(view, "h1")) - 100); + Settle(); + var header = SectionHeader(view, "h1"); + var top = TopOf(header); + Assert.That(scroller.Offset.Y, Is.GreaterThan(0), "precondition: the view is scrolled"); + Assert.That(top, Is.EqualTo(100).Within(1), "precondition: the header is mid-viewport"); + + // A mouse press focuses the header before its Click. + header.Focus(); + ClickHeader(header); + Settle(); + Assert.That(TopOf(SectionHeader(view, "h1")), Is.EqualTo(top).Within(1), + "collapsing leaves the header where it was"); + + ClickHeader(SectionHeader(view, "h1")); + Settle(); + Assert.That(TopOf(SectionHeader(view, "h1")), Is.EqualTo(top).Within(1), + "expanding leaves the header where it was"); + } + + [AvaloniaTest] + public void AutomationPeer_ReportsAndDrivesTheExpandCollapseState() + { + var view = Show(Header("h1", "Sense 1", 0), Text("g1", "Gloss", 1), Header("leaf", "Variants", 0)); + IExpandCollapseProvider Provider(string id) => ControlAutomationPeer + .CreatePeerForElement(SectionHeader(view, id)).GetProvider(); + + Assert.That(Provider("h1"), Is.Not.Null, "the header exposes the ExpandCollapse pattern"); + Assert.That(Provider("h1").ExpandCollapseState, Is.EqualTo(ExpandCollapseState.Expanded)); + Assert.That(Provider("leaf").ExpandCollapseState, Is.EqualTo(ExpandCollapseState.LeafNode)); + + Provider("h1").Collapse(); + Dispatcher.UIThread.RunJobs(); + Assert.That(Provider("h1").ExpandCollapseState, Is.EqualTo(ExpandCollapseState.Collapsed)); + Assert.That(TextBoxPresent(view, "g1"), Is.False); + + Provider("h1").Collapse(); + Dispatcher.UIThread.RunJobs(); + Assert.That(Provider("h1").ExpandCollapseState, Is.EqualTo(ExpandCollapseState.Collapsed), + "collapsing a collapsed section leaves it collapsed"); + + Provider("h1").Expand(); + Dispatcher.UIThread.RunJobs(); + Assert.That(TextBoxPresent(view, "g1"), Is.True); + } + + [AvaloniaTest] + public void ExpansionState_IsRecordedUnderTheExpansionKey() + { + var header = Header("h1@101", "Senses", 0); + header.ExpansionKey = "h1@guid"; + var model = new DetailModel("LexEntry", "Normal", + new List { header, Text("g1", "Gloss", 1) }, new List()); + var store = new Dictionary(); + + ClickHeader(SectionHeader(ShowWithStore(model, store), "h1@101")); + + Assert.That(store, Is.EqualTo(new Dictionary { ["h1@guid"] = false })); + } + + private static DetailField ObjectHeader(string id, int hvo) => new DetailField( + id, id, null, null, DetailFieldKind.Header, EditorClassification.GroupingNone, + id, null, HostRouting.Inherit, null, null, null, + isEditable: false, indent: 0, isCollapsible: true, objectHvo: hvo); + + private static DetailField ObjectRow(string id, int hvo) => new DetailField( + id, id, id, null, DetailFieldKind.Text, EditorClassification.Known, id, null, + HostRouting.Inherit, new List { new DetailWsValue("en", "value") }, + null, null, indent: 1, objectHvo: hvo); + + // LT-22689: collapsing removes the rows under a header, so a removed reference vector's + // current item must not stay the target of the item-relative commands. + [AvaloniaTest] + public void CollapsingTheSectionOfTheCurrentVectorItem_LeavesNoCurrentItem() + { + var vectorField = new DetailField("LexEntry/x/#7", "Subentries", "Subentries", null, + DetailFieldKind.ReferenceVector, EditorClassification.Known, "Subentries", null, + HostRouting.Inherit, null, null, null, isEditable: false, indent: 1, + items: new List { new DetailChoiceOption("a", "A"), new DetailChoiceOption("b", "B") }); + var view = Show(Header("h1", "Sense 1", 0), vectorField); + var vector = view.GetVisualDescendants().OfType().Single(); + Assert.That(vector.SelectItem("b"), Is.True); + Assert.That(DetailFocusMemory.Capture(view).VectorItemKey, Is.EqualTo("b"), + "precondition: the vector row has a current item"); + + ClickHeader(SectionHeader(view, "h1")); + + Assert.That(DetailFocusMemory.Capture(view).VectorItemKey, Is.Null); + } + + // An entry with one sense, then the same entry after a second sense (hvo 11) was added. + private static DetailModel SensesBefore() => new DetailModel("LexEntry", "Normal", new List + { + ObjectHeader("senses", 1), ObjectRow("g10", 10), + ObjectHeader("allomorphs", 1), ObjectRow("f20", 20) + }, new List()); + + private static DetailModel SensesAfterInsert() => new DetailModel("LexEntry", "Normal", new List + { + ObjectHeader("senses", 1), ObjectRow("g10", 10), ObjectRow("g11", 11), + ObjectHeader("allomorphs", 1), ObjectRow("f20", 20) + }, new List()); + + [AvaloniaTest] + public void RevealObjectsAddedSince_ExpandsTheCollapsedSectionHidingANewObject() + { + var store = new Dictionary { ["senses"] = false, ["allomorphs"] = false }; + var view = ShowWithStore(SensesAfterInsert(), store); + Assert.That(TextBoxPresent(view, "g11"), Is.False, "precondition: the new sense is hidden"); + + view.RevealObjectsAddedSince(new DataTree(SensesBefore())); + Dispatcher.UIThread.RunJobs(); + + Assert.That(TextBoxPresent(view, "g11"), Is.True, "the inserted sense is shown"); + Assert.That(store["senses"], Is.True, "the expansion is recorded like a click"); + Assert.That(TextBoxPresent(view, "f20"), Is.False, "a section that gained nothing stays collapsed"); + Assert.That(store["allomorphs"], Is.False); + } + + // LT-22689: typing into a section and then clicking its +/- commits the text on the + // press and collapses on the release. The re-show that commit causes must not reopen + // the section the user just closed. + [AvaloniaTest] + public void RevealObjectsAddedSince_LeavesSectionsAlone_AfterThePreviousViewsOwnEdit() + { + var context = new FakeDetailEditContext(); + var previous = new DataTree(SensesBefore(), context); + previous.RaiseEvent(new Avalonia.Input.KeyEventArgs + { + RoutedEvent = Avalonia.Input.InputElement.KeyDownEvent, + Key = Avalonia.Input.Key.Enter + }); + Assert.That(context.CommitCount, Is.EqualTo(1), "precondition: the previous view committed"); + var store = new Dictionary { ["senses"] = false }; + var view = ShowWithStore(SensesAfterInsert(), store); + + view.RevealObjectsAddedSince(previous); + Dispatcher.UIThread.RunJobs(); + + Assert.That(TextBoxPresent(view, "g11"), Is.False, "the section stays collapsed"); + Assert.That(store["senses"], Is.False, "the user's collapse stays recorded"); + } + [AvaloniaTest] public void LabelTooltips_Splitter_BoldEmphasis_AndCopyMenu_RenderLikeLegacy() { diff --git a/Src/Common/FwAvalonia/FwAvaloniaTests/DetailVisibilityTests.cs b/Src/Common/FwAvalonia/FwAvaloniaTests/DetailVisibilityTests.cs index 27bffb99f2..f654bdd4b5 100644 --- a/Src/Common/FwAvalonia/FwAvaloniaTests/DetailVisibilityTests.cs +++ b/Src/Common/FwAvalonia/FwAvaloniaTests/DetailVisibilityTests.cs @@ -152,5 +152,75 @@ public void GetVisibleFields_ReturnsOnlyVisibleFieldsInOrder() Assert.That(result, Is.EqualTo(new[] { h1, h2, g2 })); } + + // LT-22689: state is read by ExpansionKey, so a key naming the object by guid finds the + // state recorded in an earlier session even when the row's StableId has changed. + [Test] + public void ComputeVisibility_ReadsStateByExpansionKey_NotStableId() + { + var header = Header("Senses@101", "Senses", 0); + header.ExpansionKey = "Senses@guid"; + var fields = new[] { header, Text("g1", "Gloss", 1) }; + + var visible = DetailVisibility.ComputeVisibility(fields, + key => key == "Senses@guid" ? (bool?)false : null); + + Assert.That(visible, Is.EqualTo(new[] { true, false })); + } + + private static DetailField ObjectHeader(string id, int hvo) => new DetailField( + id, id, null, null, DetailFieldKind.Header, EditorClassification.GroupingNone, + null, null, HostRouting.Inherit, null, null, null, + isEditable: false, indent: 0, isCollapsible: true, objectHvo: hvo); + + private static DetailField ObjectRow(string id, int hvo) => new DetailField( + id, id, id, null, DetailFieldKind.Text, EditorClassification.Known, id, null, + HostRouting.Inherit, null, null, null, indent: 1, objectHvo: hvo); + + [Test] + public void GetHeadersHidingNewObjects_ReturnsOnlyCollapsedHeadersOverANewObject() + { + var previous = new[] + { + ObjectHeader("senses", 1), ObjectRow("g10", 10), + ObjectHeader("allomorphs", 1), ObjectRow("f20", 20), + ObjectHeader("variants", 1), ObjectRow("v30", 30) + }; + var current = new[] + { + ObjectHeader("senses", 1), ObjectRow("g10", 10), ObjectRow("g11", 11), + ObjectHeader("allomorphs", 1), ObjectRow("f20", 20), + ObjectHeader("variants", 1), ObjectRow("v30", 30), ObjectRow("v31", 31) + }; + var collapsed = new HashSet { "senses", "allomorphs" }; + + var headers = DetailVisibility.GetHeadersHidingNewObjects(previous, current, + key => collapsed.Contains(key) ? (bool?)false : null); + + Assert.That(headers, Is.EqualTo(new[] { current[0] }), + "only Senses is both collapsed and over a new object: Allomorphs gained nothing, " + + "and Variants is already expanded"); + } + + [Test] + public void GetHeadersHidingNewObjects_ExpandsEveryCollapsedAncestorOfTheNewObject() + { + var outer = ObjectHeader("senses", 1); + var item = new DetailField("sense11", "2 gloss", null, null, DetailFieldKind.Header, + EditorClassification.GroupingNone, null, null, HostRouting.Inherit, null, null, null, + isEditable: false, indent: 1, isCollapsible: true, isInitiallyExpanded: false, + objectHvo: 11); + var gloss = new DetailField("g11", "Gloss", "Gloss", null, DetailFieldKind.Text, + EditorClassification.Known, "g11", null, HostRouting.Inherit, null, null, null, + indent: 2, objectHvo: 11); + var previous = new[] { outer }; + var current = new[] { outer, item, gloss }; + + var headers = DetailVisibility.GetHeadersHidingNewObjects(previous, current, + key => key == "senses" ? (bool?)false : null); + + Assert.That(headers, Is.EqualTo(new[] { outer, item }), + "the new item's own collapsed header is expanded too, so its rows show"); + } } } diff --git a/Src/Common/FwAvalonia/FwAvaloniaTests/ViewDefinitionTests.cs b/Src/Common/FwAvalonia/FwAvaloniaTests/ViewDefinitionTests.cs index 52a0ffbee4..79f3bcd411 100644 --- a/Src/Common/FwAvalonia/FwAvaloniaTests/ViewDefinitionTests.cs +++ b/Src/Common/FwAvalonia/FwAvaloniaTests/ViewDefinitionTests.cs @@ -34,6 +34,11 @@ public class XmlLayoutImporterTests + + + + + @@ -122,6 +127,24 @@ public void Import_NestedGrouping_ProducesGroupWithChildren() Assert.That(header.Children[1].Field, Is.EqualTo("Bibliography")); } + // LT-22689: only "collapsed" on the part reference starts a section collapsed. The + // shipped "expanded", no attribute, and Notebook's one "doNotExpand" all leave it open. + [TestCase("expansion='expanded'", ViewExpansion.Expanded)] + [TestCase("expansion='collapsed'", ViewExpansion.Collapsed)] + [TestCase("", ViewExpansion.NotApplicable)] + [TestCase("expansion='doNotExpand'", ViewExpansion.NotApplicable)] + public void Import_SectionExpansion_ComesFromThePartReference(string attribute, ViewExpansion expected) + { + var model = Import($@" + + +"); + + var section = model.Roots.Single(); + Assert.That(section.Kind, Is.EqualTo(ViewNodeKind.Group), "precondition: a section with rows"); + Assert.That(section.Expansion, Is.EqualTo(expected)); + } + [Test] public void Import_SequenceAndCustomFieldPlaceholder() { diff --git a/Src/Common/FwAvaloniaTheme/Tokens/DataTree/DataTreeTokens.axaml b/Src/Common/FwAvaloniaTheme/Tokens/DataTree/DataTreeTokens.axaml index ee2263252b..1e37b0b9fd 100644 --- a/Src/Common/FwAvaloniaTheme/Tokens/DataTree/DataTreeTokens.axaml +++ b/Src/Common/FwAvaloniaTheme/Tokens/DataTree/DataTreeTokens.axaml @@ -69,6 +69,20 @@ chooser tree's per-depth inset), so depth-indented rows indent identically. --> 14 + + 9 + + + 5 + + + 1 + + + 4 + diff --git a/Src/Common/FwAvaloniaTheme/Tokens/FwColorTokens.axaml b/Src/Common/FwAvaloniaTheme/Tokens/FwColorTokens.axaml index 840c1be971..aefee7a895 100644 --- a/Src/Common/FwAvaloniaTheme/Tokens/FwColorTokens.axaml +++ b/Src/Common/FwAvaloniaTheme/Tokens/FwColorTokens.axaml @@ -70,6 +70,20 @@ option, so Gray stays for pixel parity. --> + + + + + + + + + + 9px tree-view glyph box. --> 9 - + - + - + diff --git a/Src/xWorks/Avalonia/Composer/DetailComposer.cs b/Src/xWorks/Avalonia/Composer/DetailComposer.cs index 804f2611b5..020999c926 100644 --- a/Src/xWorks/Avalonia/Composer/DetailComposer.cs +++ b/Src/xWorks/Avalonia/Composer/DetailComposer.cs @@ -3479,8 +3479,8 @@ private void WalkSequence(ViewNode node, ICmObject obj, int depth) // HeavySummary part ref binds mnuDataTree-Sense in LexSense.fwlayout) -- the // sequence node itself usually has none. var itemBinding = ResolveItemMenuBinding(node, item); - // Its ExpansionKey names the item, not its position, so expansion state - // follows the item through a reorder. + // Its ExpansionKey names the item, so expansion state follows the item + // through a reorder. AddField(new DetailField($"{StableId(node, obj)}/item{i}", itemLabel, node.Field, null, DetailFieldKind.Header, EditorClassification.GroupingNone, null, null, HostRouting.Inherit, @@ -3650,8 +3650,7 @@ private string ResolveShortName(int hvo) private static string StableId(ViewNode node, ICmObject obj) => $"{node.StableId}@{obj.Hvo}"; - // An object's hvo is assigned per session, so a key persisted across sessions names - // the object by guid instead. + // Names the object by guid, which identifies it in every session. private static string ExpansionKey(string nodeId, ICmObject obj) => $"{nodeId}@{obj.Guid:N}"; private static string Localize(string label) From 537306989b8992897531ab33a9de1935039cda7b Mon Sep 17 00:00:00 2001 From: Zachary Burnham Date: Tue, 13 Oct 2026 17:52:44 -0400 Subject: [PATCH 3/4] LT-22689: Reveal inserts unless the section was closed after an edit The insert auto-expand skipped every re-show that followed the view's own edit. A re-show can carry an insert from another window, or from a menu command whose refresh joined the edit's re-show, and those inserts stayed hidden in their collapsed sections. The view now records the sections the user collapsed after its own edit completed, and the reveal leaves out only those. Typing into a section and then collapsing it still leaves it collapsed. Co-Authored-By: Claude Opus 5.5 --- Src/Common/FwAvalonia/Detail/DataTree.cs | 21 +++++--- Src/Common/FwAvalonia/DetailHostControl.cs | 2 +- .../DetailViewingParityTests.cs | 48 +++++++++++++++---- 3 files changed, 55 insertions(+), 16 deletions(-) diff --git a/Src/Common/FwAvalonia/Detail/DataTree.cs b/Src/Common/FwAvalonia/Detail/DataTree.cs index 4024a89e67..01f7f0b21c 100644 --- a/Src/Common/FwAvalonia/Detail/DataTree.cs +++ b/Src/Common/FwAvalonia/Detail/DataTree.cs @@ -462,6 +462,8 @@ private void OnCancel() // Set once this view commits or cancels an edit of its own. The re-show that follows // replaces the view, so the flag never outlives that edit. private bool _completedOwnEdit; + // The expansion keys of the sections the user collapsed after that edit completed. + private readonly HashSet _collapsedAfterOwnEdit = new HashSet(); /// /// Raises the completion the host re-shows on -- unless a pointer gesture is in flight, @@ -592,24 +594,29 @@ private void ToggleSection(DetailField field, DetailSectionHeader header) { if (!header.CanToggle) return; - RecordExpansion(field, !IsExpanded(field)); + var expanded = !IsExpanded(field); + RecordExpansion(field, expanded); + if (!expanded && _completedOwnEdit) + _collapsedAfterOwnEdit.Add(field.ExpansionKey); ApplyVisibility(); } /// /// Expands every collapsed section that hides a row of an object absent from the view /// , recording each through the expansion hooks, so an object - /// just inserted into a collapsed section is shown. Does nothing when - /// is null, or completed an edit of its own: an object that - /// edit created was typed where the user could see it, so a section hiding it now was - /// collapsed on purpose. + /// just inserted into a collapsed section is shown. A section the user collapsed in + /// after its own edit completed stays collapsed, as when + /// typing into a section and then collapsing it. Does nothing when + /// is null. /// public void RevealObjectsAddedSince(DataTree previous) { - if (previous == null || previous._completedOwnEdit) + if (previous == null) return; var headers = DetailVisibility.GetHeadersHidingNewObjects(previous.Model.Fields, Model.Fields, - GetRecordedExpansion); + GetRecordedExpansion) + .Where(header => !previous._collapsedAfterOwnEdit.Contains(header.ExpansionKey)) + .ToList(); if (headers.Count == 0) return; foreach (var header in headers) diff --git a/Src/Common/FwAvalonia/DetailHostControl.cs b/Src/Common/FwAvalonia/DetailHostControl.cs index 4da33f0196..a150cd730d 100644 --- a/Src/Common/FwAvalonia/DetailHostControl.cs +++ b/Src/Common/FwAvalonia/DetailHostControl.cs @@ -42,7 +42,7 @@ public DetailHostControl() /// and the label column width across. When names the same /// object as the previous show, any collapsed section hiding an object that was not in /// the previous view is expanded, so an object inserted into a collapsed section is - /// shown; a re-show that follows the previous view's own edit expands nothing. A + /// shown, except a section the user collapsed after the previous view's own edit. A /// of 0 never matches. /// public void ShowDetail(DetailModel detail, IDetailEditContext editContext = null, diff --git a/Src/Common/FwAvalonia/FwAvaloniaTests/DetailViewingParityTests.cs b/Src/Common/FwAvalonia/FwAvaloniaTests/DetailViewingParityTests.cs index 4fae2842c4..d6192ff1a1 100644 --- a/Src/Common/FwAvalonia/FwAvaloniaTests/DetailViewingParityTests.cs +++ b/Src/Common/FwAvalonia/FwAvaloniaTests/DetailViewingParityTests.cs @@ -539,21 +539,37 @@ public void RevealObjectsAddedSince_ExpandsTheCollapsedSectionHidingANewObject() Assert.That(store["allomorphs"], Is.False); } - // LT-22689: typing into a section and then clicking its +/- commits the text on the - // press and collapses on the release. The re-show that commit causes must not reopen - // the section the user just closed. - [AvaloniaTest] - public void RevealObjectsAddedSince_LeavesSectionsAlone_AfterThePreviousViewsOwnEdit() + // A shown view of SensesBefore() that has saved an edit of its own, and then collapsed + // the senses section when collapseSensesAfterward is true. + private static DataTree PreviousViewThatSavedAnEdit(Dictionary store, + bool collapseSensesAfterward) { var context = new FakeDetailEditContext(); - var previous = new DataTree(SensesBefore(), context); + var previous = new DataTree(SensesBefore(), context, + getExpansionState: key => store.TryGetValue(key, out var e) ? e : (bool?)null, + expansionChanged: (key, e) => store[key] = e); + new Window { Content = previous, Width = 480, Height = 300 }.Show(); + Dispatcher.UIThread.RunJobs(); previous.RaiseEvent(new Avalonia.Input.KeyEventArgs { RoutedEvent = Avalonia.Input.InputElement.KeyDownEvent, Key = Avalonia.Input.Key.Enter }); - Assert.That(context.CommitCount, Is.EqualTo(1), "precondition: the previous view committed"); - var store = new Dictionary { ["senses"] = false }; + Assert.That(context.CommitCount, Is.EqualTo(1), "precondition: the previous view saved an edit"); + if (collapseSensesAfterward) + ClickHeader(SectionHeader(previous, "senses")); + return previous; + } + + // LT-22689: typing into a section and then clicking its +/- commits the text on the + // press and collapses on the release. The re-show that commit causes leaves that section + // collapsed. + [AvaloniaTest] + public void RevealObjectsAddedSince_LeavesASectionCollapsedAfterTheOwnEditCollapsed() + { + var store = new Dictionary(); + var previous = PreviousViewThatSavedAnEdit(store, collapseSensesAfterward: true); + Assert.That(store["senses"], Is.False, "precondition: senses was collapsed after the edit"); var view = ShowWithStore(SensesAfterInsert(), store); view.RevealObjectsAddedSince(previous); @@ -563,6 +579,22 @@ public void RevealObjectsAddedSince_LeavesSectionsAlone_AfterThePreviousViewsOwn Assert.That(store["senses"], Is.False, "the user's collapse stays recorded"); } + // LT-22689: a re-show after the view's own edit can also carry an insert from another + // window or a menu command. A section collapsed before the edit still opens to show it. + [AvaloniaTest] + public void RevealObjectsAddedSince_ExpandsASectionCollapsedBeforeTheOwnEdit() + { + var store = new Dictionary { ["senses"] = false }; + var previous = PreviousViewThatSavedAnEdit(store, collapseSensesAfterward: false); + var view = ShowWithStore(SensesAfterInsert(), store); + + view.RevealObjectsAddedSince(previous); + Dispatcher.UIThread.RunJobs(); + + Assert.That(TextBoxPresent(view, "g11"), Is.True, "the inserted sense is shown"); + Assert.That(store["senses"], Is.True); + } + [AvaloniaTest] public void LabelTooltips_Splitter_BoldEmphasis_AndCopyMenu_RenderLikeLegacy() { From d488ed97386f76dcafa3c6f138296d8ad4cacdee Mon Sep 17 00:00:00 2001 From: Zachary Burnham Date: Tue, 13 Oct 2026 18:04:10 -0400 Subject: [PATCH 4/4] LT-22689: Keep a nested section closed when its parent stays closed The insert auto-expand judged each collapsed header on its own. When a parent the user collapsed after an edit stayed closed, a collapsed child under it was still expanded and saved, so reopening the parent later also reopened the child. The reveal now decides per new row: a row that a must-stay-collapsed header hides contributes none of its headers. Co-Authored-By: Claude Opus 5.5 --- Src/Common/FwAvalonia/Detail/DataTree.cs | 5 +-- .../FwAvalonia/Detail/DetailVisibility.cs | 40 ++++++++++++------- .../FwAvaloniaTests/DetailVisibilityTests.cs | 31 ++++++++++++++ 3 files changed, 59 insertions(+), 17 deletions(-) diff --git a/Src/Common/FwAvalonia/Detail/DataTree.cs b/Src/Common/FwAvalonia/Detail/DataTree.cs index 01f7f0b21c..2db4a87af6 100644 --- a/Src/Common/FwAvalonia/Detail/DataTree.cs +++ b/Src/Common/FwAvalonia/Detail/DataTree.cs @@ -614,9 +614,8 @@ public void RevealObjectsAddedSince(DataTree previous) if (previous == null) return; var headers = DetailVisibility.GetHeadersHidingNewObjects(previous.Model.Fields, Model.Fields, - GetRecordedExpansion) - .Where(header => !previous._collapsedAfterOwnEdit.Contains(header.ExpansionKey)) - .ToList(); + GetRecordedExpansion, + header => previous._collapsedAfterOwnEdit.Contains(header.ExpansionKey)); if (headers.Count == 0) return; foreach (var header in headers) diff --git a/Src/Common/FwAvalonia/Detail/DetailVisibility.cs b/Src/Common/FwAvalonia/Detail/DetailVisibility.cs index 64d6f5fd0e..298b1928fa 100644 --- a/Src/Common/FwAvalonia/Detail/DetailVisibility.cs +++ b/Src/Common/FwAvalonia/Detail/DetailVisibility.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; +using System.Linq; namespace SIL.FieldWorks.Common.FwAvalonia.Detail { @@ -102,13 +103,17 @@ public static IReadOnlyList ComputeVisibility( /// /// The collapsed headers in that hide a row belonging to an /// object no row of belonged to, in field order. Expanding - /// them shows an object that was just added to the record. Expansion state is read as in - /// . + /// them shows an object that was just added to the record. A row hidden by any header + /// accepts contributes none of its headers, since + /// that row stays hidden either way. Expansion state is read as in + /// ; a null keeps + /// no header collapsed. /// public static IReadOnlyList GetHeadersHidingNewObjects( IReadOnlyList previous, IReadOnlyList current, - Func getExpansionState) + Func getExpansionState, + Func mustStayCollapsed = null) { if (previous == null || current == null) return Array.Empty(); @@ -117,23 +122,30 @@ public static IReadOnlyList GetHeadersHidingNewObjects( foreach (var field in previous) known.Add(field.ObjectHvo); - var result = new List(); + var collapsed = new List(); foreach (var range in GetCollapsibleRanges(current)) { - var header = current[range.HeaderIndex]; - if (IsExpanded(header, getExpansionState)) + if (!IsExpanded(current[range.HeaderIndex], getExpansionState)) + collapsed.Add(range); + } + + var revealed = new SortedSet(); + for (var i = 0; i < current.Count; i++) + { + var hvo = current[i].ObjectHvo; + if (hvo == 0 || known.Contains(hvo)) continue; - for (var i = range.Start; i < range.End; i++) + var row = i; + var hiding = collapsed.Where(range => range.Start <= row && row < range.End).ToList(); + if (mustStayCollapsed != null + && hiding.Any(range => mustStayCollapsed(current[range.HeaderIndex]))) { - var hvo = current[i].ObjectHvo; - if (hvo != 0 && !known.Contains(hvo)) - { - result.Add(header); - break; - } + continue; } + foreach (var range in hiding) + revealed.Add(range.HeaderIndex); } - return result; + return revealed.Select(index => current[index]).ToList(); } private static bool IsExpanded(DetailField header, Func getExpansionState) diff --git a/Src/Common/FwAvalonia/FwAvaloniaTests/DetailVisibilityTests.cs b/Src/Common/FwAvalonia/FwAvaloniaTests/DetailVisibilityTests.cs index f654bdd4b5..3327c91bfe 100644 --- a/Src/Common/FwAvalonia/FwAvaloniaTests/DetailVisibilityTests.cs +++ b/Src/Common/FwAvalonia/FwAvaloniaTests/DetailVisibilityTests.cs @@ -202,6 +202,37 @@ public void GetHeadersHidingNewObjects_ReturnsOnlyCollapsedHeadersOverANewObject + "and Variants is already expanded"); } + private static DetailField NestedHeader(string id, int indent, int hvo) => new DetailField( + id, id, null, null, DetailFieldKind.Header, EditorClassification.GroupingNone, + null, null, HostRouting.Inherit, null, null, null, + isEditable: false, indent: indent, isCollapsible: true, objectHvo: hvo); + + private static DetailField NestedRow(string id, int indent, int hvo) => new DetailField( + id, id, id, null, DetailFieldKind.Text, EditorClassification.Known, id, null, + HostRouting.Inherit, null, null, null, indent: indent, objectHvo: hvo); + + // LT-22689: a new row under a collapsed child of a parent that must stay collapsed stays + // hidden, so neither header is expanded; the child keeps its own collapse. + [Test] + public void GetHeadersHidingNewObjects_SkipsEveryHeaderOverARowAParentKeepsHidden() + { + var parent = NestedHeader("parent", 0, 1); + var child = NestedHeader("child", 1, 2); + var other = NestedHeader("other", 0, 3); + var previous = new[] { parent, child, other }; + var current = new[] + { + parent, child, NestedRow("underChild", 2, 11), + other, NestedRow("underOther", 1, 12) + }; + + var headers = DetailVisibility.GetHeadersHidingNewObjects(previous, current, + key => false, header => header.ExpansionKey == "parent"); + + Assert.That(headers, Is.EqualTo(new[] { other }), + "the row the parent keeps hidden reveals nothing; the other section's row still does"); + } + [Test] public void GetHeadersHidingNewObjects_ExpandsEveryCollapsedAncestorOfTheNewObject() {