feat(SelectTable): add ShowToolbar parameter#7668
Merged
Conversation
Contributor
Reviewer's GuideAdds toolbar customization support to SelectTable, aligns Empty component template naming and styling, adjusts SelectTable minimum width handling, and updates associated tests and JavaScript behavior. Class diagram for updated SelectTable toolbar parametersclassDiagram
class SelectTable {
+int? TableMinWidth
+bool ShowAppendArrow
+string? MultiSelectedItemMaxWidth
+bool ShowToolbar
+RenderFragment? ToolbarTemplate
+RenderFragment? TableExtensionToolbarTemplate
}
class Table {
+bool ShowToolbar
+RenderFragment? ToolbarTemplate
+RenderFragment? TableExtensionToolbarTemplate
+bool ShowDefaultButtons
+bool ShowRefresh
}
SelectTable --> Table : uses
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![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 left some high level feedback:
- In
SelectTable.razor, the innerTablenow forcesShowDefaultButtons="false"andShowRefresh="false"; if consumers might want the standard toolbar buttons in addition to custom templates, consider exposing pass-through parameters instead of hard-coding these to false. - The default min width
602is now duplicated in both theTableMinWidthXML doc comment andSelectTable.razor.js; consider centralizing this value (or deriving the JS default from the component parameter) to avoid these getting out of sync in future changes.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `SelectTable.razor`, the inner `Table` now forces `ShowDefaultButtons="false"` and `ShowRefresh="false"`; if consumers might want the standard toolbar buttons in addition to custom templates, consider exposing pass-through parameters instead of hard-coding these to false.
- The default min width `602` is now duplicated in both the `TableMinWidth` XML doc comment and `SelectTable.razor.js`; consider centralizing this value (or deriving the JS default from the component parameter) to avoid these getting out of sync in future changes.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 #7668 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 749 749
Lines 33196 33199 +3
Branches 4605 4605
=========================================
+ Hits 33196 33199 +3
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 pull request adds toolbar support to the SelectTable component, addressing issue #7663 which requested the ability to add custom controls inside the SelectTable's modal/popover. The PR also includes a typo fix for "empty-telemplate" → "empty-template" and bumps the version to 10.3.3-beta01.
Changes:
- Added
ShowToolbar,ToolbarTemplate, andTableExtensionToolbarTemplateparameters to SelectTable component - Updated minimum width default from 580px to 602px to accommodate toolbar display
- Fixed typo "telemplate" to "template" in Empty component
- Added comprehensive unit tests for the new toolbar functionality
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/BootstrapBlazor/Components/Select/SelectTable.razor.cs | Added three new parameters: ShowToolbar (bool), ToolbarTemplate (RenderFragment), and TableExtensionToolbarTemplate (RenderFragment) with proper documentation |
| src/BootstrapBlazor/Components/Select/SelectTable.razor | Passed new toolbar parameters to inner Table component with ShowDefaultButtons="false" and ShowRefresh="false" |
| src/BootstrapBlazor/Components/Select/SelectTable.razor.js | Updated minimum width default from 580px to 602px to accommodate toolbar |
| test/UnitTest/Components/SelectTableTest.cs | Added comprehensive test for toolbar functionality and fixed typo in existing test |
| src/BootstrapBlazor/Components/Empty/Empty.razor | Fixed typo "empty-telemplate" to "empty-template"; removed UTF-8 BOM character |
| src/BootstrapBlazor/Components/Empty/Empty.razor.scss | Fixed CSS class name typo from "empty-telemplate" to "empty-template"; removed UTF-8 BOM character |
| src/BootstrapBlazor/BootstrapBlazor.csproj | Bumped version from 10.3.2 to 10.3.3-beta01 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -1,4 +1,4 @@ | |||
| @use "../../wwwroot/scss/variables" as *; | |||
| @use "../../wwwroot/scss/variables" as *; | |||
There was a problem hiding this comment.
The UTF-8 BOM character () should be preserved at the beginning of this file to maintain consistency with the codebase convention. All .scss files that start with @use in the codebase use UTF-8 BOM encoding (see examples: src/BootstrapBlazor/Components/Alert/Alert.razor.scss:1, src/BootstrapBlazor/Components/Avatar/Avatar.razor.scss:1, and many others). The line should start with @use instead of @use.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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 #7663
Summary By Copilot
Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Add toolbar customization support to SelectTable and align empty state styling and table width behavior.
New Features:
Bug Fixes:
Tests: