Productize QuickGrid

[Nick-Stanton]

Productize QuickGrid

Productizes the experimental QuickGrid package, which aims to provide a simple and performant data grid component for Blazor.

Description

There is a list of issues that we could address either as part of this PR or include small fixes and additions following a merge here. There is already a lot of new code and API being proposed so non-bug fixes may be best for after this PR.

Following initial review here I will open an API proposal for these changes.

Tested:

• Manually
• E2E

Fixes #46317

[dougbu]

@Nick-Stanton, you might want to instead use the https://github.com/dotnet/aspnetcore-internal/tree/main/scripts/consolidation infrastruture to copy over every relevant file, commit, issue reference, and PR reference from the AspLabs repo. @wtgodbe or I could help you get started on that. Ignore this if the history isn't relevant from the @dotnet/aspnet-blazor-eng perspective.

[Nick-Stanton]

@Nick-Stanton, you might want to instead use the https://github.com/dotnet/aspnetcore-internal/tree/main/scripts/consolidation infrastruture to copy over every relevant file, commit, issue reference, and PR reference from the AspLabs repo. @wtgodbe or I could help you get started on that. Ignore this if the history isn't relevant from the @dotnet/aspnet-blazor-eng perspective.

Thanks @dougbu, TIL. Unless the Blazor team disagrees, I will keep the PR as-is and include a co-author credit to @SteveSandersonMS in the squashed merge commit. We were already tracking issues here so I don't expect a need for the consolidation infra.

[SteveSandersonMS]

This looks great to me.

However since I wrote the QuickGrid implementation I'm not sure we get the most value if I'm the one approving this PR :) @Nick-Stanton, if you feel the implementation is clear to you then we're probably good to go with it. But if there are aspects that seem unclear we might also want to pull in at least one other team member to inspect it, share out an understanding of what's going on internally, and maybe spot any opportunities to improve it further.

[MackinnonBuck]

This looks good to me!

Should we consider sealing some (most?) of these public types? I don't see why someone would need to derive from GridSort<TGridItem>, or Defer, for example.

[Nick-Stanton]

This looks good to me!

Should we consider sealing some (most?) of these public types? I don't see why someone would need to derive from GridSort<TGridItem>, or Defer, for example.

Great point. I sealed what I knew for sure would be good, and added [EditorBrowsable(EditorBrowsableState.Never)] on a couple infra classes that ideally would be private/internal but must be public. There are public classes without inheritors that I didn't seal, as I could see places that a developer may want to inherit and build on.

[Nick-Stanton]

@dotnet/aspnet-blazor-eng @dotnet/aspnet-build This should be ready for final review. Failures are in the quarantined pipeline. The last commit includes everything from API review with the exception of a couple findings in an email thread. I will also include them below:

1. Adding Attributes dictionary to the grid – historically, this is accomplished relatively easily during a BuildRenderTree() override that many of the components we (Blazor) provide have built-in. However, the current implementation doesn’t allow for this override due to the complexity of the QuickGrid component. Because of this, I could not find an easy way to add an unknown number of attributes to this component.

EDIT: ISortBuilderColumn has been removed.

[dougbu]

@dotnet/aspnet-build only owns a couple of files in this PR. I made a couple of comments there but am not approving because I ignored almost everything else.

[Nick-Stanton]

While technically not correct, the compiler would not eat the original line. This may be due to the existence of a struct with a generic parameter and a static class (that just grabs instances of the struct) with the same name. Regardless, the actual output of this is reduced to ValueTask<TResult> anyways, so this clarification isn't any more useful.

[Nick-Stanton]

Blocked on timeouts in the Helix pipeline

[Nick-Stanton]

@SteveSandersonMS has offered to bring this across the finish line now that I'm in Azure Core. Here's what's remaining:

1. Everything is good to merge once the Helix timeline doesn't time out. Please make sure that Steve is listed as a co-author in the merge commit 😄
2. Backport to .NET 8 preview 2. This is very low risk because these projects are being consumed as NuGet packages.
3. Ping Artak

CC: @mkArtakMSFT

[SteveSandersonMS]

Thanks very much @Nick-Stanton! It's merged, the backport PR is here, and @mkArtakMSFT has been pinged.

[sjd2021]

This looks good to me!

Should we consider sealing some (most?) of these public types? I don't see why someone would need to derive from GridSort<TGridItem>, or Defer, for example.

Because you can't envision a need to derive from it, it shouldn't be open to it? That seems flawed. Why don't we seal things that we don't want inherited for a specific reason?

[ghost]

Hi @sjd2021. It looks like you just commented on a closed PR. The team will most probably miss it. If you'd like to bring something important up to their attention, consider filing a new issue and add enough details to build context.

[Nick-Stanton]

This looks good to me!
Should we consider sealing some (most?) of these public types? I don't see why someone would need to derive from GridSort<TGridItem>, or Defer, for example.

Because you can't envision a need to derive from it, it shouldn't be open to it? That seems flawed. Why don't we seal things that we don't want inherited for a specific reason?

@sjd2021 in general, types that aren't intended to be extensible are sealed to provide potential startup and size optimizations. While this guidance generally applies to private types within .NET, limitations in the Razor compiler prevented some of these types from being non-public. QuickGrid by design has multiple points of extension for developers that want to build on top of it without a custom fork of the runtime, but just not at the layer of types like GridSort<TGridItem> and Defer. If you disagree with this process, then I encourage you to open an issue so that the Blazor team can discuss further with you - I recently moved to a different team within Microsoft so my understanding of these decisions may be outdated.

[ghost]

Hi @Nick-Stanton. It looks like you just commented on a closed PR. The team will most probably miss it. If you'd like to bring something important up to their attention, consider filing a new issue and add enough details to build context.

[sjd2021]

@Nick-Stanton The more you know.. Thanks for the thoughtful explanation. I've been annoyed at how trivial adjustments or tests I've wanted to make were made difficult because I wanted to extend a sealed class that had no interface. I hadn't thought much about the benefits you've referenced. This helps.

[ghost]

Hi @sjd2021. It looks like you just commented on a closed PR. The team will most probably miss it. If you'd like to bring something important up to their attention, consider filing a new issue and add enough details to build context.