feat(EditorForm): add EditorFormGroupType parameter #7612
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
Reviewer's GuideAdds selectable group layout support to EditorForm so it can render traditional GroupBox or new row-header styling, updates samples and styles to showcase it, and covers the behavior with unit tests and localization keys. Sequence diagram for switching EditorForm group layoutsequenceDiagram
actor User
participant Browser
participant EditorForms
participant EditorForm_TModel_ as EditorForm<TModel>
User->>Browser: Click Segmented item (GroupBox or RowHeader)
Browser->>EditorForms: Trigger change event with selected EditorFormGroupType
EditorForms->>EditorForms: Update _groupType
EditorForms->>EditorForm_TModel_: Render EditorForm GroupType=_groupType
EditorForm_TModel_->>EditorForm_TModel_: Build ClassString
EditorForm_TModel_->>Browser: Render with .bb-editor-group-row-header when GroupType==RowHeader
Browser->>User: Display grouped fields in selected layout
Class diagram for new EditorFormGroupType integrationclassDiagram
direction LR
class EditorFormGroupType {
<<enumeration>>
GroupBox
RowHeader
}
class EditorForm_TModel_ {
+EditorFormGroupType GroupType
-string ClassString
}
class EditorForms {
-EditorFormGroupType _groupType
}
EditorForm_TModel_ ..> EditorFormGroupType : uses
EditorForms ..> EditorFormGroupType : uses
EditorForms ..> EditorForm_TModel_ : provides_GroupType
class GroupBox {
+object? AdditionalAttributes
+string ClassString
+object? ChildContent
+string? Title
}
class EditorFormStyles {
<<stylesheet>>
+.bb-editor
+.bb-editor-group-row-header
}
EditorForm_TModel_ ..> EditorFormStyles : mapped_by_ClassString
GroupBox ..> EditorFormStyles : uses_legend_class
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7612 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 749 749
Lines 33005 33007 +2
Branches 4580 4580
=========================================
+ Hits 33005 33007 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds a GroupType parameter to the EditorForm component, allowing developers to choose between two display styles for grouped form items: GroupBox (default) and RowHeader. The RowHeader style displays groups with a vertical header orientation and a vertical divider line.
Changes:
- Added new
EditorFormGroupTypeenum withGroupBoxandRowHeaderoptions - Added
GroupTypeparameter to EditorForm component to control group display style - Changed GroupBox component's legend element from
<label>to<span>for better semantic HTML - Added SCSS styling for the RowHeader display mode
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/BootstrapBlazor/Enums/EditorFormGroupType.cs | New enum defining the two group display types (GroupBox and RowHeader) |
| src/BootstrapBlazor/Components/EditorForm/EditorForm.razor.cs | Added GroupType parameter and conditional CSS class application |
| src/BootstrapBlazor/Components/GroupBox/GroupBox.razor | Changed legend element from label to span for semantic correctness |
| src/BootstrapBlazor/Components/EditorForm/EditorForm.razor.scss | Added styling for RowHeader mode with vertical text and divider |
| test/UnitTest/Components/EditorFormTest.cs | Added test to verify GroupType parameter behavior |
| src/BootstrapBlazor.Server/Components/Samples/EditorForms.razor | Demo showcasing the new GroupType feature with toggle capability |
| src/BootstrapBlazor.Server/Components/Samples/EditorForms.razor.cs | Added _groupType field to support the demo |
| src/BootstrapBlazor.Server/Locales/en-US.json | Added English localization strings for the new feature |
| src/BootstrapBlazor.Server/Locales/zh-CN.json | Added Chinese localization strings for the new feature |
| src/BootstrapBlazor/BootstrapBlazor.csproj | Version bump from beta02 to beta03 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| </Segmented> | ||
| </BootstrapInputGroup> | ||
| </section> | ||
| <EditorForm Model="@Model" GroupType="_groupType"> |
There was a problem hiding this comment.
Missing @ symbol before the variable reference. The GroupType parameter binding should be GroupType="@_groupType" instead of GroupType="_groupType". Without the @ symbol, the component will receive the literal string "_groupType" rather than the value of the variable.
Suggested change
| <EditorForm Model="@Model" GroupType="_groupType"> | |
| <EditorForm Model="@Model" GroupType="@_groupType"> |
| "EditorFormTips4": "The buttons in the form can be set up multiplely, just set the buttons <code>Buttons</code> template", | ||
| "GroupBoxTitle": "An example of a form", | ||
| "GroupDescription": "Grouping is enabled by setting the <code>GroupName</code> parameter of the <code>EditorItem</code> component, and the order is controlled by <code>GroupOrder</code>.", | ||
| "GroupIntro": "The grouping format can be controlled by setting the <code>groupType</code> value.", |
There was a problem hiding this comment.
Inconsistent casing for parameter name. The text uses "groupType" (lowercase 'g') but should be "GroupType" (uppercase 'G') to match the parameter name. Other parameter references in the localization files use proper casing (e.g., "RowType" at line 3733).
Suggested change
| "GroupIntro": "The grouping format can be controlled by setting the <code>groupType</code> value.", | |
| "GroupIntro": "The grouping format can be controlled by setting the <code>GroupType</code> value.", |
| private IStringLocalizer<Foo>? FooLocalizer { get; set; } | ||
|
|
||
| private List<string> _ignoreItems = []; | ||
| private EditorFormGroupType _groupType = EditorFormGroupType.GroupBox; |
There was a problem hiding this comment.
Field '_groupType' can be 'readonly'.
Suggested change
| private EditorFormGroupType _groupType = EditorFormGroupType.GroupBox; | |
| private readonly EditorFormGroupType _groupType = EditorFormGroupType.GroupBox; |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The change from a
<label>to a<span>forGroupBoxlegends alters semantics and accessibility behavior; consider confirming that no consumers depended on thelabelsemantics (e.g., screen readers or automation tools) and, if needed, add appropriate ARIA roles or heading semantics to preserve usability. - The new
.bb-editor-group-row-headerlayout with vertical legends and a fixed divider could be tight on smaller screens; consider adding responsive adjustments (e.g., media queries to fall back to the default stacked layout) to keep the form usable on narrow viewports.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The change from a `<label>` to a `<span>` for `GroupBox` legends alters semantics and accessibility behavior; consider confirming that no consumers depended on the `label` semantics (e.g., screen readers or automation tools) and, if needed, add appropriate ARIA roles or heading semantics to preserve usability.
- The new `.bb-editor-group-row-header` layout with vertical legends and a fixed divider could be tight on smaller screens; consider adding responsive adjustments (e.g., media queries to fall back to the default stacked layout) to keep the form usable on narrow viewports.
## Individual Comments
### Comment 1
<location> `test/UnitTest/Components/EditorFormTest.cs:181-190` </location>
<code_context>
}
+ [Fact]
+ public void GroupType_Ok()
+ {
+ var foo = new Foo();
+ var cut = Context.Render<EditorForm<Foo>>(pb =>
+ {
+ pb.Add(a => a.Model, foo);
+ pb.Add(a => a.GroupType, EditorFormGroupType.GroupBox);
+ });
+
+ cut.DoesNotContain("bb-editor-group-row-header");
+
+ cut.Render(pb =>
+ {
+ pb.Add(a => a.GroupType, EditorFormGroupType.RowHeader);
+ });
+ cut.Contains("bb-editor-group-row-header");
+ }
+
</code_context>
<issue_to_address>
**suggestion (testing):** Add a test to verify the default GroupType behavior when the parameter is not set.
This test only validates explicit `GroupType` values and never asserts the default. Please add a separate test that renders `EditorForm<Foo>` without setting `GroupType` and verifies that the `bb-editor-group-row-header` CSS class is absent, so we also cover the documented default behavior and guard against regressions in the default/`ClassString` logic.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+181
to
+190
| public void GroupType_Ok() | ||
| { | ||
| var foo = new Foo(); | ||
| var cut = Context.Render<EditorForm<Foo>>(pb => | ||
| { | ||
| pb.Add(a => a.Model, foo); | ||
| pb.Add(a => a.GroupType, EditorFormGroupType.GroupBox); | ||
| }); | ||
|
|
||
| cut.DoesNotContain("bb-editor-group-row-header"); |
Contributor
There was a problem hiding this comment.
suggestion (testing): Add a test to verify the default GroupType behavior when the parameter is not set.
This test only validates explicit GroupType values and never asserts the default. Please add a separate test that renders EditorForm<Foo> without setting GroupType and verifies that the bb-editor-group-row-header CSS class is absent, so we also cover the documented default behavior and guard against regressions in the default/ClassString logic.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Link issues
fixes #7611
Summary By Copilot
Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Add configurable grouping layout for EditorForm and demonstrate its usage.
New Features:
Enhancements:
Tests: