danroth27 / blazorscaffolding Goto Github PK

Altogether this looks really good to me! I'm glad to see you were able to keep almost all of it very simple and direct. It doesn't feel conceptually any harder than our earlier web frameworks despite scaling up to so much richer functionality.

File layout

In terms of file layout, what you have is very conventional for ASP.NET historically so will definitely be OK as-is. My personal preference would be to try to reduce the "spread out all the files based on their type" a bit if possible and try to group more by app feature area, but the only opportunity for that here is perhaps combining the Data, Models, and Migrations folders into a single top-level folder, e.g.:

• Data/
  • AppDbContext.cs
  • Movie.cs
  • Migrations/
    • 20230701025650_BlahBlahBlah.cs
    • etc

... however if this would force more nonstandard config options then it's not worth it, and what you have is already perfectly fine.

CRUD pages

Is the = new() useful here? I thought it would get overwritten with null if it's not currently handling a form post. But I guess you must have tried it out.

BlazorScaffoldingContext is an odd name. How about AppDbContext? I faintly remember the older scaffolders used a name like that.

In all URLs, make sure to use base-relative paths and not root-relative ones. For example, in this case it should be movies and not /movies. Otherwise it will break if people add a nonempty pathbase.

Did you try this approach? dotnet/aspnetcore#45654 (comment) I'd be interested to know if that doesn't work.

My guess is that the HTTP status code can easily be set to 404 this way, but perhaps the wider issue is you then have to put in some UI logic like if (notFound) { <h1>Not found</h1> } else { ... } and that's cumbersome and makes the whole thing feel less ergonomic. Is that the concern?

Do you think it's worth enabling pagination? Otherwise if people wire this up to a DB table with a large number rows, the page will appear to be broken, or at least will have very poor perf since it will actually retrieve all the rows from the DB.

Do you know for sure that id values will always be numeric/GUIDs? If it's possible for them to be other strings, you'd need to URL-encode the ID parameter in URLs like this. One fairly clean option would be to add a static method to some of the pages to return their URL, e.g., in Details.razor, add:

public static string Url(Movie movie) => $"movies/details/{UrlEncode(movie.id)}";

And then elsewhere you can write Details.Url(movie). That's the sort of direct, non-magic approach I used to find more effective than framework-based URL generation.

I notice there's no streaming rendering in here. That's totally fine and I wouldn't necessarily suggest introducing that unless you can observe a UX benefit. I don't know how well it would work or not with the particular QuickGrid usage pattern you're using. But curious to know if you tried it.