diff --git a/ILSpy.Tests/Metadata/MetadataFilterRowEndToEndTests.cs b/ILSpy.Tests/Metadata/MetadataFilterRowEndToEndTests.cs index 9496003c7..b11a6270e 100644 --- a/ILSpy.Tests/Metadata/MetadataFilterRowEndToEndTests.cs +++ b/ILSpy.Tests/Metadata/MetadataFilterRowEndToEndTests.cs @@ -17,10 +17,18 @@ // DEALINGS IN THE SOFTWARE. using System.Linq; +using System.Reflection; +using Avalonia; using Avalonia.Controls; +using Avalonia.Controls.Primitives; +using Avalonia.Headless; using Avalonia.Headless.NUnit; +using Avalonia.Input; using Avalonia.LogicalTree; +using Avalonia.Styling; +using Avalonia.Threading; +using Avalonia.VisualTree; using AwesomeAssertions; @@ -40,6 +48,137 @@ public class MetadataFilterRowEndToEndTests public string Name { get; set; } = ""; } + sealed class FlagsRow + { + public int RID { get; set; } + public TypeAttributes Attributes { get; set; } + } + + /// + /// The funnel icon hosting the attached filter flyout for the Attributes column. + /// + static Border FindAttributesFunnel(Visual headerPanel) => + headerPanel.GetVisualDescendants().OfType() + .First(b => ToolTip.GetTip(b) as string == "Filter Attributes"); + + /// + /// Forces the flyout's internal popup into the owner window's overlay layer — the + /// configuration the app actually runs with (Program.cs sets + /// X11PlatformOptions.OverlayPopups = true), where all popup input flows through the + /// owner window's pipeline. The style reaches the popup through the logical tree. + /// + static void ForceOverlayPopups(Window window) => + window.Styles.Add(new Style(x => x.OfType()) { + Setters = { new Setter(Popup.ShouldUseOverlayLayerProperty, true) }, + }); + + [AvaloniaTest] + public void Clicking_Inside_The_Popup_Never_Sorts_The_Column_Of_A_Real_DataGrid() + { + // Full assembly of the real parts: an actual DataGrid with CanUserSortColumns + // (as MetadataTablePage.axaml configures it), the builder's columns, the overlay + // popup host the app runs with, and real input. Opening the popup via the funnel + // and clicking a chip inside it must not raise DataGrid.Sorting. + var page = new MetadataTablePageModel(); + MetadataColumnBuilder.Populate(page); + + var grid = new DataGrid { + ItemsSource = new System.Collections.Generic.List { + new() { RID = 1, Attributes = TypeAttributes.Public }, + new() { RID = 2, Attributes = TypeAttributes.Sealed }, + }, + CanUserSortColumns = true, + }; + foreach (var column in page.Columns) + grid.Columns.Add(column); + var window = new Window { Content = grid, Width = 1000, Height = 600 }; + ForceOverlayPopups(window); + window.Show(); + window.UpdateLayout(); + + int sortingRaised = 0; + grid.Sorting += (_, _) => sortingRaised++; + // An unhandled press reaching a column header is user-visible even without a + // sort: DataGridColumnHeader sets IsPressed and flashes its :pressed background. + // These listeners mirror the header's own subscriptions (bubble, skip handled), + // so any hit here is a press the header would visibly react to. + int headerUnhandledPresses = 0, headerUnhandledReleases = 0; + foreach (var header in grid.GetVisualDescendants().OfType()) + { + header.AddHandler(InputElement.PointerPressedEvent, (_, _) => headerUnhandledPresses++); + header.AddHandler(InputElement.PointerReleasedEvent, (_, _) => headerUnhandledReleases++); + } + + var attributesColumn = page.Columns.Single(c => (string?)c.Tag == "Attributes"); + var headerPanel = (StackPanel)attributesColumn.Header!; + + // Open the flyout with a real click on the funnel icon (the Border carrying the + // "Filter Attributes" tooltip). + var funnel = FindAttributesFunnel(headerPanel); + var flyout = (Flyout)FlyoutBase.GetAttachedFlyout(funnel)!; + var funnelCenter = funnel.TranslatePoint(new Point(funnel.Bounds.Width / 2, funnel.Bounds.Height / 2), window)!.Value; + window.MouseDown(funnelCenter, MouseButton.Left); + window.MouseUp(funnelCenter, MouseButton.Left); + flyout.IsOpen.Should().BeTrue("setup precondition — the funnel click must open the flyout"); + window.UpdateLayout(); + Dispatcher.UIThread.RunJobs(); + sortingRaised.Should().Be(0, "the funnel click that opens the flyout must not sort the column"); + + // Click every interactive control inside the flyout: mutex chips (ToggleButton), + // tri-state pills and Clear (Button), plus the non-interactive hint TextBlock. + // The mode ComboBox goes last because clicking it opens its own dropdown over + // the other controls. + // The flyout body scrolls (MaxHeight 400) and TypeAttributes has more chip groups + // than fit, so restrict the sweep to controls whose center actually lies within + // the flyout's on-screen rect — clicking a scrolled-out control's nominal position + // is a click outside the flyout and legitimately light-dismisses. + var popupContent = (Visual)flyout.Content!; + Rect PopupRect() + { + var topLeft = popupContent.TranslatePoint(default, window)!.Value; + return new Rect(topLeft, popupContent.Bounds.Size); + } + var targets = popupContent.GetVisualDescendants() + .Where(v => v is Button || (v is TextBlock { Text: { } hintText } && hintText.StartsWith("Click a chip"))) + .Concat(popupContent.GetVisualDescendants().Where(v => v is ComboBox)) + .Cast() + .ToList(); + targets.Should().NotBeEmpty("setup precondition — the popup must expose clickable controls"); + int clicked = 0; + foreach (var target in targets) + { + // Recompute at click time: a previous click may have re-flowed the summary + // line and shifted everything below it. + window.UpdateLayout(); + var center = target.TranslatePoint(new Point(target.Bounds.Width / 2, target.Bounds.Height / 2), window); + if (center is not { } point || !PopupRect().Contains(point)) + continue; + clicked++; + window.MouseDown(point, MouseButton.Left); + window.MouseUp(point, MouseButton.Left); + flyout.IsOpen.Should().BeTrue( + $"clicking the {target.GetType().Name} inside the flyout must not close it"); + // ProcessSort is dispatched via Dispatcher.UIThread.Post — drain per click so + // a leaked sort is attributed to the control that caused it. + Dispatcher.UIThread.RunJobs(); + sortingRaised.Should().Be(0, + $"clicking the {target.GetType().Name} inside the popup must not sort the column"); + headerUnhandledPresses.Should().Be(0, + $"clicking the {target.GetType().Name} inside the popup must not deliver an unhandled press to a column header (the header would flash its pressed visual)"); + headerUnhandledReleases.Should().Be(0, + $"clicking the {target.GetType().Name} inside the popup must not deliver an unhandled release to a column header"); + } + clicked.Should().BeGreaterThan(2, "setup precondition — the sweep must actually click several visible controls"); + + // ProcessSort is dispatched via Dispatcher.UIThread.Post — drain the queue so a + // leaked sort would actually fire before the assertion. + Dispatcher.UIThread.RunJobs(); + + sortingRaised.Should().Be(0, + "neither opening the filter popup nor clicking controls inside it may sort the column"); + } + + [AvaloniaTest] public void Typing_Into_The_Header_TextBox_Drives_The_Matching_ColumnFilter_Text() { diff --git a/ILSpy/Metadata/MetadataColumnBuilder.cs b/ILSpy/Metadata/MetadataColumnBuilder.cs index fb68f12c9..818b6b229 100644 --- a/ILSpy/Metadata/MetadataColumnBuilder.cs +++ b/ILSpy/Metadata/MetadataColumnBuilder.cs @@ -135,8 +135,7 @@ namespace ILSpy.Metadata // asks for it — clicking the funnel focuses the textbox, after which it stays // visible while focused or while a filter is set — so brushing the cursor // across the column name does not swap the label out. For [Flags] columns the - // input is just the dropdown trigger; the popup is parked invisibly so it can - // remain anchored to the trigger button. + // funnel is just the trigger for the filter flyout attached to it. var label = new TextBlock { Text = columnName, FontWeight = FontWeight.SemiBold, @@ -182,16 +181,17 @@ namespace ILSpy.Metadata bool isFlagsColumn = propertyType.IsEnum && Attribute.IsDefined(propertyType, typeof(FlagsAttribute)); - Popup? popup = null; + Flyout? flyout = null; TextBox? textBox = null; Control headerContent; if (isFlagsColumn) { // Flags columns: label stays visible always — the funnel icon alone is the - // popup trigger, no separate dropdown chevron needed since the popup carries - // the entire filter UI. PlacementTarget is the funnel itself so the popup + // flyout trigger, no separate dropdown chevron needed since the flyout + // carries the entire filter UI. Attached to the funnel itself so the flyout // drops below the icon's hit area. - popup = BuildFlagsPopup(filter, propertyType, filterIconHost, pageKey, columnName); + flyout = BuildFlagsFlyout(filter, propertyType, pageKey, columnName); + FlyoutBase.SetAttachedFlyout(filterIconHost, flyout); headerContent = label; } else @@ -217,14 +217,14 @@ namespace ILSpy.Metadata headerRow.Children.Add(headerContent); filterIconHost.PointerPressed += (_, e) => { - if (popup != null) + if (flyout != null) { - // Flag columns: the funnel always opens the popup. Modifying the filter - // requires the chip UI inside the popup, and clearing is owned by the - // popup's own Clear button — calling FlagsState.Clear() from out here + // Flag columns: the funnel always opens the flyout. Modifying the filter + // requires the chip UI inside the flyout, and clearing is owned by the + // flyout's own Clear button — calling FlagsState.Clear() from out here // would leave the chip IsChecked state stale because we'd skip the // SyncFromState() pass that FlagsFilterPopup.Clear() does. - popup.IsOpen = true; + FlyoutBase.ShowAttachedFlyout(filterIconHost); } else { @@ -240,13 +240,8 @@ namespace ILSpy.Metadata e.Handled = true; }; - // The popup needs to live inside the visual tree but doesn't contribute layout. - // Park it in a Panel sibling so the header row's height stays driven solely by - // label / input. var root = new StackPanel { Orientation = Orientation.Vertical }; root.Children.Add(headerRow); - if (popup != null) - root.Children.Add(popup); bool popupOpen = false; bool focusInside = false; @@ -269,9 +264,9 @@ namespace ILSpy.Metadata // Icon affordance: the X form indicates "click to clear". That only matches // the click behaviour on text columns (where clicking when active does clear); - // for flag columns the click always opens the popup, so we keep the funnel + // for flag columns the click always opens the flyout, so we keep the funnel // shape and just tint it SteelBlue to signal the filter is active. - bool useXForm = active && popup is null; + bool useXForm = active && flyout is null; if (useXForm) { filterIcon.Data = xGeometry; @@ -296,10 +291,10 @@ namespace ILSpy.Metadata textBox.LostFocus += (_, _) => { focusInside = false; Update(); }; } filter.PropertyChanged += (_, _) => Update(); - if (popup != null) + if (flyout != null) { - popup.Opened += (_, _) => { popupOpen = true; Update(); }; - popup.Closed += (_, _) => { popupOpen = false; Update(); }; + flyout.Opened += (_, _) => { popupOpen = true; Update(); }; + flyout.Closed += (_, _) => { popupOpen = false; Update(); }; } Update(); @@ -331,62 +326,54 @@ namespace ILSpy.Metadata return box; } - static Popup BuildFlagsPopup(ColumnFilter filter, Type enumType, Control placementTarget, string? pageKey, string columnName) + static Flyout BuildFlagsFlyout(ColumnFilter filter, Type enumType, string? pageKey, string columnName) { - // Schema-driven popup: FlagsFilterPopup distinguishes mutex sub-ranges + // Schema-driven flyout: FlagsFilterPopup distinguishes mutex sub-ranges // (multi-select chips) from independent flags (tri-state pills) and drives - // ColumnFilter.FlagsState. The funnel icon owns the open gesture, so this - // helper only assembles the Popup itself — no separate trigger button needed. + // ColumnFilter.FlagsState. A Flyout (not a raw Popup) supplies light dismiss, + // Escape-to-close, focus handling, and theme-correct presenter chrome. var schema = ILSpy.Metadata.Filters.FlagsSchemaInferer.For(enumType); bool freshlyCreated = filter.FlagsState is null; filter.FlagsState ??= new ILSpy.Metadata.Filters.FilterState(schema); - // On first-popup-open, restore any persisted state for this (table, column). + // On first build, restore any persisted state for this (table, column). // Subsequent opens already carry the live state in-memory. Then subscribe so // later mutations write back to SessionSettings. if (freshlyCreated && pageKey != null) ApplyPersistedFilterState(filter.FlagsState, pageKey, columnName); var popupContent = new ILSpy.Views.Filters.FlagsFilterPopup(filter.FlagsState); - var popupRoot = new Border { - BorderBrush = Brushes.Gray, - BorderThickness = new Thickness(1), - Background = Brushes.White, - // Force the arrow cursor on the popup surface — without this, the + var flyoutContent = new ScrollViewer { + MaxHeight = 400, + Content = popupContent, + // Force the arrow cursor on the flyout surface — without this, the // EW-resize cursor set on the DataGrid column header's drag-grip can - // leak into the popup if the pointer enters from there before Avalonia + // leak into the flyout if the pointer enters from there before Avalonia // recomputes the cursor for the new hit-test target. Cursor = new global::Avalonia.Input.Cursor(global::Avalonia.Input.StandardCursorType.Arrow), - Child = new ScrollViewer { - MaxHeight = 400, - Content = popupContent, - }, }; - // Stop wheel events from bubbling out of the popup. Without this, scrolling + // Stop wheel events from bubbling out of the flyout. Without this, scrolling // inside the dropdown also scrolls the underlying DataGrid because the // PointerWheelChanged event keeps bubbling up the routed-event tree once the // inner ScrollViewer has consumed (or ignored) it. - popupRoot.AddHandler(global::Avalonia.Input.InputElement.PointerWheelChangedEvent, + flyoutContent.AddHandler(global::Avalonia.Input.InputElement.PointerWheelChangedEvent, (_, e) => e.Handled = true, handledEventsToo: true); - var popup = new Popup { - PlacementTarget = placementTarget, + // Likewise for pointer clicks: the flyout's internal popup is logically + // parented to the funnel inside the column header, so its routed events bubble + // on into the DataGridColumnHeader, which treats an unhandled left-button + // press/release pair as a sort click. Interactive children (chips, the Clear + // button) handle their own pointer events; swallow whatever reaches the + // content root unhandled — clicks on padding, hint, and summary text. + flyoutContent.AddHandler(global::Avalonia.Input.InputElement.PointerPressedEvent, + (_, e) => e.Handled = true); + flyoutContent.AddHandler(global::Avalonia.Input.InputElement.PointerReleasedEvent, + (_, e) => e.Handled = true); + + return new Flyout { + Content = flyoutContent, Placement = PlacementMode.BottomEdgeAlignedLeft, - IsLightDismissEnabled = true, - Child = popupRoot, + ShowMode = FlyoutShowMode.Transient, }; - // Escape closes the popup — matches the standard popup convention. Tunnel - // routing on the popup root so the key is intercepted before any inner control - // (e.g. the TextBox in a flag-name search field) tries to consume it. - popupRoot.AddHandler(global::Avalonia.Input.InputElement.KeyDownEvent, - (_, e) => { - if (e.Key == global::Avalonia.Input.Key.Escape) - { - popup.IsOpen = false; - e.Handled = true; - } - }, - global::Avalonia.Interactivity.RoutingStrategies.Tunnel); - return popup; } ///