ListBox with a dynamic number of TextBox elements

Question

I am making a kind of form, and I want the user to be able to enter a dynamic number of string inputs. The idea is that a single TextBox will be displayed, and once the user has filled that out, they will be able to add another one, and repeat the process as they keep filling the text boxes. Once there are multiple, they can remove any text box if they no longer want it.

This is accomplished with a ListBox containing a custom DataTemplate which is a DockPanel with a TextBox and two Buttons.

XAML:

<ListBox ItemsSource="{Binding ExcludedPaths}">
 <ListBox.ItemContainerStyle>
 <Style TargetType="ListBoxItem">
 <Setter Property="Template">
 <Setter.Value>
 <ControlTemplate TargetType="ListBoxItem">
 <ContentPresenter />
 </ControlTemplate>
 </Setter.Value>
 </Setter>
 </Style>
 </ListBox.ItemContainerStyle>
 <ListBox.ItemTemplate>
 <DataTemplate>
 <DockPanel LastChildFill="True">
 <Button DockPanel.Dock="Right"
 Content="+"
 Command="{Binding DataContext.AddExcludedPathCommand,
 RelativeSource={RelativeSource AncestorType=ListBox}}"
 CommandParameter="{Binding}"
 Visibility="{Binding AddButtonVisibility}" />
 <Button DockPanel.Dock="Right"
 Content="-"
 Command="{Binding DataContext.RemoveExcludedPathCommand,
 RelativeSource={RelativeSource AncestorType=ListBox}}"
 CommandParameter="{Binding}" />
 <TextBox DockPanel.Dock="Left"
 Text="{Binding Path,
 Mode=TwoWay,
 UpdateSourceTrigger=PropertyChanged}"
 HorizontalAlignment="Stretch"
 HorizontalContentAlignment="Stretch" />
 </DockPanel>
 </DataTemplate>
 </ListBox.ItemTemplate>
</ListBox>

ExcludedPath:

public class ExcludedPath : INotifyPropertyChanged
{
 private Visibility _addButtonVisibility;
 private string _path;
 public Visibility AddButtonVisibility
 {
 get
 {
 return _addButtonVisibility;
 }
 set
 {
 _addButtonVisibility = value;
 NotifyPropertyChanged();
 }
 }
 public string Path
 {
 get
 {
 return _path;
 }
 set
 {
 _path = value;
 NotifyPropertyChanged();
 }
 }
 public event PropertyChangedEventHandler PropertyChanged;
 private void NotifyPropertyChanged(
 [CallerMemberName] string propertyName = "")
 {
 PropertyChanged?.Invoke(
 this,
 new PropertyChangedEventArgs(propertyName));
 }
}

ViewModel (Relevant Portions):

internal ViewModel()
{
 SetUpProperties();
}
public ICommand AddExcludedPathCommand =>
 new RelayCommand(AddExcludedPath, CanAddExcludedPath);
public ObservableCollection<ExcludedPath> ExcludedPaths { get;
 private set; }
public ICommand RemoveExcludedPathCommand =>
 new RelayCommand<ExcludedPath>(
 RemoveExcludedPath, CanRemoveExcludedPath);
private void AddExcludedPath()
{
 ExcludedPaths.Add(new ExcludedPath());
}
private bool CanAddExcludedPath()
{
 return ExcludedPaths.All(x => !x.Path.IsNullOrWhiteSpace());
}
private bool CanRemoveExcludedPath(ExcludedPath excludedPath = null)
{
 return ExcludedPaths.Count > 1;
}
private void OnExcludedPathsChanged(
 object sender, NotifyCollectionChangedEventArgs e)
{
 foreach (var excludedPath in ExcludedPaths)
 {
 excludedPath.AddButtonVisibility = Visibility.Hidden;
 }
 ExcludedPaths.Last().AddButtonVisibility = Visibility.Visible;
}
private void RemoveExcludedPath(ExcludedPath excludedPath)
{
 ExcludedPaths.Remove(excludedPath);
}
private void SetUpProperties()
{
 ExcludedPaths = new ObservableCollection<ExcludedPath>();
 ExcludedPaths.CollectionChanged += OnExcludedPathsChanged;
 ExcludedPaths.Add(new ExcludedPath());
}

Answer 1

ExcludedPath

If the value of the properties aren't changed you won't need to raise the PropertyChangedEvent so a simple check like

public Visibility AddButtonVisibility
{
 get
 {
 return _addButtonVisibility;
 }
 set
 {
 if (_addButtonVisibility == value)
 {
 return;
 }
 _addButtonVisibility = value;
 NotifyPropertyChanged();
 }
} 

will be better.

---

ViewModel

private bool CanRemoveExcludedPath(ExcludedPath excludedPath = null)
{
 return ExcludedPaths.Count > 1;
} 

You don't use the method parameter, just remove it.

• Michael Brandon Morris: As a note, the parameter is required, otherwise there is an error, due to the RemoveExcludedPath method which does require a parameter. I can't mix parameter-required and parameter-less methods in a RelayCommand it seems.

Answer 2

There is not much to improve...

additional to the points from Heslacher:

a) That code:

foreach (var excludedPath in ExcludedPaths)
{
 excludedPath.AddButtonVisibility = Visibility.Hidden;
}
ExcludedPaths.Last().AddButtonVisibility = Visibility.Visible;

could be optimized to

var last = ExcludedPaths.Last();
foreach (var excludedPath in ExcludedPaths)
{
 excludedPath.AddButtonVisibility = excludePath == last ? 
 Visibility.Visible : Visibility.Hidden;
}

That avoids to set the last propery twice.

b) If that is actually your productive code, I would recomment to use a MVVM framework that provides a more comfortable way to raise property changed events. At least implement your own base class that implements INotifyPropertyChanged.

c) In XAML, you don't need the CommandParameter binding for the AddExcludedPathCommand, therfore I would remove it.

---

d) Maybe a question of philosophy,

but I don't like properties of type Visibility in view models. A boolean property is more appropriated to represent the logical state of something in the view model. The transformation from the logical state to it's visual state should be part of the view / XAML (using a BoolToVisibility converter).

But that is probably just a matter if taste...