feat(Transfer): add IsWrapItem/ItemWidth parameter #6960
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
This reverts commit fce10f6.
Co-Authored-By: 18636609650 <[email protected]>
Reviewer's GuideRenames the Transfer component’s base CSS class to ‘bb-transfer’, and adds two new parameters—IsWrapItem and ItemWidth—with corresponding SCSS updates and unit tests to control item wrapping and custom widths. Entity relationship diagram for Transfer component style variableserDiagram
TRANSFER_PANEL_ITEM {
string ItemWidth
bool IsWrapItem
}
TRANSFER_PANEL_LIST {
string MaxHeight
string MinHeight
}
TRANSFER_PANEL_ITEM ||--|| TRANSFER_PANEL_LIST : contains
Class diagram for updated Transfer component parametersclassDiagram
class Transfer_TValue {
+bool IsWrapItem
+string ItemWidth
+string Height
+string ClassString
+string StyleString
}
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.
Pull Request Overview
This PR adds new parameters to the Transfer component to support item wrapping and custom item width configuration. The changes enable developers to control whether transfer panel items wrap to multiple lines and set custom widths for items.
- Added
IsWrapItemparameter to enable/disable text wrapping in transfer items - Added
ItemWidthparameter to allow custom width configuration for transfer items - Updated CSS styling to support the new wrapping behavior and flexbox layout
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| TransferTest.cs | Added unit tests for the new IsWrapItem and ItemWidth parameters |
| variables.scss | Removed unused CSS variable for item margin |
| Transfer.razor.scss | Updated styles to support wrapping behavior and flexbox layout |
| Transfer.razor.cs | Added new parameters and updated CSS class generation |
| BootstrapBlazor.csproj | Version bump to 9.11.3-beta06 |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- In StyleString you’re injecting CSS variables with AddClass, but you should use AddStyle to properly add inline style declarations.
- You removed the default margin on .transfer-panel-item, which could break item spacing—consider reintroducing a margin (or separate spacing logic) for both wrap and non‐wrap modes.
- Renaming the root CSS class from 'transfer' to 'bb-transfer' is a breaking change; make sure all existing references are updated or consider aliasing the old class temporarily.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In StyleString you’re injecting CSS variables with AddClass, but you should use AddStyle to properly add inline style declarations.
- You removed the default margin on .transfer-panel-item, which could break item spacing—consider reintroducing a margin (or separate spacing logic) for both wrap and non‐wrap modes.
- Renaming the root CSS class from 'transfer' to 'bb-transfer' is a breaking change; make sure all existing references are updated or consider aliasing the old class temporarily.
## Individual Comments
### Comment 1
<location> `src/BootstrapBlazor/Components/Transfer/Transfer.razor.scss:70` </location>
<code_context>
.transfer-panel-item {
border-radius: var(--bs-border-radius);
padding: var(--bb-transfer-panel-item-padding);
- margin: var(--bb-transfer-panel-item-margin);
width: var(--bb-transfer-panel-item-width);
}
</code_context>
<issue_to_address>
**suggestion:** Removing margin from transfer-panel-item may cause items to appear crowded.
Consider adding a default margin or gap to ensure adequate spacing between items, especially when they wrap.
Suggested implementation:
```
.transfer-panel-list {
max-height: var(--bb-transfer-panel-list-max-height);
min-height: var(--bb-transfer-panel-list-min-height);
display: flex;
flex-wrap: wrap;
gap: var(--bb-transfer-panel-list-gap, 0.5rem); // fallback gap for spacing
}
```
```
.transfer-panel-item {
border-radius: var(--bs-border-radius);
padding: var(--bb-transfer-panel-item-padding);
margin: var(--bb-transfer-panel-item-margin, 0.25rem); // default margin for spacing
}
}
```
</issue_to_address>
### Comment 2
<location> `src/BootstrapBlazor/Components/Transfer/Transfer.razor.cs:194-198` </location>
<code_context>
[Parameter]
public string? Height { get; set; }
+ /// <summary>
+ /// 获得/设置 是否为换行模式 默认 false 不换行
+ /// </summary>
+ [Parameter]
+ public bool IsWrapItem { get; set; }
+
+ /// <summary>
</code_context>
<issue_to_address>
**suggestion:** Adding IsWrapItem parameter increases flexibility but may need documentation for usage.
Please update user-facing documentation to explain how `IsWrapItem` works, including its effects with custom styles and responsive layouts.
```suggestion
/// <summary>
/// 获得/设置 是否为换行模式。默认值为 <c>false</c>，即不换行。
/// <para>
/// 当 <c>IsWrapItem</c> 设置为 <c>true</c> 时，候选项将在容器宽度不足时自动换行显示，适用于自定义样式或响应式布局场景。
/// </para>
/// <para>
/// <b>注意：</b> 启用换行模式后，建议为 <c>ItemWidth</c> 或容器设置合适的宽度，以确保布局效果符合预期。自定义样式（如 <c>width</c>、<c>flex</c> 等）可能影响换行行为，请根据实际需求调整相关样式。
/// </para>
/// <para>
/// 在响应式布局下，<c>IsWrapItem</c> 可提升候选项的适配能力，使其在不同屏幕尺寸下自动换行，提升用户体验。
/// </para>
/// </summary>
[Parameter]
public bool IsWrapItem { get; set; }
```
</issue_to_address>
### Comment 3
<location> `test/UnitTest/Components/TransferTest.cs:264-265` </location>
<code_context>
cut.Contains("Right-ItemTemplate-Test4");
}
+
+ [Fact]
+ public void IsWrapItem_Ok()
+ {
+ var cut = Context.RenderComponent<Transfer<string>>(pb =>
</code_context>
<issue_to_address>
**suggestion (testing):** Consider adding tests for edge cases with IsWrapItem parameter.
Please add tests for cases where IsWrapItem is unset (default), and when Items is empty or null, to cover additional edge scenarios.
Suggested implementation:
```csharp
[Fact]
public void IsWrapItem_Ok()
{
var cut = Context.RenderComponent<Transfer<string>>(pb =>
{
pb.Add(a => a.Value, "2");
pb.Add(a => a.Items, new List<SelectedItem>()
{
new("1", "Test1 with a very long text"),
new("2", "Test2 with a very long text")
});
pb.Add(a => a.IsWrapItem, false);
});
cut.DoesNotContain("wrap-item");
}
[Fact]
public void IsWrapItem_Default_Ok()
{
var cut = Context.RenderComponent<Transfer<string>>(pb =>
{
pb.Add(a => a.Value, "1");
pb.Add(a => a.Items, new List<SelectedItem>()
{
new("1", "Test1 with a very long text"),
new("2", "Test2 with a very long text")
});
// IsWrapItem not set, should use default
});
// Depending on default value, adjust assertion
// If default is true:
cut.Contains("wrap-item");
// If default is false, use cut.DoesNotContain("wrap-item");
}
[Fact]
public void IsWrapItem_EmptyItems_Ok()
{
var cut = Context.RenderComponent<Transfer<string>>(pb =>
{
pb.Add(a => a.Items, new List<SelectedItem>());
pb.Add(a => a.IsWrapItem, true);
});
// Should not throw, and should not render any items
cut.DoesNotContain("Test1 with a very long text");
cut.DoesNotContain("Test2 with a very long text");
}
[Fact]
public void IsWrapItem_NullItems_Ok()
{
var cut = Context.RenderComponent<Transfer<string>>(pb =>
{
pb.Add(a => a.Items, null);
pb.Add(a => a.IsWrapItem, true);
});
// Should not throw, and should not render any items
cut.DoesNotContain("Test1 with a very long text");
cut.DoesNotContain("Test2 with a very long text");
}
```
- If the default value of `IsWrapItem` is not `true`, adjust the assertion in `IsWrapItem_Default_Ok()` accordingly.
- If your test framework or component throws on null/empty items, you may need to handle exceptions or adjust the assertions.
</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 #6960 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 740 740
Lines 31896 31900 +4
Branches 4473 4473
=========================================
+ Hits 31896 31900 +4
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:
|
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 #6957
Summary By Copilot
Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Add IsWrapItem and ItemWidth parameters to the Transfer component for toggling item wrapping and customizing item width, rename CSS class prefix to bb-transfer, update styles to support wrapping and dynamic width, and include corresponding unit tests
New Features:
Enhancements:
Tests: