-
Notifications
You must be signed in to change notification settings - Fork 10.2k
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
Improve the Virtualization docs to clarify that placeholder content must be the same size as item content #44430
Comments
@Alerinos thanks for contacting us. When you are scrolling fast, You have a cancellation token in the ItemsProviderRequest that you can flow to stop the ongoing queries before starting a new one to avoid sending multiple queries in parallel. |
@javiercn var count = await _search._context.Article.AsNoTracking().Where(x => x.Status == Models.Article.StatusType.Show).CountAsync(request.CancellationToken); <--- here
var numEmployees = Math.Min(request.Count, count - request.StartIndex);
var dto2 = await _search._context.Article
.AsNoTracking()
.Include(x => x.Details)
.Include(x => x.Names)
.Include(x => x.Images)
.Where(x => x.Status == Models.Article.StatusType.Show)
.OrderByDescending(x => x.Details.Created)
.Skip(request.StartIndex)
.Take(request.Count)
.ToListAsync(request.CancellationToken); <--- here Are you able to see how it should be done? I think it is also worth updating the documentation, there may be many people like me. Edit: The same error
|
@Alerinos thanks for the additional details. We already have documentation covering this. You can check it here. The suggestion would be to instantiate a new DbContext per operation as described in the article. I am not 100% sure why passing the cancellation token approach did not work. That would be a question for the EF folks. |
I found a similar problem. @javiercn I think it is also Blazor's fault, see for yourself that instead of loading a new element once, it refreshes it many times until the exception. my code: bool IsRun = false;
private async ValueTask<ItemsProviderResult<DTO.Article>> LoadEmployees(ItemsProviderRequest request)
{
if (IsRun is true)
await Task.Delay(TimeSpan.FromSeconds(1));
IsRun = true;
...
IsRun = false;
return new ItemsProviderResult<DTO.Article>(dto3, count); Edit: |
@Alerinos to be more concrete, use a |
In the case of Cancel Token:
[Inject] IDbContextFactory<Context> DbFactory { get; set; } = null!;
private async ValueTask<ItemsProviderResult<DTO.Article>> LoadEmployees(ItemsProviderRequest request)
{
using var context = DbFactory.CreateDbContext();
var count = await context.Article.AsNoTracking().Where(x => x.Status == Models.Article.StatusType.Show).CountAsync(); // request.CancellationToken
var numEmployees = Math.Min(request.Count, count - request.StartIndex);
var dto2 = await context.Article
.AsNoTracking()
.Include(x => x.Details)
.Include(x => x.Names)
.Include(x => x.Images)
.Where(x => x.Status == Models.Article.StatusType.Show)
.OrderByDescending(x => x.Details.Created)
.Skip(request.StartIndex)
.Take(request.Count)
.ToListAsync(); // request.CancellationToken @javiercn I understand that this is correct behavior and is it supposed to be? |
@Alerinos it is not obvious what is going on in your app. Could you provide a minimal repro project so that we can try and determine if there is an issue here? At this point I do not think I have the expertise in this area to be accurate, so someone more familiar with the code will help here. |
Hi @Alerinos. We have added the "Needs: Author Feedback" label to this issue, which indicates that we have an open question for you before we can take further action. This issue will be closed automatically in 7 days if we do not hear back from you by then - please feel free to re-open it if you come back to this issue after that time. |
@javiercn edit: |
@javiercn Can you also open this topic? It's closed and I don't want it to be forgotten. |
Thanks for contacting us. |
@Alerinos I think this will likely come down to an issue in the sample app (once we get a chance to look at it) or a more complete doc, I have peek through the framework code, and I don't see anything obviously wrong. |
await Task.Delay(TimeSpan.FromSeconds(1)); I used something like that. It seems that the component does not wait for return to be executed, but creates a new instance. I will try to test with rest api. A whole class of siteusing Microsoft.AspNetCore.Components.Web.Virtualization;
using Microsoft.AspNetCore.Components;
using Microsoft.EntityFrameworkCore;
namespace Virtualize_bug.Pages;
[Route("Articles")]
public partial class Articles : ComponentBase
{
[Inject] IDbContextFactory<Context> DbFactory { get; set; } = null!;
private async ValueTask<ItemsProviderResult<DTO.Article>> LoadEmployees(ItemsProviderRequest request)
{
await Task.Delay(TimeSpan.FromSeconds(1));
using var context = DbFactory.CreateDbContext();
var count = await context.Article.AsNoTracking().CountAsync(); // request.CancellationToken
var numEmployees = Math.Min(request.Count, count - request.StartIndex);
var dto2 = await context.Article
.AsNoTracking()
.Include(x => x.Details)
.OrderByDescending(x => x.Details.Created)
.Skip(request.StartIndex)
.Take(request.Count)
.ToListAsync(); // request.CancellationToken
var dto3 = dto2.Select(x => new DTO.Article
{
Title = x.Details.Title,
Description = x.Details.Description,
Created = x.Details.Created
}).ToList();
return new ItemsProviderResult<DTO.Article>(dto3, count);
}
} Edit: |
@Alerinos can you please share a minimum repro project hosted as a public GitHub repo, so that we can investigate this further. Thanks! |
@MackinnonBuck |
@Alerinos That seems like a reasonable suggestion. We currently have #10522 tracking the general idea of throttling/debouncing events, but the |
Thanks, I created a new topic. @MackinnonBuck I found another problem, the component does not "cache" HTML elements but cleans them, so there may be another security problem. See in the attached video how I load the database by playing only with the scroll. You should also consider not disposing of the loaded item or keeping it somewhere in memory. |
Thanks, @Alerinos.
This can already be done by manually caching results and returning them from the |
I meant more on the browser side, thanks to which we will save transfer between the server and the client. For large pages, this will make a big difference. Edit: |
@Alerinos If I'm understanding correctly, you're suggesting we cache the removed DOM elements on the JavaScript side and reinsert them (still via JavaScript) when they get scrolled back into view? I'm inclined to think that's not a direction we would take for the following reasons:
So, this might end up resulting in a lot of added complexity for marginal gain, but please correct me if I'm misinterpreting the suggestion 🙂 |
Currently loaded items Instead of deleting them, we can leave them and possibly give an ID so that blazor knows which elements are already loaded. So in theory we don't need javascript integration. https://gyazo.com/bbed2cdd93a7adfe88721ec6516f3854 Also thanks to this, we solve the problem when we lose the Internet connection. We have offline content that we just watched. Unfortunately, blazor server is problematic, in wasm you can add everything to the browser's storage and display data from it. Another thing I would improve is:
Return total quantity of items. I believe that it is unnecessary. Why? We have to make an additional query to the server how many elements are there. It is inconvenient and resource-consuming. If the first parameter is zero, it means that there are no elements, you can additionally handle this exception like this: <Virtualize Context="article" ItemsProvider="@LoadEmployees">
<ItemContent>
<div style="height: 50px;">
<h3>@article.Title</h3>
<p>@article.Content</p>
</div>
</ItemContent>
<Placeholder>
<div style="height: 50px;">
In progress...
</div>
</Placeholder>
<Missing>
<div style="height: 50px;">
Sorry, no content, please continue or refresh the page.
</div>
</Missing>
</Virtualize> I think that something like this would make the work of developers easier and save resources. Let me know what you think. |
Another problem I see is when we have 200 articles. Someone adds 20 items while browsing, the cout will change. How will the component react? We will not have duplicate data? At the time of viewing the data, there were fewer items to display than there is now, the new items are higher. It seems to me that this is a difficult logistic topic that is worth considering and discussing. |
I did another test. If we have a high screen, for example, we are shown 30 records, each downward movement causes reloading of 30 elements. (Why load the same if it already exists) What's the problem? The same data is downloaded every time you move up or down. How should it be done in my opinion? |
If you want to preserve already-rendered HTML, then
It's necessary because
It seems you have a lot of ideas and concerns, and that's awesome. However, it's best that we keep comments on this issue related to this issue's topic, which we've decided should be improving the Going forward, would you please use the following process when making feature requests?
Thanks for your feedback and enthusiasm! |
Thanks for contacting us. We're moving this issue to the |
I like this idea. @MackinnonBuck , are you saying that this is possible now? I don't see a property on Virtualize with the number of items to keep in DOM, nor do I see a way to get the number of items currently in the DOM. |
Thanks for contacting us. We're moving this issue to the |
@szalapski Sorry for the late reply - just saw your question. Here's a basic demonstration of how this could work: <Virtualize ItemsProvider="LoadItemsAsync">
<ItemContent>
<p @key="@context">@context</p>
</ItemContent>
</Virtualize>
@code {
private const int PageSize = 40;
private int totalItemCount = 20;
private async ValueTask<ItemsProviderResult<int>> LoadItemsAsync(ItemsProviderRequest request)
{
var items = Enumerable.Range(request.StartIndex, request.Count);
if (request.StartIndex + request.Count >= totalItemCount)
{
// Simulate an artificial delay when loading new content
await Task.Delay(250, request.CancellationToken);
totalItemCount += PageSize;
}
return new(items, totalItemCount);
}
} |
@guardrex Would you be able to clarify the virtualization docs, as suggested in this comment? (I know there's a lot going on in this issue, but we're just using it to track improving this area of the docs) |
Addressed on dotnet/AspNetCore.Docs#31499. |
Thanks @guardrex! Closing this out. |
Is there an existing issue for this?
Describe the bug
I used this example from the documentation:
https://learn.microsoft.com/pl-pl/aspnet/core/blazor/components/virtualization?view=aspnetcore-7.0
My code looks like this:
My model:
Razor:
The first results are correct. The problem starts with scrolling.
First loading, its ok:
![image](https://user-images.githubusercontent.com/42449535/194731707-c20af223-965c-4902-a850-f32f5d1513db.png)
1 break point scrool:
![image](https://user-images.githubusercontent.com/42449535/194731718-7b617779-4658-4386-a505-df37f044a71d.png)
The next one is right away, I don't have to do anything.
![image](https://user-images.githubusercontent.com/42449535/194731726-fdfe260f-ab06-4046-91c6-5361a7fc9ddf.png)
Scrolls on, 2 break point scroll:
![image](https://user-images.githubusercontent.com/42449535/194731740-e82dae9b-b13e-40fb-950e-3fb2b89280bf.png)
Scrolls on, 3 brak point scroll:
![image](https://user-images.githubusercontent.com/42449535/194731746-f74e80c8-ecc5-4bdf-ba1b-e66f829a18b9.png)
Still ok, now takes the breakpoint off.
It turns out that everything is happening fast ...
I'm sitting for this half day today. I do not understand what's going on...
There is a documentation error or I don't understand how to do this.
Can I ask for help?
Expected Behavior
No response
Steps To Reproduce
No response
Exceptions (if any)
No response
.NET Version
.net 7 core rc1
Anything else?
No response
The text was updated successfully, but these errors were encountered: