-
Notifications
You must be signed in to change notification settings - Fork 1k
related to issue 10466: add short cut ↑, ↓, ←, → to demo console #13513
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #13513 +/- ##
===================================================
+ Coverage 76.59791% 76.59837% +0.00046%
===================================================
Files 3230 3230
Lines 639165 639165
Branches 47297 47297
===================================================
+ Hits 489587 489590 +3
+ Misses 146007 145999 -8
- Partials 3571 3576 +5
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Adds arrow key move shortcuts to the demo console’s design surface and cleans up the initialization loop.
- Extend the
DoAction
mapping to includeKEYMOVE↑/↓/←/→
commands - Hook arrow key Up/Down/Left/Right in
MainForm
to invoke move actions - Refactor
InitFormDesigner
to use a single loop and unified variable naming
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
src/test/integration/DesignSurface/DesignSurfaceExt/DesignSurfaceExt.cs | Added KEYMOVEUP , KEYMOVEDOWN , KEYMOVELEFT , KEYMOVERIGHT to the DoAction switch |
src/test/integration/DesignSurface/DemoConsole/MainForm.cs | Consolidated two loops into one, renamed isurf to designSurfaceExtended , and wired up KeyUp handlers for arrow keys |
Comments suppressed due to low confidence (1)
src/test/integration/DesignSurface/DesignSurfaceExt/DesignSurfaceExt.cs:353
- [nitpick] No automated tests verify the new arrow-key commands in
DoAction
. Add unit or integration tests to ensureKEYMOVEUP
,KEYMOVEDOWN
,KEYMOVELEFT
, andKEYMOVERIGHT
are correctly handled.
"KEYMOVEUP" => MenuCommands.KeyMoveUp,
_selectionService = (ISelectionService)(designSurfaceExtended.GetIDesignerHost().GetService(typeof(ISelectionService))); | ||
if (_selectionService is not null) | ||
{ | ||
_selectionService.SelectionChanged += OnSelectionChanged; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reassigning the single _selectionService
field inside the loop will overwrite previous services and only keep the last one. Consider storing each surface’s ISelectionService
separately or subscribing without relying on a single field.
_selectionService = (ISelectionService)(designSurfaceExtended.GetIDesignerHost().GetService(typeof(ISelectionService))); | |
if (_selectionService is not null) | |
{ | |
_selectionService.SelectionChanged += OnSelectionChanged; | |
var selectionService = (ISelectionService)(designSurfaceExtended.GetIDesignerHost().GetService(typeof(ISelectionService))); | |
if (selectionService is not null) | |
{ | |
_selectionServices[i] = selectionService; | |
selectionService.SelectionChanged += OnSelectionChanged; |
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All LGTM!
related to #10466
Proposed changes
After
Test methodology
manually
Microsoft Reviewers: Open in CodeFlow