xceedsoftware / wpftoolkit

All the controls missing in WPF. Over 1 million downloads.
Other
3.9k stars 878 forks source link

Collection Editor Add button is grayed out for a collection with no setter #1686

Open masonwheeler opened 3 years ago

masonwheeler commented 3 years ago
class Foo
{
   public ObservableCollection<Bar> Stuff {get;} = new();
}

This is a standard, idiomatic way to set up collections in C#. But when attempting to edit one in the Collection Editor, it considers the collection's item list to be read-only because the property itself is read-only.

Obviously this is not correct. As long as the class implements ICollection<T>, the item list is editable. If it implements IList<T>, it's indexable (and thus reorderable) as well.

XceedBoucherS commented 3 years ago

Hi, Simply set the NewItemTypes property. This will tell which types can be added in the collection: <xctk:CollectionControlButton x:Name="_collectionControlButton" ItemsSource="{Binding}" />

public partial class MainWindow : Window { public MainWindow() { this.DataContext = this; InitializeComponent();

  _collectionControlButton.NewItemTypes = new List<Type>() { typeof(Bar) };
}

public ObservableCollection<Bar> Stuff { get; } = new ObservableCollection<Bar>();

}

public class Bar { public string MyString { get; set; } }

masonwheeler commented 3 years ago

Yes, that can be done. But it doesn't really address the issue. If the exact same collection property is marked get/set, no NewItemTypes is needed. The problem, then, isn't that the code isn't already capable of determining the element type of a collection; the problem is that it's confusing a read-only collection property for a read-only collection item list.

XceedBoucherS commented 3 years ago

Hi,

Well, if I take the sample I've paste earlier, set the property "Stuff" to be a get/set and remove the line: _collectionControlButton.NewItemTypes = new List() { typeof(Bar) }; The CollectionEditor won't make the "Add" option available. The NewItemTypes must be set for the "Add" option to be available. You need to specify what type of object can be added. If for example you have the base class "Person" and the derive classes "Man" and "Woman"(which derive from "Person"), and your property is a collection of Person, you may want to Add Man and Woman, or only Man or only Woman. You need to specify it to the CollectionEditor. This is done with the NewItemTypes property.

masonwheeler commented 3 years ago

The NewItemTypes must be set for the "Add" option to be available. You need to specify what type of object can be added.

This is simply not true. Have a look at CollectionEditor.ResolveValueBinding. If you haven't set NewItemTypes, it will infer the type for you, and as long as the type fits the criteria set forth in CollectionControl.CanAddNewCore, the Add button will be enabled. The problem is that the check for this.IsReadOnly at the top of CanAddNewCore fails if the collection's property does not have a setter.

Setting NewItemTypes manually is only useful if

  1. you have types to add that aren't the collection's <T> type, as in the example given,
  2. you're using Collection Editor directly and not through PropertyGrid (or is there a way to set this up?), and
  3. you will only be working with one collection whose type parameters are known at compile time
XceedBoucherS commented 3 years ago

Hi, Ok, I was testing a CollectionControlButton, not a PropertyGrid. I understand your point. Here's a fix (it will be included in v4.3).

In file Xceed.Wpf.Toolkit/PropertyGrid/Implementation/Converters/PropertyItemEditorConverter.cs, Replace the method Convert() with: ` public object Convert( object[] values, Type targetType, object parameter, CultureInfo culture ) { if( ( values == null ) || ( values.Length != 3 ) ) return null;

  var editor = values[ 0 ];
  var isPropertyGridReadOnly = values[ 1 ] as bool?;
  var isPropertyItemReadOnly = values[ 2 ] as bool?;

  if( ( editor == null ) || !isPropertyGridReadOnly.HasValue || !isPropertyItemReadOnly.HasValue )
    return editor;

  // Get Editor.IsReadOnly
  var editorType = editor.GetType();
  var editorIsReadOnlyPropertyInfo = editorType.GetProperty( "IsReadOnly" );
  if( editorIsReadOnlyPropertyInfo != null )
  {
    if( !this.IsPropertySetLocally( editor, TextBoxBase.IsReadOnlyProperty )  )
    {
      // Set Editor.IsReadOnly to PropertyGrid.IsReadOnly & propertyItem.IsReadOnly.
      var isReadOnlyValue = isPropertyGridReadOnly.Value
                            ? true
                            : ( editor is PropertyGridEditorCollectionControl ) ? false : isPropertyItemReadOnly.Value;

      editorIsReadOnlyPropertyInfo.SetValue( editor, isReadOnlyValue, null );

    }
  }
  // No Editor.IsReadOnly property, set the Editor.IsEnabled property.
  else
  {
    var editorIsEnabledPropertyInfo = editorType.GetProperty( "IsEnabled" );
    if( editorIsEnabledPropertyInfo != null )
    {
      if( !this.IsPropertySetLocally( editor, UIElement.IsEnabledProperty ) )
      {
        // Set Editor.IsReadOnly to PropertyGrid.IsReadOnly & propertyItem.IsReadOnly.
        var isEnabledValue = isPropertyGridReadOnly.Value
                            ? false
                            : ( editor is PropertyGridEditorCollectionControl ) ? true : !isPropertyItemReadOnly.Value;

        editorIsEnabledPropertyInfo.SetValue( editor, isEnabledValue, null );
      }
    }
  }

  return editor;
}`
masonwheeler commented 3 years ago

Thanks!

Could you get someone to look at #1687, BTW? It's been sitting there for almost 3 weeks with no attention...

XceedBoucherS commented 3 years ago

Hi masonwheeler,

Issues reported here are not answered as fast as for the paying version available on https://xceed.com/en/our-products/product/words-for-net. Paying users have a 24 hours support while for the free version is answered when we have time.

Concerning #1867, I can tell that DbConnectionStringBuilder as never been used for testing the Toolkit so it may not work as expected. It looks like a more complex issue. We will eventually take a look at it, depending on our schedule and availability.

Thank you for your comprehension.