-
Notifications
You must be signed in to change notification settings - Fork 7.6k
Keyboard Controls for Quick Editors Unit Tests #1273
Conversation
var editor, | ||
pos = {line: 3, ch: 1}, | ||
lineBefore, | ||
lineAfter; |
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.
- lineAfter is unused
- lineBefore is assigned but never read, so I think it can be removed also
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.
Good catch!
// get text before insert operation | ||
lineBefore = editor.document.getLine(pos.line); | ||
|
||
CodeHintManager.handleKeyEvent(editor, e); |
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.
Since we have this simulateKeyEvent() utility now, would it be cleaner / more consistent to use it here too?
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.
Randy mentioned that this one requires a modifier key (Ctrl), and it seems nontrivial to add modifier-key support to simulateKeyEvent()... so it's fine by me to leave this as-is.
Done reviewing |
done with updates |
Great! Merging now. Good catch on adding that extra check to InlineEditorProviders-test, btw... |
Unit Tests for Esc key handling (Quick Editors, code hints, and context menu) Also, add new utility to SpecRunnerUtils for simulating key events
Added a new function to SpecRunnerUtils for simulating keyboard events. Currently no way to specify modifier keys.
Added test to close an inline editor via Esc key. There are currently no unit tests for multiple inline editors, so there is no test for closing more than 1 via Esc key.
Added test for code hints.
Added test for closing context-menu via Esc Key. Opened issue #1270 for contextMenuClose event. There are no unit tests for closing main menus, so no tests were added for Esc key.