-
-
Notifications
You must be signed in to change notification settings - Fork 362
refactor(Table): improve performance use Contains #6891
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideRefactored Table toolbar column visibility logic to improve performance by precomputing visible column name lists and using Contains for lookups instead of Find or DistinctBy. Class diagram for updated Table toolbar column visibility logicclassDiagram
class Table {
+IEnumerable<ITableColumn> GetVisibleColumns()
+bool GetColumnsListState(ColumnVisibleItem item)
-List<ColumnVisibleItem> _visibleColumns
-IEnumerable<ITableColumn> Columns
-int ScreenSize
}
class ITableColumn {
+bool Visible
+string Name
+string GetFieldName()
+bool GetIgnore()
+int ShownWithBreakPoint
}
class ColumnVisibleItem {
+string Name
+bool Visible
}
Table --> ITableColumn
Table --> ColumnVisibleItem
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR improves Table component performance by optimizing column visibility lookup operations to use Contains() instead of Find() methods.
- Replaces linear
Find()searches with more efficientContains()lookups using HashSet-like operations - Refactors the
GetColumnsListStatemethod to use a cleaner, more readable approach - Bumps version from 9.11.2-beta06 to 9.11.2-beta07
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/BootstrapBlazor/Components/Table/Table.razor.Toolbar.cs | Optimizes column visibility checks by using Contains() instead of Find() and refactors GetColumnsListState method |
| src/BootstrapBlazor/BootstrapBlazor.csproj | Version bump to 9.11.2-beta07 |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey there - I've reviewed your changes - here's some feedback:
- Consider using a HashSet for the projected visible column names to get O(1) lookups and avoid repeated list allocations.
- Right now GetColumnsListState rebuilds the visible names list on every call—extracting that projection into a shared helper or caching it would reduce redundant enumeration.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider using a HashSet<string> for the projected visible column names to get O(1) lookups and avoid repeated list allocations.
- Right now GetColumnsListState rebuilds the visible names list on every call—extracting that projection into a shared helper or caching it would reduce redundant enumeration.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 #6891 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 739 739
Lines 31765 31768 +3
Branches 4466 4467 +1
=========================================
+ Hits 31765 31768 +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:
|
|
@sourcecy-ai review |
Link issues
fixes #6890
Summary By Copilot
Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Optimize table column visibility logic by replacing repeated Find calls with precomputed name lists and using Contains for faster lookups.
Enhancements: