-
-
Notifications
You must be signed in to change notification settings - Fork 5k
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
Desktop: Fixes #10007: Fixed Toggle Comment & Delete/Duplicate/Sort Line Options in Beta Editor #10016
Conversation
…tions in Beta Editor
@@ -57,4 +57,23 @@ describe('CodeMirrorControl', () => { | |||
expect(control.execCommand('myTestCommand')).toBe('test'); | |||
expect(command).toHaveBeenCalledTimes(1); | |||
}); | |||
|
|||
it('toggleComment should work', () => { |
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.
Please use the same pattern as other tests - it('should...
. And generally we should use plain English in the description so something it('should toggle comments'...
expect(control.getValue()).toBe('Hello\nWorld\n'); | ||
}); | ||
|
||
it('deleteLine should work', () => { |
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.
Same here
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.
updated it!
I also added the duplicate line option in the v6 editor. It required a custom EditorCommandFunction, as far as I have tested the v5 version, it has the same behaviour. Also added tests for it |
Also added the sort selected lines option in v6 editor, and its corresponding EditorCommandFunction. It is based on the old v5 plugin present at https://github.com/laurent22/joplin/blob/dev/packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/utils/useLineSorting.ts Also added tests. |
so noticed that swapLine also did not have a test so added that as well, have made all tests independent of editorCommands as suggested |
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.
I've left one minor stylistic comment. Aside from that, this looks good to me!
I've tested it locally on Linux (OpenSUSE) and the commands all seem to work.
Thank you for working on this!
@PackElend label me please, Fixes #10007