AdminTool: Enter saves and Esc cancels in the New Item and New Robot dialogs - #54
Merged
Sellafield merged 1 commit intoAug 17, 2026
Conversation
The other sixteen AdminTool dialogs already pair IsDefault on their confirm button with IsCancel on Cancel. These two were the exceptions: Enter did nothing and Esc did nothing. IsDefault on its own would have been a regression. Enter activates the default button without moving focus, so a TextBox bound with the default LostFocus trigger is still holding an uncommitted value when Save runs, and roughly fifteen fields in these dialogs are bound that way -- Mass, Volume, Health and Quantity among them. Measured rather than assumed: with the hook removed, typing 555 into Mass and pressing Enter produced an INSERT carrying mass = 0. The PreviewKeyDown handler pushes the focused single-line TextBox to its source first. Multi-line boxes consume Enter themselves and are left alone. Verified by hand against the local database in SQL script mode, since no test tier covers the AdminTool: Enter saves and the typed value reaches the generated SQL, Esc closes without saving, Enter inserts a newline in the Note box, and Enter still commits a DataGrid cell rather than saving the item. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Enter now saves and Esc now cancels in the New Item and New Robot dialogs of the AdminTool.
Where this came from, since it explains the scope
The original idea was for the game client: make Enter activate the OK/Confirm button in popups that
already open with their text field focused — the Shift-drag stack-split dialog is the obvious case,
where you type the quantity and then still have to reach for the mouse.
That is not something this repository can do, and the new client is in progress and not shareable, so
the idea is parked until it is. This pull request is the same idea applied where it is reachable, and
it was deliberately used as a small exercise: pick a bounded change, in an area with an existing
pattern to copy, and see whether it can be made without breaking anything around it.
It stands on its own regardless of that origin. Sixteen AdminTool dialogs already pair
IsDefaultontheir confirm button with
IsCancelon Cancel. These two were the only exceptions: Enter did nothingand Esc did nothing, in the two largest data-entry dialogs in the tool. So this is quality of life, and
it removes an inconsistency rather than introducing a new convention.
The part worth reviewing
IsDefault="True"on its own would have been a regression, which is why this is four files instead oftwo attributes.
Enter activates the default button without moving focus. A
TextBoxbound with WPF's defaultUpdateSourceTrigger=LostFocusis therefore still holding an uncommitted value whenSaveCommandruns. Roughly fifteen fields across these two dialogs are bound that way —
Mass,Volume,Healthand
Quantityamong them; onlyDefinitionName,CategoryFlags,AttributeFlagsandDescriptionTokenusePropertyChanged.Measured rather than argued. With the hook removed, typing
555into Mass and pressing Entergenerated:
With the hook in place, the typed value reaches the generated SQL. So without the
PreviewKeyDownhandler, adding the shortcut would have silently written blank fields — worse than having no shortcut.
The handler pushes the focused single-line
TextBoxto its source before the key propagates.Multi-line boxes are skipped explicitly: they consume Enter themselves, so the Note field still inserts
a newline.
IsCancelwas the other thing checked rather than assumed.CancelCommandalready closes the dialogthrough
CloseRequested, soIsCanceladds a second path. That is not new: the existing sixteendialogs all pair
IsCancel="True"with aClickhandler that setsDialogResult = false, so the samedouble set is already in production and works.
Validation
No test tier covers the AdminTool, so this was verified by hand against a local database with the
session in SQL-script mode, which writes a file instead of touching the database:
mass = 0— the regression observed failing, per therepository's own rule
DataGridcell and moves down rather than saving the item — this one mattered,since the handler runs on the window's
PreviewKeyDown, before the grid sees the keySolution builds clean; the unit and integration tiers and the smoke script were run and are unaffected,
as none of them reach the AdminTool.
One question
If you would rather the two dialogs matched the other sixteen exactly —
Clickhandlers instead ofCommandbindings — say so and I will convert them. I left the MVVM shape alone because changing itwould be a larger diff than the behaviour being added.