From 3306ed7457bd80ef98ba19d939b79b46bea5d855 Mon Sep 17 00:00:00 2001 From: Philippe Matray Date: Thu, 13 Aug 2026 14:40:30 +0200 Subject: [PATCH 1/5] chore(#300): scaffold draft PR for configureCollection on every item-form builder From 52479691b81d3d761631ae007580bc910c0a4941 Mon Sep 17 00:00:00 2001 From: Philippe Matray Date: Thu, 13 Aug 2026 14:52:51 +0200 Subject: [PATCH 2/5] test(mudblazor): let every item-form builder configure its collection --- .../Fields/CollectionItemFixture.cs | 115 +++++++++++------- .../Fields/CollectionItemFixtureTests.cs | 96 +++++++++++++++ 2 files changed, 166 insertions(+), 45 deletions(-) diff --git a/FormCraft.ForMudBlazor.UnitTests/Fields/CollectionItemFixture.cs b/FormCraft.ForMudBlazor.UnitTests/Fields/CollectionItemFixture.cs index 64c6df00..a90eb38e 100644 --- a/FormCraft.ForMudBlazor.UnitTests/Fields/CollectionItemFixture.cs +++ b/FormCraft.ForMudBlazor.UnitTests/Fields/CollectionItemFixture.cs @@ -118,51 +118,66 @@ internal static NamedOrderModel NewNamedOrder(string name = "", string itemName /// A collection whose item form holds one string field labelled "Product" — the text path. /// internal static IFormConfiguration TextItemForm( - Action>? configure = null) => + Action>? configure = null, + Action>? configureCollection = null) => FormBuilder .Create() - .AddCollectionField(x => x.Items, collection => collection - .WithLabel("Items") - .WithItemForm(item => item - .AddField(x => x.ProductName, field => - { - field.WithLabel("Product"); - configure?.Invoke(field); - }))) + .AddCollectionField(x => x.Items, collection => + { + collection + .WithLabel("Items") + .WithItemForm(item => item + .AddField(x => x.ProductName, field => + { + field.WithLabel("Product"); + configure?.Invoke(field); + })); + configureCollection?.Invoke(collection); + }) .Build(); /// /// A collection whose item form holds one int field labelled "Quantity" — the numeric path. /// internal static IFormConfiguration NumericItemForm( - Action>? configure = null) => + Action>? configure = null, + Action>? configureCollection = null) => FormBuilder .Create() - .AddCollectionField(x => x.Lines, collection => collection - .WithLabel("Lines") - .WithItemForm(item => item - .AddField(x => x.Quantity, field => - { - field.WithLabel("Quantity"); - configure?.Invoke(field); - }))) + .AddCollectionField(x => x.Lines, collection => + { + collection + .WithLabel("Lines") + .WithItemForm(item => item + .AddField(x => x.Quantity, field => + { + field.WithLabel("Quantity"); + configure?.Invoke(field); + })); + configureCollection?.Invoke(collection); + }) .Build(); /// /// A collection whose item form holds one DateTime field labelled "When" — the date path. /// internal static IFormConfiguration DateItemForm( - Action>? configure = null) => + Action>? configure = null, + Action>? configureCollection = null) => FormBuilder .Create() - .AddCollectionField(x => x.Slots, collection => collection - .WithLabel("Slots") - .WithItemForm(item => item - .AddField(x => x.When, field => - { - field.WithLabel("When"); - configure?.Invoke(field); - }))) + .AddCollectionField(x => x.Slots, collection => + { + collection + .WithLabel("Slots") + .WithItemForm(item => item + .AddField(x => x.When, field => + { + field.WithLabel("When"); + configure?.Invoke(field); + })); + configureCollection?.Invoke(collection); + }) .Build(); /// @@ -170,17 +185,22 @@ internal static IFormConfiguration DateItemForm( /// one component that binds neither adornments nor Required. /// internal static IFormConfiguration BooleanItemForm( - Action>? configure = null) => + Action>? configure = null, + Action>? configureCollection = null) => FormBuilder .Create() - .AddCollectionField(x => x.Lines, collection => collection - .WithLabel("Lines") - .WithItemForm(item => item - .AddField(x => x.IsGift, field => - { - field.WithLabel("Gift"); - configure?.Invoke(field); - }))) + .AddCollectionField(x => x.Lines, collection => + { + collection + .WithLabel("Lines") + .WithItemForm(item => item + .AddField(x => x.IsGift, field => + { + field.WithLabel("Gift"); + configure?.Invoke(field); + })); + configureCollection?.Invoke(collection); + }) .Build(); /// @@ -190,17 +210,22 @@ internal static IFormConfiguration BooleanItemForm( /// other; culture-sensitive parsing in particular only shows up on the decimal one. /// internal static IFormConfiguration DecimalItemForm( - Action>? configure = null) => + Action>? configure = null, + Action>? configureCollection = null) => FormBuilder .Create() - .AddCollectionField(x => x.Lines, collection => collection - .WithLabel("Lines") - .WithItemForm(item => item - .AddField(x => x.Price, field => - { - field.WithLabel("Price"); - configure?.Invoke(field); - }))) + .AddCollectionField(x => x.Lines, collection => + { + collection + .WithLabel("Lines") + .WithItemForm(item => item + .AddField(x => x.Price, field => + { + field.WithLabel("Price"); + configure?.Invoke(field); + })); + configureCollection?.Invoke(collection); + }) .Build(); /// diff --git a/FormCraft.ForMudBlazor.UnitTests/Fields/CollectionItemFixtureTests.cs b/FormCraft.ForMudBlazor.UnitTests/Fields/CollectionItemFixtureTests.cs index d70e8bb3..32420ffc 100644 --- a/FormCraft.ForMudBlazor.UnitTests/Fields/CollectionItemFixtureTests.cs +++ b/FormCraft.ForMudBlazor.UnitTests/Fields/CollectionItemFixtureTests.cs @@ -420,4 +420,100 @@ public void NewAppointment_Should_Default_Blank_And_Honour_Its_Seed() blank.Slots[0].When.ShouldBe(default); seeded.Slots[0].When.ShouldBe(new DateTime(2030, 12, 31)); } + + [Fact] + public void TextItemForm_Should_Apply_The_Callers_Collection_Configuration() + { + // Arrange & Act - AllowReorder is a property of the COLLECTION, not of a field, so no field + // callback can reach it. Without this parameter a suite needing a collection-level setting + // has to hand-roll the whole configuration, and keeps its own model copy with it — the + // duplication this fixture exists to remove. + var reorderable = this.RenderItemForm( + CollectionItemFixture.NewOrder(), + CollectionItemFixture.TextItemForm( + configureCollection: collection => collection.AllowReorder())); + + // ...and the same form WITHOUT the callback, which is the half that makes this a test of the + // callback rather than of MudBlazor. Asserting only that the buttons appear would stay green + // if reordering ever became the default, or if a regression rendered the controls + // unconditionally — proving nothing about the parameter the test is named for. + var plain = this.RenderItemForm( + CollectionItemFixture.NewOrder(), + CollectionItemFixture.TextItemForm()); + + // Assert - reorder controls only render when the collection allows reordering + reorderable.FindAll("button[aria-label='Move up']").ShouldNotBeEmpty(); + plain.FindAll("button[aria-label='Move up']").ShouldBeEmpty(); + } + + [Fact] + public void NumericItemForm_Should_Apply_The_Callers_Collection_Configuration() + { + // Arrange & Act - NumericItemForm and BooleanItemForm configure the SAME collection + // (BasketModel.Lines) on the same root model, so a passing test for one says nothing about + // the other: each builder wires its own callback and each needs its own proof. + var reorderable = this.RenderItemForm( + CollectionItemFixture.NewBasket(), + CollectionItemFixture.NumericItemForm( + configureCollection: collection => collection.AllowReorder())); + var plain = this.RenderItemForm( + CollectionItemFixture.NewBasket(), + CollectionItemFixture.NumericItemForm()); + + // Assert - reorder controls only render when the collection allows reordering + reorderable.FindAll("button[aria-label='Move up']").ShouldNotBeEmpty(); + plain.FindAll("button[aria-label='Move up']").ShouldBeEmpty(); + } + + [Fact] + public void DateItemForm_Should_Apply_The_Callers_Collection_Configuration() + { + // Arrange & Act + var reorderable = this.RenderItemForm( + CollectionItemFixture.NewAppointment(), + CollectionItemFixture.DateItemForm( + configureCollection: collection => collection.AllowReorder())); + var plain = this.RenderItemForm( + CollectionItemFixture.NewAppointment(), + CollectionItemFixture.DateItemForm()); + + // Assert - reorder controls only render when the collection allows reordering + reorderable.FindAll("button[aria-label='Move up']").ShouldNotBeEmpty(); + plain.FindAll("button[aria-label='Move up']").ShouldBeEmpty(); + } + + [Fact] + public void BooleanItemForm_Should_Apply_The_Callers_Collection_Configuration() + { + // Arrange & Act - the other half of the BasketModel.Lines pair; see NumericItemForm's + // self-test for why sharing a collection property does not let one stand in for the other. + var reorderable = this.RenderItemForm( + CollectionItemFixture.NewBasket(), + CollectionItemFixture.BooleanItemForm( + configureCollection: collection => collection.AllowReorder())); + var plain = this.RenderItemForm( + CollectionItemFixture.NewBasket(), + CollectionItemFixture.BooleanItemForm()); + + // Assert - reorder controls only render when the collection allows reordering + reorderable.FindAll("button[aria-label='Move up']").ShouldNotBeEmpty(); + plain.FindAll("button[aria-label='Move up']").ShouldBeEmpty(); + } + + [Fact] + public void DecimalItemForm_Should_Apply_The_Callers_Collection_Configuration() + { + // Arrange & Act + var reorderable = this.RenderItemForm( + CollectionItemFixture.NewPricedBasket(), + CollectionItemFixture.DecimalItemForm( + configureCollection: collection => collection.AllowReorder())); + var plain = this.RenderItemForm( + CollectionItemFixture.NewPricedBasket(), + CollectionItemFixture.DecimalItemForm()); + + // Assert - reorder controls only render when the collection allows reordering + reorderable.FindAll("button[aria-label='Move up']").ShouldNotBeEmpty(); + plain.FindAll("button[aria-label='Move up']").ShouldBeEmpty(); + } } From 50655dc51789b102b1b7cb9298c9f37933f30b65 Mon Sep 17 00:00:00 2001 From: Philippe Matray Date: Thu, 13 Aug 2026 15:14:11 +0200 Subject: [PATCH 3/5] test(mudblazor): give the root-field builder a collection callback --- .../Fields/CollectionItemFixture.cs | 23 +++++++------ .../Fields/CollectionItemFixtureTests.cs | 32 +++++++++++++++++++ 2 files changed, 46 insertions(+), 9 deletions(-) diff --git a/FormCraft.ForMudBlazor.UnitTests/Fields/CollectionItemFixture.cs b/FormCraft.ForMudBlazor.UnitTests/Fields/CollectionItemFixture.cs index a90eb38e..504d0893 100644 --- a/FormCraft.ForMudBlazor.UnitTests/Fields/CollectionItemFixture.cs +++ b/FormCraft.ForMudBlazor.UnitTests/Fields/CollectionItemFixture.cs @@ -347,7 +347,8 @@ internal static IFormConfiguration MultiFieldItemForm( /// internal static IFormConfiguration RootFieldAndItemForm( Action>? configureRoot = null, - Action>? configureItem = null) => + Action>? configureItem = null, + Action>? configureCollection = null) => FormBuilder .Create() .AddField(x => x.Name, field => @@ -355,14 +356,18 @@ internal static IFormConfiguration RootFieldAndItemForm( field.WithLabel("Name"); configureRoot?.Invoke(field); }) - .AddCollectionField(x => x.Items, collection => collection - .WithLabel("Items") - .WithItemForm(item => item - .AddField(x => x.Name, field => - { - field.WithLabel("Item name"); - configureItem?.Invoke(field); - }))) + .AddCollectionField(x => x.Items, collection => + { + collection + .WithLabel("Items") + .WithItemForm(item => item + .AddField(x => x.Name, field => + { + field.WithLabel("Item name"); + configureItem?.Invoke(field); + })); + configureCollection?.Invoke(collection); + }) .Build(); } diff --git a/FormCraft.ForMudBlazor.UnitTests/Fields/CollectionItemFixtureTests.cs b/FormCraft.ForMudBlazor.UnitTests/Fields/CollectionItemFixtureTests.cs index 32420ffc..7a9ee5ad 100644 --- a/FormCraft.ForMudBlazor.UnitTests/Fields/CollectionItemFixtureTests.cs +++ b/FormCraft.ForMudBlazor.UnitTests/Fields/CollectionItemFixtureTests.cs @@ -516,4 +516,36 @@ public void DecimalItemForm_Should_Apply_The_Callers_Collection_Configuration() reorderable.FindAll("button[aria-label='Move up']").ShouldNotBeEmpty(); plain.FindAll("button[aria-label='Move up']").ShouldBeEmpty(); } + + [Fact] + public void RootFieldAndItemForm_Should_Apply_The_Callers_Collection_Configuration() + { + // Arrange & Act - three callbacks now, two of which target fields and one the collection. + // A mis-wire is easy in that shape, so this asserts the new one reaches the collection AND + // that the two existing ones still reach their own fields — a builder that routed the root + // callback into the collection would otherwise pass a test that only looked at reordering. + var reorderable = this.RenderItemForm( + CollectionItemFixture.NewNamedOrder(), + CollectionItemFixture.RootFieldAndItemForm( + root => root.WithLabel("Customer name"), + item => item.WithLabel("Product name"), + collection => collection.AllowReorder())); + + // ...and the same form WITHOUT the collection callback: the negative half, which is what + // makes this a test of the parameter rather than of MudBlazor's defaults. + var plain = this.RenderItemForm( + CollectionItemFixture.NewNamedOrder(), + CollectionItemFixture.RootFieldAndItemForm( + root => root.WithLabel("Customer name"), + item => item.WithLabel("Product name"))); + + // Assert - the collection callback reaches the collection... + reorderable.FindAll("button[aria-label='Move up']").ShouldNotBeEmpty(); + plain.FindAll("button[aria-label='Move up']").ShouldBeEmpty(); + + // ...and the field callbacks still reach their own fields + var fields = reorderable.FindComponents>(); + fields[0].Instance.Label.ShouldBe("Customer name"); + fields[1].Instance.Label.ShouldBe("Product name"); + } } From cef80da76de2384fbdac8f0d351edd3831b2b3c4 Mon Sep 17 00:00:00 2001 From: Philippe Matray Date: Thu, 13 Aug 2026 15:19:05 +0200 Subject: [PATCH 4/5] test(mudblazor): build the characterisation suite's order form from the fixture --- .../CollectionRenderCharacterisationTests.cs | 47 ++++++++++--------- 1 file changed, 24 insertions(+), 23 deletions(-) diff --git a/FormCraft.ForMudBlazor.UnitTests/Components/CollectionRenderCharacterisationTests.cs b/FormCraft.ForMudBlazor.UnitTests/Components/CollectionRenderCharacterisationTests.cs index 958eee1f..867bda74 100644 --- a/FormCraft.ForMudBlazor.UnitTests/Components/CollectionRenderCharacterisationTests.cs +++ b/FormCraft.ForMudBlazor.UnitTests/Components/CollectionRenderCharacterisationTests.cs @@ -371,30 +371,31 @@ private IRenderedComponent> RenderOrderForm( bool allowAdd = false, bool allowRemove = false) { - var config = FormBuilder - .Create() - .AddCollectionField(x => x.Items, collection => + // The label and the one-string-field item form are byte-identical to TextItemForm's, so the + // only thing this suite varies is the three flags — and configureCollection (#300) is how a + // caller reaches settings that belong to the collection rather than to a field. + // + // They stay CONDITIONAL. Applying them unconditionally would render an Add button on every + // call, and the sibling tests locate it with FindComponent(), which throws when + // it is absent and matches a newly-rendered one when it is not expected — so the flags are + // load-bearing for what those tests assert, not just for what this helper builds. + var config = TextItemForm(configureCollection: collection => + { + if (allowReorder) + { + collection.AllowReorder(); + } + + if (allowAdd) { - collection.WithLabel("Items"); - if (allowReorder) - { - collection.AllowReorder(); - } - - if (allowAdd) - { - collection.AllowAdd(); - } - - if (allowRemove) - { - collection.AllowRemove(); - } - - collection.WithItemForm(item => item - .AddField(x => x.ProductName, field => field.WithLabel("Product"))); - }) - .Build(); + collection.AllowAdd(); + } + + if (allowRemove) + { + collection.AllowRemove(); + } + }); return Render>(parameters => parameters .Add(p => p.Model, model) From 910723df41b642c4efa0279b5994a11af0965576 Mon Sep 17 00:00:00 2001 From: Philippe Matray Date: Thu, 13 Aug 2026 15:43:55 +0200 Subject: [PATCH 5/5] test(mudblazor): pin the collection callback's invoke order and document it --- .../CollectionRenderCharacterisationTests.cs | 7 +- .../Fields/CollectionItemFixture.cs | 65 ++++- .../Fields/CollectionItemFixtureTests.cs | 230 +++++++++++------- 3 files changed, 206 insertions(+), 96 deletions(-) diff --git a/FormCraft.ForMudBlazor.UnitTests/Components/CollectionRenderCharacterisationTests.cs b/FormCraft.ForMudBlazor.UnitTests/Components/CollectionRenderCharacterisationTests.cs index 867bda74..376ea3b1 100644 --- a/FormCraft.ForMudBlazor.UnitTests/Components/CollectionRenderCharacterisationTests.cs +++ b/FormCraft.ForMudBlazor.UnitTests/Components/CollectionRenderCharacterisationTests.cs @@ -397,9 +397,10 @@ private IRenderedComponent> RenderOrderForm( } }); - return Render>(parameters => parameters - .Add(p => p.Model, model) - .Add(p => p.Configuration, config)); + // ...and the render goes through the fixture's own helper too, which is the rest of "built + // from the fixture": this was the last hand-rolled Model/Configuration wiring in a suite + // that already calls RenderItemForm everywhere else. + return this.RenderItemForm(model, config); } /// diff --git a/FormCraft.ForMudBlazor.UnitTests/Fields/CollectionItemFixture.cs b/FormCraft.ForMudBlazor.UnitTests/Fields/CollectionItemFixture.cs index fe42a85f..2f6a3569 100644 --- a/FormCraft.ForMudBlazor.UnitTests/Fields/CollectionItemFixture.cs +++ b/FormCraft.ForMudBlazor.UnitTests/Fields/CollectionItemFixture.cs @@ -67,8 +67,24 @@ namespace FormCraft.ForMudBlazor.UnitTests.Fields; /// /// /// Models are deliberately dumb and stable — no behaviour, no validation attributes, no computed -/// members. Everything a suite wants to vary goes through the configure callback on the item-form -/// builders instead, so a change here cannot ripple into what an unrelated suite asserts. +/// members. Everything a suite wants to vary goes through a callback on the item-form builders +/// instead, so a change here cannot ripple into what an unrelated suite asserts. +/// +/// +/// There are two such callbacks on every builder, and the split is by target rather than by +/// convenience: configure (or the per-field configureText/configureNumeric/… on +/// ) reaches a field, while configureCollection reaches +/// the collection — AllowReorder, AllowAdd, WithMinItems and the rest, +/// which no field callback can express. Until #300 only had the +/// latter, so a suite needing a collection-level setting on a single-field item form hand-rolled the +/// whole configuration; the parameter is now uniform across every builder here. +/// +/// +/// Both callbacks run after the builder has applied its own label and item form, so a caller +/// can override those. That ordering is the contract, and it is pinned by +/// CollectionItemFixtureTests.Each_Item_Forms_Collection_Callback_Should_Run_After_The_Fixtures_Own_Configuration +/// — a reorder-only test cannot catch a builder that invokes the callback first, because +/// AllowReorder is an independent setter that reads the same either way. /// /// internal static class CollectionItemFixture @@ -121,6 +137,12 @@ internal static NamedOrderModel NewNamedOrder(string name = "", string itemName /// /// A collection whose item form holds one string field labelled "Product" — the text path. /// + /// Configures the item's field. Runs after its default label, so it can override it. + /// + /// Configures the collection — AllowReorder, AllowAdd, WithMinItems + /// and the rest, none of which a field callback can reach. Runs after this builder has set the + /// label and the item form, so it can override those too (#300). + /// internal static IFormConfiguration TextItemForm( Action>? configure = null, Action>? configureCollection = null) => @@ -143,6 +165,11 @@ internal static IFormConfiguration TextItemForm( /// /// A collection whose item form holds one int field labelled "Quantity" — the numeric path. /// + /// Configures the item's field. Runs after its default label, so it can override it. + /// + /// Configures the collection rather than a field; runs last, after the label and item + /// form. See for the full contract (#300). + /// internal static IFormConfiguration NumericItemForm( Action>? configure = null, Action>? configureCollection = null) => @@ -165,6 +192,11 @@ internal static IFormConfiguration NumericItemForm( /// /// A collection whose item form holds one DateTime field labelled "When" — the date path. /// + /// Configures the item's field. Runs after its default label, so it can override it. + /// + /// Configures the collection rather than a field; runs last, after the label and item + /// form. See for the full contract (#300). + /// internal static IFormConfiguration DateItemForm( Action>? configure = null, Action>? configureCollection = null) => @@ -188,6 +220,12 @@ internal static IFormConfiguration DateItemForm( /// A collection whose item form holds one bool field labelled "Gift" — the checkbox, the /// one component that binds neither adornments nor Required. /// + /// Configures the item's field. Runs after its default label, so it can override it. + /// + /// Configures the collection rather than a field; runs last, after the label and item + /// form. Distinct from 's despite both configuring + /// BasketModel.Lines — separate builders wire separate callbacks (#300). + /// internal static IFormConfiguration BooleanItemForm( Action>? configure = null, Action>? configureCollection = null) => @@ -213,6 +251,11 @@ internal static IFormConfiguration BooleanItemForm( /// MudNumericField<int>, so a suite that asserts on one has said nothing about the /// other; culture-sensitive parsing in particular only shows up on the decimal one. /// + /// Configures the item's field. Runs after its default label, so it can override it. + /// + /// Configures the collection rather than a field; runs last, after the label and item + /// form. See for the full contract (#300). + /// internal static IFormConfiguration DecimalItemForm( Action>? configure = null, Action>? configureCollection = null) => @@ -285,11 +328,12 @@ internal static MixedItemModel NewMixedItems(params MixedItem[] rows) /// those two; the others still render, which is what makes the row mixed. /// /// - /// is the one callback here that reaches the - /// collection rather than a field. The reorder test needs .AllowReorder(), which is - /// a property of the collection, and without it that suite would have to hand-roll the whole - /// configuration — and would keep its own model copy along with it, which is the duplication - /// this member exists to remove. + /// is the callback that reaches the collection + /// rather than a field: the reorder test needs .AllowReorder(), a property of the + /// collection, and without it that suite would have to hand-roll the whole configuration — and + /// would keep its own model copy along with it, the duplication this member exists to remove. + /// This member had it first (#282); #300 gave every other builder here the same parameter, so it + /// is no longer special — see the class summary for the ordering contract it shares with them. /// /// internal static IFormConfiguration MultiFieldItemForm( @@ -416,6 +460,13 @@ internal static IFormConfiguration TwoCollectionItemForm( /// labels would buy nothing and would leave a caller unable to tell the root field from the item /// field in a rendered form. /// + /// Configures the top-level field. Runs after its default label. + /// Configures the item's field. Runs after its default label. + /// + /// Configures the collection rather than either field; appended last so every existing + /// call site, positional or named, still compiles. See for the full + /// contract (#300). + /// internal static IFormConfiguration RootFieldAndItemForm( Action>? configureRoot = null, Action>? configureItem = null, diff --git a/FormCraft.ForMudBlazor.UnitTests/Fields/CollectionItemFixtureTests.cs b/FormCraft.ForMudBlazor.UnitTests/Fields/CollectionItemFixtureTests.cs index 7e52a0b8..cd82c2f3 100644 --- a/FormCraft.ForMudBlazor.UnitTests/Fields/CollectionItemFixtureTests.cs +++ b/FormCraft.ForMudBlazor.UnitTests/Fields/CollectionItemFixtureTests.cs @@ -484,99 +484,128 @@ public void NewAppointment_Should_Default_Blank_And_Honour_Its_Seed() } [Fact] - public void TextItemForm_Should_Apply_The_Callers_Collection_Configuration() + public void Each_Item_Form_Should_Apply_The_Callers_Collection_Configuration() { - // Arrange & Act - AllowReorder is a property of the COLLECTION, not of a field, so no field - // callback can reach it. Without this parameter a suite needing a collection-level setting - // has to hand-roll the whole configuration, and keeps its own model copy with it — the - // duplication this fixture exists to remove. - var reorderable = this.RenderItemForm( - CollectionItemFixture.NewOrder(), + // Arrange & Act - AllowReorder belongs to the COLLECTION, not to a field, so no field + // callback can reach it. Until #300 only MultiFieldItemForm took a collection callback, and + // any suite needing one on a single-field item form had to hand-roll the whole + // configuration — keeping its own model copy with it, which is the duplication this fixture + // exists to remove. + // + // Every model below carries TWO rows deliberately. CollectionFieldComponent renders Move up + // with Disabled="@(index == 0)", so a one-row collection renders the control permanently + // inert: a mere "the button exists" assertion would hold for a form on which reordering + // cannot actually happen. + var text = this.RenderItemForm( + CollectionItemFixture.NewOrderWithItems("A", "B"), CollectionItemFixture.TextItemForm( configureCollection: collection => collection.AllowReorder())); - - // ...and the same form WITHOUT the callback, which is the half that makes this a test of the - // callback rather than of MudBlazor. Asserting only that the buttons appear would stay green - // if reordering ever became the default, or if a regression rendered the controls - // unconditionally — proving nothing about the parameter the test is named for. - var plain = this.RenderItemForm( - CollectionItemFixture.NewOrder(), - CollectionItemFixture.TextItemForm()); - - // Assert - reorder controls only render when the collection allows reordering - reorderable.FindAll("button[aria-label='Move up']").ShouldNotBeEmpty(); - plain.FindAll("button[aria-label='Move up']").ShouldBeEmpty(); - } - - [Fact] - public void NumericItemForm_Should_Apply_The_Callers_Collection_Configuration() - { - // Arrange & Act - NumericItemForm and BooleanItemForm configure the SAME collection - // (BasketModel.Lines) on the same root model, so a passing test for one says nothing about - // the other: each builder wires its own callback and each needs its own proof. - var reorderable = this.RenderItemForm( - CollectionItemFixture.NewBasket(), + var numeric = this.RenderItemForm( + new BasketModel { Lines = { new BasketLine(), new BasketLine() } }, CollectionItemFixture.NumericItemForm( configureCollection: collection => collection.AllowReorder())); - var plain = this.RenderItemForm( - CollectionItemFixture.NewBasket(), - CollectionItemFixture.NumericItemForm()); - - // Assert - reorder controls only render when the collection allows reordering - reorderable.FindAll("button[aria-label='Move up']").ShouldNotBeEmpty(); - plain.FindAll("button[aria-label='Move up']").ShouldBeEmpty(); - } - - [Fact] - public void DateItemForm_Should_Apply_The_Callers_Collection_Configuration() - { - // Arrange & Act - var reorderable = this.RenderItemForm( - CollectionItemFixture.NewAppointment(), + var date = this.RenderItemForm( + new AppointmentModel { Slots = { new AppointmentSlot(), new AppointmentSlot() } }, CollectionItemFixture.DateItemForm( configureCollection: collection => collection.AllowReorder())); - var plain = this.RenderItemForm( - CollectionItemFixture.NewAppointment(), - CollectionItemFixture.DateItemForm()); - - // Assert - reorder controls only render when the collection allows reordering - reorderable.FindAll("button[aria-label='Move up']").ShouldNotBeEmpty(); - plain.FindAll("button[aria-label='Move up']").ShouldBeEmpty(); - } - - [Fact] - public void BooleanItemForm_Should_Apply_The_Callers_Collection_Configuration() - { - // Arrange & Act - the other half of the BasketModel.Lines pair; see NumericItemForm's - // self-test for why sharing a collection property does not let one stand in for the other. - var reorderable = this.RenderItemForm( - CollectionItemFixture.NewBasket(), + var boolean = this.RenderItemForm( + new BasketModel { Lines = { new BasketLine(), new BasketLine() } }, CollectionItemFixture.BooleanItemForm( configureCollection: collection => collection.AllowReorder())); - var plain = this.RenderItemForm( - CollectionItemFixture.NewBasket(), + var dec = this.RenderItemForm( + new PricedBasketModel { Lines = { new PricedLine(), new PricedLine() } }, + CollectionItemFixture.DecimalItemForm( + configureCollection: collection => collection.AllowReorder())); + + // ...and each form WITHOUT the callback, the half that makes this a test of the callback + // rather than of MudBlazor. Asserting only that the controls appear would stay green if + // reordering ever became the default, or if a regression rendered them unconditionally — + // proving nothing about the parameter the test is named for. + // + // Numeric and Boolean are both here even though they configure the SAME collection + // (BasketModel.Lines): they are separate builders wiring separate callbacks, so one passing + // says nothing about the other. + var plainText = this.RenderItemForm( + CollectionItemFixture.NewOrderWithItems("A", "B"), + CollectionItemFixture.TextItemForm()); + var plainNumeric = this.RenderItemForm( + new BasketModel { Lines = { new BasketLine(), new BasketLine() } }, + CollectionItemFixture.NumericItemForm()); + var plainDate = this.RenderItemForm( + new AppointmentModel { Slots = { new AppointmentSlot(), new AppointmentSlot() } }, + CollectionItemFixture.DateItemForm()); + var plainBoolean = this.RenderItemForm( + new BasketModel { Lines = { new BasketLine(), new BasketLine() } }, CollectionItemFixture.BooleanItemForm()); + var plainDecimal = this.RenderItemForm( + new PricedBasketModel { Lines = { new PricedLine(), new PricedLine() } }, + CollectionItemFixture.DecimalItemForm()); - // Assert - reorder controls only render when the collection allows reordering - reorderable.FindAll("button[aria-label='Move up']").ShouldNotBeEmpty(); - plain.FindAll("button[aria-label='Move up']").ShouldBeEmpty(); + // Assert - usable reorder controls, and only where the caller asked for them + ShouldOfferReordering(text); + ShouldOfferReordering(numeric); + ShouldOfferReordering(date); + ShouldOfferReordering(boolean); + ShouldOfferReordering(dec); + + ShouldNotOfferReordering(plainText); + ShouldNotOfferReordering(plainNumeric); + ShouldNotOfferReordering(plainDate); + ShouldNotOfferReordering(plainBoolean); + ShouldNotOfferReordering(plainDecimal); } [Fact] - public void DecimalItemForm_Should_Apply_The_Callers_Collection_Configuration() + public void Each_Item_Forms_Collection_Callback_Should_Run_After_The_Fixtures_Own_Configuration() { - // Arrange & Act - var reorderable = this.RenderItemForm( - CollectionItemFixture.NewPricedBasket(), - CollectionItemFixture.DecimalItemForm( - configureCollection: collection => collection.AllowReorder())); - var plain = this.RenderItemForm( - CollectionItemFixture.NewPricedBasket(), - CollectionItemFixture.DecimalItemForm()); - - // Assert - reorder controls only render when the collection allows reordering - reorderable.FindAll("button[aria-label='Move up']").ShouldNotBeEmpty(); - plain.FindAll("button[aria-label='Move up']").ShouldBeEmpty(); + // Arrange & Act - the invoke ORDER is half the contract, and AllowReorder cannot pin it: it + // is an independent setter that reads the same whether the caller's callback runs before or + // after the fixture's own WithLabel/WithItemForm. Overriding the label can only succeed if + // the caller runs LAST — the same property Each_Item_Form_Should_Apply_The_Callers_ + // Configuration pins for the field callback ("the callback runs after the default label, so + // it can override it"). Without this test all six builders could invoke the callback first + // and every other assertion here would stay green. + var forms = new IRenderedComponent[] + { + this.RenderItemForm( + CollectionItemFixture.NewOrder(), + CollectionItemFixture.TextItemForm( + configureCollection: collection => collection.WithLabel("Renamed"))), + this.RenderItemForm( + CollectionItemFixture.NewBasket(), + CollectionItemFixture.NumericItemForm( + configureCollection: collection => collection.WithLabel("Renamed"))), + this.RenderItemForm( + CollectionItemFixture.NewAppointment(), + CollectionItemFixture.DateItemForm( + configureCollection: collection => collection.WithLabel("Renamed"))), + this.RenderItemForm( + CollectionItemFixture.NewBasket(), + CollectionItemFixture.BooleanItemForm( + configureCollection: collection => collection.WithLabel("Renamed"))), + this.RenderItemForm( + CollectionItemFixture.NewPricedBasket(), + CollectionItemFixture.DecimalItemForm( + configureCollection: collection => collection.WithLabel("Renamed"))), + this.RenderItemForm( + CollectionItemFixture.NewNamedOrder(), + CollectionItemFixture.RootFieldAndItemForm( + configureCollection: collection => collection.WithLabel("Renamed"))), + + // MultiFieldItemForm has carried the parameter since #282; asserting it here is what + // makes the ordering uniform across all SEVEN builders rather than the six #300 touched. + this.RenderItemForm( + CollectionItemFixture.NewMixedItems(new MixedItem()), + CollectionItemFixture.MultiFieldItemForm( + configureCollection: collection => collection.WithLabel("Renamed"))), + }; + + // Assert - the caller's label won, so its callback ran after the fixture set its own. + // The collection's label renders as the header MudText (Typo.h6). + foreach (var form in forms) + { + form.Find("h6").TextContent.Trim().ShouldBe("Renamed"); + } } [Fact] @@ -587,7 +616,7 @@ public void RootFieldAndItemForm_Should_Apply_The_Callers_Collection_Configurati // that the two existing ones still reach their own fields — a builder that routed the root // callback into the collection would otherwise pass a test that only looked at reordering. var reorderable = this.RenderItemForm( - CollectionItemFixture.NewNamedOrder(), + new NamedOrderModel { Items = { new NamedOrderItem(), new NamedOrderItem() } }, CollectionItemFixture.RootFieldAndItemForm( root => root.WithLabel("Customer name"), item => item.WithLabel("Product name"), @@ -596,18 +625,47 @@ public void RootFieldAndItemForm_Should_Apply_The_Callers_Collection_Configurati // ...and the same form WITHOUT the collection callback: the negative half, which is what // makes this a test of the parameter rather than of MudBlazor's defaults. var plain = this.RenderItemForm( - CollectionItemFixture.NewNamedOrder(), - CollectionItemFixture.RootFieldAndItemForm( - root => root.WithLabel("Customer name"), - item => item.WithLabel("Product name"))); + new NamedOrderModel { Items = { new NamedOrderItem(), new NamedOrderItem() } }, + CollectionItemFixture.RootFieldAndItemForm()); // Assert - the collection callback reaches the collection... - reorderable.FindAll("button[aria-label='Move up']").ShouldNotBeEmpty(); - plain.FindAll("button[aria-label='Move up']").ShouldBeEmpty(); + ShouldOfferReordering(reorderable); + ShouldNotOfferReordering(plain); - // ...and the field callbacks still reach their own fields + // ...and adding a third callback did not re-route the two that were already there var fields = reorderable.FindComponents>(); fields[0].Instance.Label.ShouldBe("Customer name"); fields[1].Instance.Label.ShouldBe("Product name"); } + + /// + /// Asserts that a rendered collection offers usable reorder controls — the observable + /// effect of AllowReorder(), and therefore the proof that a builder's collection callback + /// reached the collection. + /// + /// + /// The enabled-ness check is the point. CollectionFieldComponent disables Move up on row + /// 0 and Move down on the last row, so on a one-row collection both controls render and both are + /// permanently inert — "a Move up button exists" is true of a form that cannot be reordered. + /// Callers therefore pass two-row models, and this asserts one control per row with the second + /// one live. + /// + private static void ShouldOfferReordering( + IRenderedComponent> component) + where TModel : new() + { + var moveUp = component.FindAll("button[aria-label='Move up']"); + moveUp.Count.ShouldBe(2); + moveUp[0].HasAttribute("disabled").ShouldBeTrue(); + moveUp[1].HasAttribute("disabled").ShouldBeFalse(); + } + + /// + /// The negative control for : no callback, so the + /// collection never allows reordering and the controls are absent rather than merely disabled. + /// + private static void ShouldNotOfferReordering( + IRenderedComponent> component) + where TModel : new() + => component.FindAll("button[aria-label='Move up']").ShouldBeEmpty(); }