Skip to content

Add in-app network editor#3

Merged
wnj00524 merged 1 commit into
masterfrom
codex/wpf-network-simulator
Apr 7, 2026
Merged

Add in-app network editor#3
wnj00524 merged 1 commit into
masterfrom
codex/wpf-network-simulator

Conversation

@wnj00524
Copy link
Copy Markdown
Owner

@wnj00524 wnj00524 commented Apr 7, 2026

Allow users to create new networks and edit traffic types, nodes, profiles, and edges directly in the WPF application.

Allow users to create new networks and edit traffic types, nodes, profiles, and edges directly in the WPF application.
@wnj00524 wnj00524 merged commit 0bbc54a into master Apr 7, 2026
0 of 2 checks passed
@wnj00524 wnj00524 deleted the codex/wpf-network-simulator branch April 7, 2026 13:30
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces comprehensive in-app network editing capabilities to the MedW Network Simulator. Key changes include replacing static text displays with editable text boxes for network metadata, adding a 'Network Editor' tab with data grids for managing traffic types, nodes, and edges, and refactoring ViewModels to support two-way data binding and dynamic updates. The feedback focuses on improving the user experience by using combo boxes for ID selection, centralizing hardcoded colors into resources, and optimizing performance during bulk updates to prevent redundant UI refreshes.

Padding="10,8"
FontSize="28"
FontWeight="SemiBold"
Background="#FFFDF9F3"
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The background color #FFFDF9F3 is hardcoded here and in several other places (e.g., lines 48, 561, 611, 681). To ensure consistency and support future theming, consider defining these colors as StaticResource entries in App.xaml or a dedicated resource dictionary.

SelectionMode="Single"
SelectionUnit="FullRow">
<DataGrid.Columns>
<DataGridTextColumn Binding="{Binding TrafficType, Mode=TwoWay}" Header="Traffic Type" Width="*" />
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Using a DataGridTextColumn for the traffic type requires users to manually type the name, which is error-prone. Since TrafficTypeNameOptions is available in the ViewModel, using a DataGridComboBoxColumn would provide a much better user experience.

                                                <DataGridComboBoxColumn Header="Traffic Type" Width="*" SelectedItemBinding="{Binding TrafficType, Mode=TwoWay}" ItemsSource="{Binding DataContext.TrafficTypeNameOptions, RelativeSource={RelativeSource AncestorType=Window}}" />

Comment on lines +711 to +712
<DataGridTextColumn Binding="{Binding FromNodeId, Mode=TwoWay}" Header="From" Width="82" />
<DataGridTextColumn Binding="{Binding ToNodeId, Mode=TwoWay}" Header="To" Width="82" />
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The From and To node identifiers are currently editable via text boxes. It is highly recommended to use DataGridComboBoxColumn bound to NodeIdOptions to allow users to select existing nodes from a dropdown, preventing invalid references.

                                                <DataGridComboBoxColumn Header="From" Width="82" SelectedItemBinding="{Binding FromNodeId, Mode=TwoWay}" ItemsSource="{Binding DataContext.NodeIdOptions, RelativeSource={RelativeSource AncestorType=Window}}" />
                                                <DataGridComboBoxColumn Header="To" Width="82" SelectedItemBinding="{Binding ToNodeId, Mode=TwoWay}" ItemsSource="{Binding DataContext.NodeIdOptions, RelativeSource={RelativeSource AncestorType=Window}}" />


private void RefreshTrafficSummariesFromCurrentState()
{
TrafficTypes.Clear();
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Calling TrafficTypes.Clear() whenever the network structure changes causes the UI to lose the current selection in the traffic list. This can be disruptive when editing. Consider synchronizing the collection or capturing and restoring the SelectedTraffic by name after the refresh.

Comment on lines +603 to +619
private void HandleNodeIdChanged(object? sender, ValueChangedEventArgs<string> e)
{
foreach (var edge in Edges)
{
if (Comparer.Equals(edge.FromNodeId, e.OldValue))
{
edge.FromNodeId = e.NewValue;
}

if (Comparer.Equals(edge.ToNodeId, e.OldValue))
{
edge.ToNodeId = e.NewValue;
}
}

Edges.Add(new EdgeViewModel(edgeModel, sourceNode, targetNode));
RefreshDerivedStateAfterStructureChange("Updated node identifiers and any connected edges.");
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Updating node identifiers or traffic type names triggers a full RefreshDerivedStateAfterStructureChange for every connected edge or profile updated in the loop. For networks with many connections, this redundant processing (clearing collections, recalculating workspace, etc.) could lead to UI performance issues. Consider suppressing refreshes until the bulk update is complete.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant