Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideRefactors where the table’s default sort column and order are initialized so that the DefaultSortOrder is applied on every render cycle where columns are rebuilt, not only the first render. Sequence diagram for updated table default sort initializationsequenceDiagram
participant Renderer
participant Table
Renderer->>Table: OnAfterRenderAsync(firstRender)
activate Table
Table->>Table: BuildTableColumnsAsync()
Table->>Table: set SortName and SortOrder from DefaultSortOrder
Table->>Table: QueryAsync(true, 1, false, true, IsAutoQueryFirstRender)
deactivate Table
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 - I've left some high level feedback:
- The
DefaultSort/DefaultSortOrderinitialization is now tied specifically to theOnAfterRenderAsync(firstRender: true)path; ifBuildTableColumnsAsyncis invoked in other scenarios (e.g., column rebuilds after initial render), the default sort will no longer be applied there, so consider extracting this logic into a reusable helper and calling it from all relevant code paths. - Given that the bug description mentions
DefaultSortOrderonly working on the first render, it would be good to double-check that moving this logic into thefirstRenderbranch actually addresses that problem rather than further restricting whenDefaultSortOrderis applied.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `DefaultSort`/`DefaultSortOrder` initialization is now tied specifically to the `OnAfterRenderAsync(firstRender: true)` path; if `BuildTableColumnsAsync` is invoked in other scenarios (e.g., column rebuilds after initial render), the default sort will no longer be applied there, so consider extracting this logic into a reusable helper and calling it from all relevant code paths.
- Given that the bug description mentions `DefaultSortOrder` only working on the first render, it would be good to double-check that moving this logic into the `firstRender` branch actually addresses that problem rather than further restricting when `DefaultSortOrder` is applied.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 #8062 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 766 766
Lines 34155 34155
Branches 4697 4697
=========================================
Hits 34155 34155
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
Fixes a bug where setting DefaultSort/DefaultSortOrder on a Table column caused sorting to be reset on every column rebuild, breaking user-initiated sorting after the first render. The default sort initialization is moved from BuildTableColumnsAsync (which runs on every render) to the first-render branch of OnAfterRenderAsync, so user sort selections persist on subsequent renders.
Changes:
- Move default sort assignment (
SortName/SortOrderfrom a column withSortable=true, DefaultSort=true) out ofBuildTableColumnsAsync. - Apply default sort once in
OnAfterRenderAsyncfirst-render branch, after building columns and before initial query.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Link issues
fixes #8053
Summary By Copilot
Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Bug Fixes: