Skip to content

Conversation

@StevenTCramer
Copy link
Contributor

Pull Request

📖 Description

This PR improves the data refresh logic in the FluentDataGrid component to better handle state changes, particularly those related to pagination. It addresses potential inconsistencies that could occur during rapid state changes or when other events trigger state updates.

This change is a fix that enhances the reliability and consistency of the FluentDataGrid component.

🎫 Issues

👩‍💻 Reviewer Notes

Reviewers should focus on:

  1. The changes in the OnParametersSetAsync method, particularly the mustRefreshData calculation.
  2. The updated RefreshDataCoreAsync method, noting the new position of the _lastRefreshedPaginationStateHash update.

📑 Test Plan

  • Manual testing has been performed.

✅ Checklist

General

  • I have added tests for my changes.
  • I have tested my changes.
  • I have updated the project documentation to reflect my changes.
  • I have read the CONTRIBUTING documentation and followed the standards for this project.

Component-specific

  • I have added a new component
  • I have added Unit Tests for my new component
  • I have modified an existing component
  • I have validated the Unit Tests for an existing component

This commit enhances the OnParametersSetAsync method in FluentDataGrid to
better handle state changes:

- Updated mustRefreshData calculation to consider pagination state changes
- Moved _lastRefreshedPaginationStateHash update earlier in RefreshDataCoreAsync

These changes ensure that:
1. The grid correctly detects when a refresh is needed due to pagination changes
2. The pagination state hash is captured at the start of the refresh process

This improvement helps maintain data consistency, especially in scenarios
with rapid state changes or when other events trigger state updates.
- Replace object casting and reference comparison with direct Equals method calls
- Separately compare Items and ItemsProvider with their last assigned values
- Eliminate false positive change detections caused by boxing
- Improve performance by reducing unnecessary data refreshes
@StevenTCramer
Copy link
Contributor Author

Add fix for #2512

@StevenTCramer
Copy link
Contributor Author

@dvoituron I updated PR you may want to review again.

@vnbaaij vnbaaij changed the title FluentDataGrid: Improve data refresh logic in FluentDataGrid [DataGrid] Improve data refresh logic Aug 20, 2024
@vnbaaij vnbaaij enabled auto-merge (squash) August 20, 2024 08:35
@vnbaaij vnbaaij merged commit 0083403 into microsoft:dev Aug 20, 2024
@vnbaaij vnbaaij added this to the v4.9.4 milestone Aug 22, 2024
dannyldj pushed a commit to dannyldj/fluentui-blazor that referenced this pull request Sep 26, 2024
* Improve data refresh logic in FluentDataGrid

This commit enhances the OnParametersSetAsync method in FluentDataGrid to
better handle state changes:

- Updated mustRefreshData calculation to consider pagination state changes
- Moved _lastRefreshedPaginationStateHash update earlier in RefreshDataCoreAsync

These changes ensure that:
1. The grid correctly detects when a refresh is needed due to pagination changes
2. The pagination state hash is captured at the start of the refresh process

This improvement helps maintain data consistency, especially in scenarios
with rapid state changes or when other events trigger state updates.

* Fix Grid data source change detection to avoid boxing

- Replace object casting and reference comparison with direct Equals method calls
- Separately compare Items and ItemsProvider with their last assigned values
- Eliminate false positive change detections caused by boxing
- Improve performance by reducing unnecessary data refreshes

* Don't use GetHashCode for comparison. TotalCount should NOT be a part of the comparison. As this is an output not an input.

* use _lastRefreshedPaginationState instead of _lastRefreshedPaginationStateHash

* Remove blank line

---------

Co-authored-by: Denis Voituron <[email protected]>
Co-authored-by: Vincent Baaij <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants