Keyboard navigation

[ravindu2012]

Summary

Brief description of the changes.

Type of Change

• Bug fix
• New feature
• Improvement/refactor
• Documentation
• Other

Related Issue

Fixes #(issue number)

Changes Made

• Change 1
• Change 2

Screenshots

If applicable, add screenshots of UI changes.

Checklist

• I have tested my changes locally
• My code follows Clean Architecture (dependencies flow inward)
• ViewModels follow MVVM pattern
• I have added tests for new business logic

[ravindu2012]

Code Review: PR #14 — Keyboard Navigation

What's Good

• Solid set of keyboard shortcuts (F1, F5, Ctrl+N/S/F, Esc)
• Access keys on form buttons (_Save, _Void, C_lear, P_rint) with correct mnemonics
• TabIndex ordering on interactive controls
• Keyboard shortcuts help dialog (F1) with theme support
• FocusSearch smartly matches TextBox by binding path (FilterText/SearchText) instead of fragile TabIndex

Issues

HIGH: ActionCommand should be in its own file or use CommunityToolkit's RelayCommand

ActionCommand is defined as a public class at the bottom of MainWindow.xaml.cs (outside the MainWindow class). This works but violates conventions — a public class should live in its own file. The project already uses CommunityToolkit.Mvvm which provides RelayCommand. Consider using that instead, or at minimum move ActionCommand to its own file under a Commands/ folder.

MEDIUM: Commands are allocated once but never raise CanExecuteChanged

All commands return CanExecute = true always, and CanExecuteChanged has empty add/remove accessors. This means:

• SaveCommand is enabled even when no tab is open
• CloseCurrentTabCommand is enabled when there's nothing to close
  This isn't a blocker but could confuse users. Consider implementing CanExecute properly or at least disabling the menu items contextually.

MEDIUM: ExecuteGlobalSave uses reflection to find SaveCommand

var prop = selectedItem.GetType().GetProperty("SaveCommand");

This is fragile — if the property is renamed or the ViewModel doesn't have it, it silently fails. Consider defining an ISaveable interface that ViewModels can implement, then check if (selectedItem is ISaveable saveable).

Same applies to ExecuteGlobalNew which searches for NewEntityCommand, NewItemCommand, NewCommand by reflection.

MEDIUM: ExecuteGlobalSave has a suspicious ExecuteAsync reflection call

var asyncMethod = command.GetType().GetMethod("ExecuteAsync");

ICommand.Execute is synchronous by design. CommunityToolkit's AsyncRelayCommand exposes ExecuteAsync, but reaching for it via reflection is fragile. Use IAsyncRelayCommand from CommunityToolkit directly:

if (command is IAsyncRelayCommand asyncCommand)
    await asyncCommand.ExecuteAsync(null);
else
    command.Execute(null);

LOW: Missing trailing newline at end of MainWindow.xaml.cs

LOW: Some formatting-only changes mixed in

Several lines were changed only to collapse multi-line if blocks to single lines (e.g., CloseTab, CloseAllTabs). While not harmful, mixing style changes with feature work makes the diff harder to review.