-
-
Notifications
You must be signed in to change notification settings - Fork 365
feat(QueryPageOptions): add TriggerByPagination parameter #5193
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 by SourceryThis pull request introduces a new Sequence diagram for Table pagination query flowsequenceDiagram
participant User
participant Table
participant QueryPageOptions
User->>Table: Click page link
activate Table
Table->>Table: OnPageLinkClick(pageIndex)
Table->>Table: QueryAsync(false, triggerByPagination: true)
Table->>Table: QueryData(triggerByPagination)
Table->>QueryPageOptions: BuildQueryPageOptions()
Note right of QueryPageOptions: Sets TriggerByPagination=true
Table->>Table: OnQueryAsync with updated options
Table->>Table: OnSelectedRowsChanged()
deactivate Table
Class diagram showing QueryPageOptions changesclassDiagram
class QueryPageOptions {
+int PageIndex
+int PageItems
+string? SearchText
+List~IFilterAction~ Filters
+List~ISortAction~ SortList
+bool IsFirstQuery
+bool TriggerByPagination
}
note for QueryPageOptions "Added TriggerByPagination property"
class Table {
-Task QueryAsync(bool shouldRender, int? pageIndex, bool triggerByPagination)
#Task QueryData(bool triggerByPagination)
#Task OnPageLinkClick(int pageIndex)
}
Table ..> QueryPageOptions : uses
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.
Hey @ArgoZhang - I've reviewed your changes - here's some feedback:
Overall Comments:
- Please provide more context in the PR description about why this TriggerByPagination flag is needed and what problem it solves. This will help reviewers better understand the motivation for these changes.
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5193 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 634 634
Lines 28167 28169 +2
Branches 4053 4053
=========================================
+ Hits 28167 28169 +2 ☔ View full report in Codecov by Sentry. |
add TriggerByPagination parameter
Summary of the changes (Less than 80 chars)
简单描述你更改了什么, 不超过80个字符;如果有关联 Issue 请在下方填写相关编号
Description
fixes #5192
Regression?
[If yes, specify the version the behavior has regressed from]
[是否影响老版本]
Risk
[Justify the selection above]
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Add a
TriggerByPaginationparameter toQueryPageOptionsto indicate whether a query was triggered by pagination. Update theQueryAsyncmethod inTable.razor.Edit.csto accept this parameter and pass it to theQueryDatamethod. Update theOnPageLinkClickmethod inTable.razor.Pagination.csto passtriggerByPagination: truewhen callingQueryAsync. Add tests to verify the behavior of theIsAutoQueryFirstQuerymethod.New Features:
TriggerByPaginationparameter toQueryPageOptionsto indicate whether a query was triggered by pagination.Tests:
IsAutoQueryFirstQuerymethod.