View
The XAML is rather... simplistic:
Each label and textbox (and the Save button) stretches all the way across the window.
Instead of just dumping controls into a StackPanel, you could consider leveraging automagic layout with a WrapPanel, and making each label+textbox duet logically regrouped.
I like that you have no WinForms-like absolute positioning, but I would have expected to see margins, or styles. I'd recommend further nesting in your XAML - use a Grid to define sections, regroup label+textbox fields in their own StackPanel, and use styles to make all fields feel the same, without having to set MinWidth and fonts and other things every time.
Having your controls logically regrouped also visually regroups them, and your user has a better experience. Here I've removed the "Work Order Name" textblock, and replaced it with a GroupBox. Here's the XAML markup for the above screenshot, I'd like to highlight this part:
<WrapPanel DataContext="{Binding SelectedWorkOrder}">
Setting the container's data context allows everything under it to bind to the SelectedWorkOrder properties without having to qualify them, which makes clearer markup.
<Window.Resources>
<Style x:Key="InputField" TargetType="{x:Type TextBox}">
<Setter Property="MinWidth" Value="172" />
<Setter Property="Foreground" Value="Blue" />
</Style>
<Style x:Key="Heading" TargetType="{x:Type TextBlock}">
<Setter Property="FontWeight" Value="Bold" />
</Style>
</Window.Resources>
<Grid Margin="4">
<Grid.RowDefinitions>
<RowDefinition Height="Auto" />
<RowDefinition Height="*" />
<RowDefinition Height="32" />
</Grid.RowDefinitions>
<!-- search box -->
<GroupBox Grid.Row="0" Padding="4">
<GroupBox.Header>
<TextBlock Style="{StaticResource Heading}"
Text="Work Order Name" />
</GroupBox.Header>
<StackPanel Orientation="Horizontal">
<TextBox Text="{Binding WorkOrderName}"
HorizontalAlignment="Left"
Width="172" Margin="2"
AcceptsReturn="False">
<TextBox.InputBindings>
<KeyBinding Key="Enter" Command="{Binding GetWorkOrderCommand}" />
</TextBox.InputBindings>
</TextBox>
<Button Command="{Binding GetWorkOrderCommand}"
Margin="2"
Padding="2">
<Button.Content>
<StackPanel Orientation="Horizontal">
<Image Source="Images/find.png" />
<!-- <TextBlock Text="Search" /> -->
</StackPanel>
</Button.Content>
</Button>
</StackPanel>
</GroupBox>
<!-- form -->
<GroupBox Grid.Row="1" Padding="2" Margin="2"
IsEnabled="{Binding IsNewWorkOrder}">
<GroupBox.Header>
<TextBlock Style="{StaticResource Heading}"
Text="Work Order Info" />
</GroupBox.Header>
<ScrollViewer VerticalScrollBarVisibility="Auto">
<WrapPanel DataContext="{Binding SelectedWorkOrder}">
<StackPanel Margin="2">
<TextBlock Text="Part Number"/>
<TextBox Style="{StaticResource InputField}"
Text="{Binding PartNumber}"/>
</StackPanel>
<StackPanel Margin="2">
<TextBlock Text="Created"/>
<TextBox Style="{StaticResource InputField}"
Text="{Binding Created}"/>
</StackPanel>
<StackPanel Margin="2">
<TextBlock Text="Closed"/>
<TextBox Style="{StaticResource InputField}"
Text="{Binding Closed}"/>
</StackPanel>
<StackPanel Margin="2">
<TextBlock Text="First Number"/>
<TextBox Style="{StaticResource InputField}"
Text="{Binding FirstNumber}"/>
</StackPanel>
<StackPanel Margin="2">
<TextBlock Text="Last Number"/>
<TextBox Style="{StaticResource InputField}"
Text="{Binding LastNumber}"/>
</StackPanel>
</WrapPanel>
</ScrollViewer>
</GroupBox>
<!-- bottom panel-->
<Border Grid.Row="2" Background="WhiteSmoke" />
<WrapPanel Grid.Row="2"
HorizontalAlignment="Right">
<Button Margin="4" Padding="2"
Width="128"
HorizontalAlignment="Right"
Content="Save"/>
<Button Margin="4" Padding="2"
HorizontalAlignment="Right"
Content="Close"/>
</WrapPanel>
</Grid>
I noticed the Save button doesn't have a command binding. You may want to fix that ;)
Comments in XAML shouldn't try to explain anything:
<!--This textbox is incharge of searching the work order in the DB. [...]
I hope not. A textbox shouldn't be in charge of anything but collecting user input and displaying a value. This comment is misleading at best, and tries to explain the mechanics of the ViewModel, which has nothing to do with the textbox itself - help yourself, remove it.
I'm not sure why you're passing your command a parameter here:
<TextBox Text="{Binding WorkOrderName}" AcceptsReturn="False"> <TextBox.InputBindings> <KeyBinding Key="Enter" Command="{Binding GetWorkOrderCommand}" CommandParameter="{Binding Path=Text, RelativeSource={RelativeSource AncestorType=TextBox}}"/> </TextBox.InputBindings> </TextBox>
The ViewModel already knows what the WorkOrderName is, so the command binding can be simplified:
<TextBox Text="{Binding WorkOrderName}" AcceptsReturn="False"> <TextBox.InputBindings> <KeyBinding Key="Enter" Command="{Binding GetWorkOrderCommand}" /> </TextBox.InputBindings> </TextBox>
Model
Not much to say about the WorkOrder model, except that I find auto-properties don't really need the whitespace in-between:
public class WorkOrder
{
public int WorkOrderId { get; set; }
public string WorkOrderName { get; set; }
public string PartNumber { get; set; }
public DateTime Created { get; set; }
public DateTime? Closed { get; set; }
public long FirstNumber { get; set; }
public long LastNumber { get; set; }
}
This class is an entity type - I don't like that the View is binding directly to it. I would create another class, a WorkOrderViewModel, that implements the INotifyPropertyChanged interface - this class only needs to expose the properties of the Model that the View is interested in, and can easily know whether it's a "new" work order:
public class WorkOrderViewModel : INotifyPropertyChanged
{
private readonly WorkOrder _entity;
public WorkOrderViewModel()
: this(new WorkOrder())
{ }
public WorkOrderViewMode(WorkOrder entity)
{
_entity = entity;
}
public WorkOrder Model { get { return _entity; } }
public string WorkOrderName
{
get { return _entity.WorkOrderName; }
set
{
_entity.WorkOrderName = value;
NotifyPropertyChanged("WorkOrderName");
}
}
// ...
public bool IsNew { get { return _entity.WorkOrderId == 0; } }
}
ViewModel
The WorkOrderManagerViewModel can then expose SelectedWorkOrder as a WorkOrderViewModel.
Comments
Again, the comments in the ViewModel should explain what's going on in the ViewModel, not in the View - this is a waiting-to-become-a-blatant-lie comment:
//bound to the work order name textbox
This one is also not clear:
//on text changed - clear all values
"clearing all values" is the result of setting the SelectedWorkOrder to null... but only because of things that happen outside the ViewModel (specifically, because of the bindings in the View) - the comment would be better if reworded like this:
// text has changed, invalidate current selection.
This one is also something that the ViewModel has no control over:
//after enter was pressed - used to search if exists or not
If the command bindings change in the View, this commment becomes wrong, or incomplete - what if you added a Search button next to the text box, for convenience? What if more controls were added, with command bindings to this command?
Other than that, I find the name GetWorkOrder a little bit awkward for a method that returns void. I like naming my command methods CanExecuteXxxxx and ExecuteXxxxx - so that would be CanExecuteGetWorkOrder and ExecuteGetWorkOrder - these names also help justifying the object parameter that the ICommand interface wants.
The CanSave logic could be simplified:
if (SelectedWorkOrder == null) return false; return true;
Is really nothing more than return SelectedWorkOrder != null;.