feat(BootstrapInputGroupLabel): add IToolbarComponent cascading value#7259
feat(BootstrapInputGroupLabel): add IToolbarComponent cascading value#7259
Conversation
Reviewer's GuideAdds cascading of the IToolbarComponent into BootstrapInputGroupLabel so it can act as an input group label inside table toolbars, updates TableToolbar cascading values accordingly, and introduces a unit test validating the new behavior. Sequence diagram for cascading IToolbarComponent into BootstrapInputGroupLabelsequenceDiagram
participant TableToolbar
participant TableToolbarComponent_TItem_ as TableToolbarComponent
participant BootstrapInputGroupLabel
TableToolbar->>TableToolbarComponent_TItem_: Render when IsShow is true
TableToolbarComponent_TItem_->>BootstrapInputGroupLabel: Render ChildContent
activate TableToolbarComponent_TItem_
TableToolbarComponent_TItem_-->>BootstrapInputGroupLabel: CascadingParameter ToolbarComponent = button
TableToolbarComponent_TItem_-->>BootstrapInputGroupLabel: CascadingParameter OnGetSelectedRows
BootstrapInputGroupLabel->>BootstrapInputGroupLabel: Compute IsInputGroupLabel
BootstrapInputGroupLabel-->>TableToolbarComponent_TItem_: Render output based on IsInputGroupLabel
deactivate TableToolbarComponent_TItem_
Class diagram for BootstrapInputGroupLabel with IToolbarComponent cascadingclassDiagram
class BootstrapInputGroupLabel {
+string? Value
+bool? IsGroupLabel
+bool ShowRequiredMark
+string? Required
+bool IsInputGroupLabel
}
class InputGroup
class IToolbarComponent {
<<interface>>
+bool IsShow
}
class TableToolbarComponent_TItem_ {
+bool IsShow
}
BootstrapInputGroupLabel --> InputGroup : CascadingParameter InputGroup
BootstrapInputGroupLabel --> IToolbarComponent : CascadingParameter ToolbarComponent
TableToolbarComponent_TItem_ ..|> IToolbarComponent
Flow diagram for IsInputGroupLabel evaluation with ToolbarComponentflowchart TD
A[Start IsInputGroupLabel evaluation] --> B{IsGroupLabel has value}
B -->|yes| C[IsInputGroupLabel = IsGroupLabel]
B -->|no| D{InputGroup is not null or ToolbarComponent is not null}
D -->|yes| E[IsInputGroupLabel = true]
D -->|no| F[IsInputGroupLabel = false]
C --> G[End]
E --> G
F --> G
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- The
IsInputGroupLabelexpression combines??and||without parentheses; given C#’s operator precedence this may not evaluate as intended, so consider explicitly grouping it asIsGroupLabel ?? (InputGroup != null || ToolbarComponent != null)(or similar) to make the logic unambiguous. - In
TableToolbar.razor, the cascading value currently usesValue="button"inside theelse if (button is TableToolbarComponent<TItem> { IsShow: true } cb)block; usingcbinstead ofbuttonwould be clearer and ensures the cascaded value is explicitly theTableToolbarComponent<TItem>instance.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `IsInputGroupLabel` expression combines `??` and `||` without parentheses; given C#’s operator precedence this may not evaluate as intended, so consider explicitly grouping it as `IsGroupLabel ?? (InputGroup != null || ToolbarComponent != null)` (or similar) to make the logic unambiguous.
- In `TableToolbar.razor`, the cascading value currently uses `Value="button"` inside the `else if (button is TableToolbarComponent<TItem> { IsShow: true } cb)` block; using `cb` instead of `button` would be clearer and ensures the cascaded value is explicitly the `TableToolbarComponent<TItem>` instance.
## Individual Comments
### Comment 1
<location> `src/BootstrapBlazor/Components/Table/TableToolbar.razor:37` </location>
<code_context>
{
- <CascadingValue Value="OnGetSelectedRows" IsFixed="true">
- @cb.ChildContent
+ <CascadingValue Value="button" IsFixed="true">
+ <CascadingValue Value="OnGetSelectedRows" IsFixed="true">
+ @cb.ChildContent
+ </CascadingValue>
</code_context>
<issue_to_address>
**issue (bug_risk):** Consider cascading `cb` (or an explicit `IToolbarComponent` cast) instead of `button` to avoid mismatched cascading type.
Because `button` is likely typed as a broader interface/base type (the collection element type), `<CascadingValue>` will infer that broader type for the cascade. That value may not be assignable to `IToolbarComponent`, so it might not reach `BootstrapInputGroupLabel`’s `IToolbarComponent` cascade parameter. Using `cb` (typed as `TableToolbarComponent<TItem>`) or an explicit `IToolbarComponent` cast keeps the cascaded type aligned with the consumer and avoids this mismatch.
</issue_to_address>
### Comment 2
<location> `test/UnitTest/Components/InputTest.cs:298-299` </location>
<code_context>
cut.Contains("input-group-text");
}
+ [Fact]
+ public void ToolbarComponent_Ok()
+ {
+ var cut = Context.Render<CascadingValue<IToolbarComponent>>(pb =>
</code_context>
<issue_to_address>
**suggestion (testing):** Add a complementary test to cover the case where a ToolbarComponent is present but IsGroupLabel is explicitly set to false.
Given the new `IsInputGroupLabel` logic (`IsGroupLabel ?? InputGroup != null || ToolbarComponent != null`), we should verify that an explicit `IsGroupLabel = false` still suppresses the `input-group-text` styling when a `ToolbarComponent` is cascaded.
A test like this would cover that override behavior and help catch regressions if the boolean expression changes:
```csharp
[Fact]
public void ToolbarComponent_WithIsGroupLabelFalse_DoesNotRenderGroupLabel()
{
var cut = Context.Render<CascadingValue<IToolbarComponent>>(pb =>
{
pb.Add(a => a.Value, new MockToolbarComponent());
pb.Add(a => a.IsFixed, true);
pb.Add(a => a.ChildContent, builder =>
{
builder.OpenComponent<BootstrapInputGroupLabel>(0);
builder.AddAttribute(1, nameof(BootstrapInputGroupLabel.IsGroupLabel), false);
builder.CloseComponent();
});
});
cut.DoesNotContain("input-group-text");
}
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7259 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 745 745
Lines 32585 32586 +1
Branches 4516 4516
=========================================
+ Hits 32585 32586 +1
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:
|
There was a problem hiding this comment.
Pull request overview
This PR enhances the BootstrapInputGroupLabel component to support automatic styling when used within toolbar contexts by adding IToolbarComponent cascading parameter support. This allows labels within TableToolbarComponent to automatically apply the input-group-text CSS class without requiring manual configuration.
- Adds
IToolbarComponentcascading parameter toBootstrapInputGroupLabel - Updates
TableToolbar.razorto cascade the toolbar button component instance - Includes comprehensive test coverage for the new functionality
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/BootstrapBlazor/Components/Input/BootstrapInputGroupLabel.cs |
Adds IToolbarComponent cascading parameter and updates logic to check for toolbar component presence when determining label styling |
src/BootstrapBlazor/Components/Table/TableToolbar.razor |
Wraps TableToolbarComponent child content with additional cascading value to pass button instance; removes BOM character from file |
test/UnitTest/Components/InputTest.cs |
Adds ToolbarComponent_Ok test with mock implementation to verify cascading parameter functionality |
src/BootstrapBlazor/BootstrapBlazor.csproj |
Bumps version from 10.1.1 to 10.1.2-beta01 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Link issues
fixes #7258
Summary By Copilot
Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Support toolbar components as input group labels and ensure they receive necessary cascading values.
New Features:
Tests: