Megabit / Blazorise

Blazorise is a component library built on top of Blazor with support for CSS frameworks like Bootstrap, Tailwind, Bulma, AntDesign, and Material.
https://blazorise.com/
Other
3.25k stars 526 forks source link

Support INotifyPropertyChanged in _DataGridRow.razor? #3019

Open MihailsKuzmins opened 2 years ago

MihailsKuzmins commented 2 years ago

Is your feature request related to a problem? Please describe. Problem: when a property is updated on the model, the change is not reflected in the UI

Describe the solution you'd like Somewhere around this place implement handling for INotifyPropertyChanged https://github.com/Megabit/Blazorise/blob/9cc9ee55e285588fd264e09f58324d33ad94b65f/Source/Extensions/Blazorise.DataGrid/_DataGridRow.razor.cs#L206-L209

something like...

@implements IDisposable

[Parameter]
public TItem? Item
{
    get => _item;
    set
    {
        if (_item.IsEqual(value))
            return;

        _item?.PropertyChanged  -= ItemOnPropertyChanged;
        _item = value;

        if (_item is INotifyPropertyChanged propChanged)
            propChanged.PropertyChanged += ItemOnPropertyChanged;
    }
}

Dispose() _item?.PropertyChanged  -= ItemOnPropertyChanged;

private ItemOnPropertyChanged(e)
{
    if (e.Property == this.Field)
        StateHasChanged()
}

Additional context

If you accept this feature request I could also implement it - does not seem difficult... at the moment 😄

stsrki commented 2 years ago

@David-Moreira This might be possible with the changes you're currently doing on the DataGrid rows?

MihailsKuzmins commented 2 years ago

P.S. maybe add support for INotifyCollectionChanged as well? Or is Filtering the preferred way of updating the collection?

David-Moreira commented 2 years ago

It seems to me this always needs some kind of custom implementation on the users side right? Datagrid.refresh should already refresh the grid and be enough? Which would be the equivalent as the implementation above?

I don't see the need to support this specific interface? Maybe this made more sense in desktop app development?

MihailsKuzmins commented 2 years ago

@David-Moreira that is the case - I reuse ViewModels on different platforms - desktop, mobile, web.

By default Blazor does not support INotifyPropertyChanged, so the UI is not updated out of the box which is not that cool... So i thought that that it could be nice if Blasorise added support for it, so to say, make it similar to desktop development

But if you don't see it as part of the library I guess the issue can be closed

David-Moreira commented 2 years ago

You're right, maybe we can consider it... with MAUI nowadays it probably makes more sense, even if it would not for web specifically?

@David-Moreira This might be possible with the changes you're currently doing on the DataGrid rows?

@stsrki what are you thinking here? Us tracking changes? I guess that'd be more costly then just supporting this interface.

stsrki commented 2 years ago

I don't think it needs to be in the setter of the Item. When you think about it we only need to trigger PropertyChanged after the item has been saved. But the thing is we already have the event that is invoked at that time. Named RowInserted, and RowUpdated. Before that, while editing, we don't do anything on an Item instance. Only on the internal dictionary that holds the Item values state.

Maybe in the end, PropertyChanged is not needed.

David-Moreira commented 2 years ago

That's not the use case the OP is solving @stsrki . He's saying that when he updates a property on his Item model (outside of datagrid updating mechanisms), he wants to automatically update the data inside the Datagrid. Like I said, supporting the interface would be an optional thing to do, and not a means to solve this, as we already support DataGrid.Refresh, which would solve this.

I almost don't do Desktop development, but I believe, supporting this interface would be just a matter of giving the option to our users to use common models, (IN MAUI for instance) that already support this interface, and would quickly work the same with DataGrid, without any extra code.