Conversation
isidorn
added
the
accessibility
alexdima
left a comment
alexdima
left a comment
There was a problem hiding this comment.
LGTM. I just left some small tweaks suggestions.
src/vs/editor/editor.all.ts
Outdated
| import 'vs/editor/contrib/wordHighlighter/wordHighlighter'; | ||
| import 'vs/editor/contrib/wordOperations/wordOperations'; | ||
| import 'vs/editor/contrib/wordPartOperations/wordPartOperations'; | ||
| import 'vs/editor/contrib/anchorSelect/anchorSelect'; |
There was a problem hiding this comment.
Small nit: this list was alphabetically sorted.
| precondition: SelectionAnchorSet, | ||
| kbOpts: { | ||
| kbExpr: EditorContextKeys.editorTextFocus, | ||
| primary: KeyChord(KeyMod.CtrlCmd | KeyCode.KEY_K, KeyCode.KEY_K), |
There was a problem hiding this comment.
We should look if cmd+k cmd+k is available. You can check the availability across OSes here -- https://github.com/microsoft/vscode-docs/tree/master/build/keybindings
| precondition: undefined, | ||
| kbOpts: { | ||
| kbExpr: EditorContextKeys.editorTextFocus, | ||
| primary: KeyChord(KeyMod.CtrlCmd | KeyCode.KEY_K, KeyCode.KEY_B), |
There was a problem hiding this comment.
We should look if cmd+k cmd+b is available. You can check the availability across OSes here -- https://github.com/microsoft/vscode-docs/tree/master/build/keybindings
| className: 'selection-anchor' | ||
| } | ||
| }]); | ||
| this.decorationId = newDecorationId.length === 1 ? newDecorationId[0] : undefined; |
There was a problem hiding this comment.
It is safe to use newDecorationIds[0]. The length will always be 1 because 1 new decoration is passed in above.
| private editor: ICodeEditor, | ||
| @IContextKeyService contextKeyService: IContextKeyService | ||
| ) { | ||
| this.selectionAnchorSetContextKey = SelectionAnchorSet.bindTo(contextKeyService); |
There was a problem hiding this comment.
I think this should also listen to editor.onDidChangeModel and reset the context key if the editor gets a new model set.
|
@alexdima thanks a lot for the review. Tackled all comments. Merging. |
2 participants
fixes #95894
This PR introduces the following commands:
Cmd + K, BkeybindingCmd + K, KkeybdiningEsckeybindingAs for changning default keybindings I woud like to have this in insiders so users try it out and then we can change it.
Potential follow up work:
widthwith!important.@alexdima let me know what you think