Codecov Report

Attention: Patch coverage is 84.61538% with 2 lines in your changes missing coverage. Please review.

Project coverage is 91.50%. Comparing base (e8b261b) to head (bb6ed2d).
Report is 1 commits behind head on dev.

@@            Coverage Diff             @@
##              dev   #10218      +/-   ##
==========================================
- Coverage   91.52%   91.50%   -0.02%     
==========================================
  Files         413      414       +1     
  Lines       12979    12979              
  Branches     2447     2450       +3     
==========================================
- Hits        11879    11877       -2     
  Misses        557      557              
- Partials      543      545       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

before, virtualize call look like

ServerDataFunc index : 0
ServerDataFunc index : 0
ServerDataFunc index : 1141
ServerDataFunc index : 5
ServerDataFunc index : 0
ServerDataFunc index : 9
ServerDataFunc index : 0
ServerDataFunc index : 14
ServerDataFunc index : 0
ServerDataFunc index : 18
ServerDataFunc index : 0
ServerDataFunc index : 22
ServerDataFunc index : 0
ServerDataFunc index : 27
ServerDataFunc index : 0
ServerDataFunc index : 31
ServerDataFunc index : 0
ServerDataFunc index : 35
ServerDataFunc index : 0
ServerDataFunc index : 40
ServerDataFunc index : 0
ServerDataFunc index : 44
ServerDataFunc index : 0
ServerDataFunc index : 48
ServerDataFunc index : 0
ServerDataFunc index : 52
ServerDataFunc index : 0
ServerDataFunc index : 56
ServerDataFunc index : 0
ServerDataFunc index : 60
ServerDataFunc index : 0
ServerDataFunc index : 64
ServerDataFunc index : 0
ServerDataFunc index : 68
ServerDataFunc index : 0
ServerDataFunc index : 72
ServerDataFunc index : 0
ServerDataFunc index : 76
ServerDataFunc index : 0
ServerDataFunc index : 80
ServerDataFunc index : 0
ServerDataFunc index : 84
ServerDataFunc index : 0
ServerDataFunc index : 88
ServerDataFunc index : 0
ServerDataFunc index : 93
ServerDataFunc index : 0
ServerDataFunc index : 97
ServerDataFunc index : 0
ServerDataFunc index : 101
ServerDataFunc index : 0

after fix :

ServerDataFunc index : 0
ServerDataFunc index : 0
ServerDataFunc index : 1141
ServerDataFunc index : 6
ServerDataFunc index : 10
ServerDataFunc index : 14
ServerDataFunc index : 19
ServerDataFunc index : 23
ServerDataFunc index : 27
ServerDataFunc index : 31
ServerDataFunc index : 35
ServerDataFunc index : 39
ServerDataFunc index : 43
ServerDataFunc index : 47
ServerDataFunc index : 52
ServerDataFunc index : 56
ServerDataFunc index : 60
ServerDataFunc index : 65
ServerDataFunc index : 69
ServerDataFunc index : 73
ServerDataFunc index : 77
ServerDataFunc index : 81
ServerDataFunc index : 85
ServerDataFunc index : 89
ServerDataFunc index : 93
ServerDataFunc index : 97
ServerDataFunc index : 101
ServerDataFunc index : 105
ServerDataFunc index : 109
ServerDataFunc index : 114
ServerDataFunc index : 118
ServerDataFunc index : 122
ServerDataFunc index : 126
ServerDataFunc index : 130

@KapustinVadim1991, could you review this to ensure it's a good fix and that it doesn't break existing virtualization functionality in the datagrid? I'm asking because you were the one who mainly implemented this feature.

                if (request.StartIndex > 0 && _serverData.TotalItems < request.StartIndex + request.Count)
                {
                    _serverData = await VirtualizeServerData(
                        stateFunc(0, request.Count),
                        request.CancellationToken
                    );
                }

@Ptipoi-jf Good afternoon, this block of code covered the following case: we have a list with TotalItems = 10, and we request through the virtualization component with StartIndex = 8, Count = 2. At this point, everything works correctly, and we receive the required records. Then, an external filter changes (I’m not sure about internal DataGrid filters), and I call ReloadServerData, which returns TotalItems = 5. However, the StartIndex in the DataGrid has not changed, so we will receive 0 records. The main question is how to reset StartIndex to 0 and request a new collection; otherwise, the DataGrid will remain in this state.

This is an edge case, but it’s crucial for the project I’m developing, as all our filters are external.

If you plan to remove this part of the code, there should be a method allowing the list request indices to reset (ReloadServerData(rbool resetIndices) or something like that).

@KapustinVadim1991 Good afternoon, thanks to review

Understood, but this is a weird fix i guess.
Your bug isn't still here when you are at the index 0 with incomplete page and the filter change ? What did you call external filter, a filter provided by service or a user field search ?

i'll create a new component in the viewer project to check that.

If you plan to remove this part of the code, there should be a method allowing the list request indices to reset (ReloadServerData(rbool resetIndices) or something like that).

=> this method already exist and named RefreshDataAsync from the virtualize component did we need to wrap it in your MudVirtualize ?
an exemple here : https://learn.microsoft.com/en-us/aspnet/core/blazor/components/virtualization?view=aspnetcore-8.0#item-provider-delegate

<Virtualize ... @ref="virtualizeComponent">
    ...
</Virtualize>

...

private Virtualize<FetchData>? virtualizeComponent;

protected override void OnInitialized()
{
    WeatherForecastSource.ForecastUpdated += async () => 
    {
        await InvokeAsync(async () =>
        {
            await virtualizeComponent?.RefreshDataAsync();
            StateHasChanged();
        });
    });
}

Btw, if i'm removing this bloc, this is now a breaking change, so i can restore this part then PR an other fix-breaking. What did you think about ?

EDIT :
you already have that method binded in MudVirtualize

        /// <summary>
        /// Refreshes the data in the Virtualize component asynchronously.
        /// </summary>
        public async Task RefreshDataAsync()
        {
            if (_virtualizeContainerReference != null)
            {
                await _virtualizeContainerReference.RefreshDataAsync();
            }
        }

@Ptipoi-jf I’m completely in favor of keeping this part of the code, as it ideally doesn’t break virtualization behavior—it simply requests records from the 0 index if, for some reason, the TotalItemsCount doesn’t match reality, as it shouldn’t be less than StartIndex + Count, if I’m not mistaken.

The filter case is as follows: on my page, I have a DataGrid, but the client requests a unified search field for all data located outside the DataGrid. So, I create a DataGrid, assign it a function for data retrieval, and also need to reload the data in the DataGrid since the filter has changed, so I call the DataGrid.ReloadServerData method.

Perhaps RefreshDataAsync uses the same indexes as before—I can’t say for sure. If it resets StartIndex, it could be a potentially elegant solution, but I guess it just call the provided function with the same indices.

Then, an external filter changes (I’m not sure about internal DataGrid filters), and I call ReloadServerData

Do I understand correctly that if DataGrid had an event that tells the filters were updated inside DataGrid, then this code part wouldn't be needed? If so, then in that case we probably would want to make that feature first before continuing this PR if there is no other ways to make things more or less compatible if there is no other ways for you to do your edge case.

Do I understand correctly that if DataGrid had an event that tells the filters were updated inside DataGrid, then this code part wouldn't be needed?

Implementing the event won’t change the situation, as the event will still call InvokeServerLoadFunc, which will reload the data from the server with the same StartIndex and Count. I reviewed the code, and if my calculations are correct, internal filters will also trigger the same method without modifying StartIndex.

Sorry, I haven’t tried reproducing it, as it would require removing the code section that you plan to delete, but here are the test parameters for checking DataGrid functionality after the changes (removing that code block with the check): create a DataGrid (Virtualize + VirtualizeServerData) with 100 items, scroll to the end of the list, and change the filter so that only 10 items (approximately) match the filter criteria. Expected result: 10 records, not 0.

it simply requests records from the 0 index if, for some reason, the TotalItemsCount doesn’t match reality, as it shouldn’t be less than StartIndex + Count, if I’m not mistaken.

that break how virtualize component work because you returning element outside requested range. my first fix here was to take _serverData.TotalItems - request.Count but this is wrong too =>if we load the previous chunk we have duplicated data in the grid

Let me test with the search field and then we can discuss about it

@KapustinVadim1991 This is an other bug, the native virtualize component is working fine.

I push if you wanna check yourself until i search the main reason ?

new viewer file : DataGridVirtualizeServerDataLoadingTestWithSearch.razor

The _currentRenderFilteredItemsCache is not accurate when using external filter and ItemProvider, maybe a race condition.

When you type slowly in the search field it works (one char by one), but when you type or remove multiple chars it display empty even if search field is empty.

i'm searching a fix.

i've got a partial fix => empty result can appear in the footer because _currentRenderFilteredItemsCache is not accurate

Good morning, I checked the DataGridVirtualizeServerDataLoadingTestWithSearch in your branch and see that everything is working perfectly, and the bug with rapid input is also fixed.

I also decided to check the dev branch with and without the if (request.StartIndex > 0 && _serverData.TotalItems < request.StartIndex + request.Count) check, and I couldn’t reproduce the bug I mentioned earlier. Apologies for the false information—either I mixed something up, or it was fixed in the new versions.

Thank you for the PR 😊

@ScarletKuro I have no more questions

I also decided to check the dev branch with and without the if (request.StartIndex > 0 && _serverData.TotalItems < request.StartIndex + request.Count) check, and I couldn’t reproduce the bug I mentioned earlier

hmm, this is a bit strange, i was able to reproduce in my branch with DataGridVirtualizeServerDataLoadingTestWithSearch before the fix

did i need to add EmptyContent to the MudVirtualize, it's missing actualy then that user template through NoRecordsContent didn't 'work' with virtualize

hmm, this is a bit strange, i was able to reproduce in my branch with DataGridVirtualizeServerDataLoadingTestWithSearch before the fix

I meant that the bug I mentioned wasn’t reproduced in 100% of cases, while the side bug with rapid input was reproducible in the dev branch but not in your branch 😊

did i need to add EmptyContent to the MudVirtualize, it's missing actualy then that user template through NoRecordsContent didn't 'work' with virtualize

I didn’t quite understand the part about EmptyContent, but NoRecordContent is used in my project across all DataGrids with VirtualizeServerData, and it works correctly.

I need to check in my branch if EmptyContent is correctly displayed with virtualization enable.
When i'm ready the code this is not the case, because EmptyContent must be now display with virtualize component => EmptyContent is not available in MudVirtualize

What is the status? Is this ready to be merged?

@ScarletKuro & @KapustinVadim1991 can you check my last commit then it's ready to merge.

The grid must show now NoRecordsContent when no resultset with virtualize=true => this implie new 'feature' in mudVirtualize to support @NoRecordsContent

Edit : can you explain me why my new component VirtualizeNoRecordsContent.razor didn't shown in viewer project ?

I will try to re-review it on weekends.

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

I actually don't know how to reliably test it.

bUnit sets the viewport to 1,000,000,000 for the Virtualize component, which means all items are rendered at once. However, I'm unsure how to reliably wait for all 20k items to render. The ServerDataFunc returns all 200k items, but simply awaiting the completion of ServerDataFunc doesn't help, since it seems the bUnit (or possibly the Virtualize component itself) renders in batches. In the first second, 100 items appear in the markup, and in the next second, more are rendered.
What I wanted to do was to wait until all the items are rendered, then click a random index and check if the _selectedIndex is correct in the DataGridVirtualizeServerDataLoadingLargeDataSetTest.

Will just merge and hope there won't be complains.

is this PR will be merge as fix for V7 too ?

Rememeber - there is another side bug when using virtualization :

empty result can appear in the footer because _currentRenderFilteredItemsCache is not accurate

This caused regression #10343

is this PR will be merge as fix for V7 too ?

I would prefer not for now, I want to get feedbacks from v8 first, and as you can see for good reason. Also, these changes have conflict with v7 if you try to port it, so you would need to create a new PR against v7 branch.

As for now I created this PR #10361 which will be on hold