Add BitTagsInput component (#11074)#12263
Add BitTagsInput component (#11074)#12263msynk wants to merge 6 commits intobitfoundation:developfrom
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThis pull request introduces a new Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes The primary complexity stems from the component code-behind, which implements intricate event handling for keyboard input, focus management, tag validation (duplicates, max length, max tags), value parsing/formatting, and multiple conditional callback flows. Supporting classes and styling are straightforward, but the comprehensive demo with 14 examples and extensive component parameters across multiple event callbacks warrant careful review of interaction logic and edge cases. Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Adds a new BitTagsInput component to Bit.BlazorUI along with demo documentation and styling, addressing the missing TagsInput component request (#11074).
Changes:
- Introduces
BitTagsInputcomponent (API surface, rendering, styles, and callback args types). - Adds a full demo page (samples, parameters/subclasses/public members documentation, navigation entry).
- Registers component SCSS in the global components stylesheet.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Shared/MainLayout.razor.NavItems.cs | Adds TagsInput to demo navigation. |
| src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/TagsInput/ValidationTagsInputModel.cs | Adds demo validation model for tags. |
| src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/TagsInput/BitTagsInputDemo.razor.samples.cs | Adds demo code snippets for samples. |
| src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/TagsInput/BitTagsInputDemo.razor.cs | Adds demo metadata (parameters/subclasses/public members) and handlers. |
| src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/TagsInput/BitTagsInputDemo.razor | Adds the demo UI page and examples. |
| src/BlazorUI/Bit.BlazorUI/Styles/components.scss | Registers TagsInput SCSS in the component bundle. |
| src/BlazorUI/Bit.BlazorUI/Components/Inputs/TagsInput/BitTagsInputClassStyles.cs | Adds class/style customization contract. |
| src/BlazorUI/Bit.BlazorUI/Components/Inputs/TagsInput/BitTagsInputBeforeArgs.cs | Adds before-add/remove callback args type. |
| src/BlazorUI/Bit.BlazorUI/Components/Inputs/TagsInput/BitTagsInput.scss | Adds component styling. |
| src/BlazorUI/Bit.BlazorUI/Components/Inputs/TagsInput/BitTagsInput.razor.cs | Implements component logic (add/remove, separators, validation integration). |
| src/BlazorUI/Bit.BlazorUI/Components/Inputs/TagsInput/BitTagsInput.razor | Implements component markup and accessibility attributes. |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/BlazorUI/Bit.BlazorUI/Components/Inputs/TagsInput/BitTagsInput.razor.cs (1)
328-361:OnAddcallback fires before value is committed.In
TryAddTags,OnAdd.InvokeAsync(text)is called inside the loop (line 353) beforeSetCurrentValueAsync(list)(line 359). If a subscriber readsCurrentValuein theOnAddhandler, the newly added tag won't be present yet.Consider whether this timing is intentional. The single-tag
TryAddTagmethod has the same pattern (lines 323-325), so this appears to be by design. If so, document this behavior; otherwise, moveOnAddinvocations after the value update.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/BlazorUI/Bit.BlazorUI/Components/Inputs/TagsInput/BitTagsInput.razor.cs` around lines 328 - 361, The OnAdd callback is invoked inside TryAddTags before the component's value is committed, so move the OnAdd calls until after SetCurrentValueAsync to ensure CurrentValue reflects the new tags when subscribers run; implement by collecting added tags (e.g., a local List<string> addedTags) during the loop in TryAddTags and after SetCurrentValueAsync call await OnAdd.InvokeAsync(tag) for each tag in addedTags, and apply the same change to TryAddTag so both paths consistently invoke OnAdd only after SetCurrentValueAsync updates CurrentValue.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/BlazorUI/Bit.BlazorUI/Components/Inputs/TagsInput/BitTagsInput.scss`:
- Line 1: In BitTagsInput.scss fix the `@import` to remove the explicit .scss
extension: change the `@import` "../../../Styles/functions.scss"; statement to
import the partial without the extension so it complies with the
scss/load-partial-extension rule (keep the same relative path but omit ".scss").
This targets the import line in BitTagsInput.scss that currently references
"../../../Styles/functions.scss".
In
`@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/TagsInput/BitTagsInputDemo.razor.cs`:
- Around line 350-355: In HandleBeforeRemove, remove the duplicated assignment
to tagExistsMsg (the second tagExistsMsg = null) so the method only sets
eventsStatus and clears tagExistsMsg once; locate the HandleBeforeRemove method
and delete the redundant tagExistsMsg = null line to avoid the duplicate
operation.
---
Nitpick comments:
In `@src/BlazorUI/Bit.BlazorUI/Components/Inputs/TagsInput/BitTagsInput.razor.cs`:
- Around line 328-361: The OnAdd callback is invoked inside TryAddTags before
the component's value is committed, so move the OnAdd calls until after
SetCurrentValueAsync to ensure CurrentValue reflects the new tags when
subscribers run; implement by collecting added tags (e.g., a local List<string>
addedTags) during the loop in TryAddTags and after SetCurrentValueAsync call
await OnAdd.InvokeAsync(tag) for each tag in addedTags, and apply the same
change to TryAddTag so both paths consistently invoke OnAdd only after
SetCurrentValueAsync updates CurrentValue.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 614765b0-1ce9-4038-958d-f9191c6d2a45
📒 Files selected for processing (11)
src/BlazorUI/Bit.BlazorUI/Components/Inputs/TagsInput/BitTagsInput.razorsrc/BlazorUI/Bit.BlazorUI/Components/Inputs/TagsInput/BitTagsInput.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/TagsInput/BitTagsInput.scsssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/TagsInput/BitTagsInputBeforeArgs.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/TagsInput/BitTagsInputClassStyles.cssrc/BlazorUI/Bit.BlazorUI/Styles/components.scsssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/TagsInput/BitTagsInputDemo.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/TagsInput/BitTagsInputDemo.razor.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/TagsInput/BitTagsInputDemo.razor.samples.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/TagsInput/ValidationTagsInputModel.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Shared/MainLayout.razor.NavItems.cs
| if (Separators is not null) | ||
| { | ||
| foreach (var separator in Separators) | ||
| { | ||
| if (_inputText.Contains(separator)) | ||
| { | ||
| var textWithoutSeparator = _inputText.Replace(separator, string.Empty).Trim(); | ||
| await TryAddTags(_inputText.Split(separator, StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries)); | ||
| // If add was rejected (e.g. duplicate), _inputText still has the separator; strip it | ||
| if (_inputText.Length > 0) | ||
| { | ||
| _inputText = textWithoutSeparator; | ||
| } | ||
| return; | ||
| } | ||
| } |
There was a problem hiding this comment.
HandleOnInput only processes the first separator it finds and returns. With multiple separators configured (e.g., [",", ";"] as shown in the demo), inputs/pastes that contain more than one separator (e.g., "a,b;c") will be split only on the first match, leaving partially-separated tags ("b;c") instead of producing three tags. Consider splitting once using all separators (e.g., string.Split(string[] separators, ...)) so any configured separator is honored in the same input value; adding a unit test for a mixed-separator paste would help prevent regressions.
| if (Separators is not null) | |
| { | |
| foreach (var separator in Separators) | |
| { | |
| if (_inputText.Contains(separator)) | |
| { | |
| var textWithoutSeparator = _inputText.Replace(separator, string.Empty).Trim(); | |
| await TryAddTags(_inputText.Split(separator, StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries)); | |
| // If add was rejected (e.g. duplicate), _inputText still has the separator; strip it | |
| if (_inputText.Length > 0) | |
| { | |
| _inputText = textWithoutSeparator; | |
| } | |
| return; | |
| } | |
| } | |
| if (Separators is not null && Separators.Any(separator => _inputText.Contains(separator))) | |
| { | |
| var textWithoutSeparators = _inputText; | |
| foreach (var separator in Separators) | |
| { | |
| textWithoutSeparators = textWithoutSeparators.Replace(separator, string.Empty); | |
| } | |
| textWithoutSeparators = textWithoutSeparators.Trim(); | |
| await TryAddTags(_inputText.Split([.. Separators], StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries)); | |
| // If add was rejected (e.g. duplicate), _inputText still has separators; strip them | |
| if (_inputText.Length > 0) | |
| { | |
| _inputText = textWithoutSeparators; | |
| } |
closes #11074
Summary by CodeRabbit
BitTagsInputcomponent for collecting and managing multiple tags with customizable templates, event callbacks (add/remove/exists handling), and configuration options (max tags, per-tag length limit, custom separators, duplicate prevention, auto-focus).