search

FritzAndFriends / BlazorWebFormsComponents

A collection of Blazor components that emulate the ASP.NET Web Forms controls of the same name
MIT License
428 stars 72 forks source link

Change from Reflection to TypeDescriptors #223

Closed Eilon closed 4 years ago

Eilon commented 4 years ago

This more closely matches how System.Web WebForms work by using TypeDescriptors, which are an asbtraction over Reflection and custom metadata providers. This is how DataBinder, data bound controls, and other binding features in ASP.NET work.

Fixes #222

Eilon commented 4 years ago

And, of course, all tests pass without any significant changes. The only test change at all was changing from dynamic to object, which in my opinion makes a little more sense anyway.

csharpfritz commented 4 years ago

Ooh.. ok, this is the type of change I was looking for and couldn't quite put my finger on it for dealing with types.

I'm excited to take a look...

On Wed, Aug 26, 2020 at 2:19 PM Eilon Lipton notifications@github.com wrote:

And, of course, all tests pass without any significant changes. The only test change at all was changing from dynamic to object, which in my opinion makes a little more sense anyway.

— You are receiving this because your review was requested. Reply to this email directly, view it on GitHub https://github.com/FritzAndFriends/BlazorWebFormsComponents/pull/223#issuecomment-681044525, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAATF4LIE4VDCTD5EJQZQ33SCVG4FANCNFSM4QMDXUVQ .

Eilon commented 4 years ago

ComponentModel, which includes TypeDescriptor and PropertyDescriptor, is an entire world unto itself. It brings both laughs and tears. But for they are tears of joy, because you don't need to use Reflection APIs. And tears of sadness because you don't get to use Reflection APIs.

csharpfritz commented 4 years ago

Definitely an improvement, and something to consider for v2

Jeff

On Thu, Aug 27, 2020 at 1:24 PM Eilon Lipton notifications@github.com wrote:

@Eilon commented on this pull request.

In src/BlazorWebFormsComponents.Test/GridView/DataTableSupport.razor https://github.com/FritzAndFriends/BlazorWebFormsComponents/pull/223#discussion_r478578554 :

@@ -3,32 +3,32 @@

- + Yes that's a good point. I'm not familiar enough with how Blazor handles generic types to figure this out. We'll have to stew on it. For now I see it as an improvement over what was there before (you already had to specify a somewhat arbitrary ItemType, now it's just a different type). — You are receiving this because your review was requested. Reply to this email directly, view it on GitHub , or unsubscribe .
hishamco commented 4 years ago

I was thinking to remove the reflection stuff especially in the extension method, but seems you got it first ;) BTW I didn't find a time to review your PR because I made a surgery in my left ankle, but what it's done is great!!