Desktop: Resolves #275: Adds spell checker support

[laurent22]

This pull request adds spell checker support for the Rich Text editor and the note title using Electron native spell checker. Has been tested so far on macOS and Windows.

Support for Markdown editor still needs to be investigated as it's not clear if it can use the browser built-in spell checker. Some ideas there: codemirror/codemirror5#1017

[CalebJohn]

Spellchecking with CodeMirror should work if the textarea is changed to contenteditable and spellcheck is set, just like in #3349. I've been testing locally and the original bug that had us using textarea doesn't seem to be an issue any more (using keys like Ctrl-B would cause the cursor to move to another location in the document).

However one issue is that we'll need to disable spellchecking for inner modes (which I don't think will be terribly difficult, but we'll likely have to patch codemirror/mode/markdown/markdown.js). Some examples of bad spellchecking

[laurent22]

That's brilliant, I've tried to add the spellcheck attribute to the current code but that did nothing, and didn't thought about using a textarea. If we can keep the same feature set with the text area, then it should be easy to add spell checking to it. I expect it will be pretty much the same code as for TinyMCE.

As for highlighting within code blocks I wouldn't worry about it for now. Let's see what feedback we get first, as it could also be a feature. For example, someone could add the Katex or Mermaid keywords to their dictionary and thus use this as a tool to validate their code.

[laurent22]

@CalebJohn, I've tried to get it working again with CodeMirror but I couldn't get it to underline spelling mistakes as in your screenshot. Could you let me know how you've done this please?

In my test, I've changed:

const cm = CodeMirror(editorParent.current, cmOptions);

to:

const cm = CodeMirror.fromTextArea(editorParent.current, cmOptions);

And:

<div style={props.style} ref={editorParent} />

To:

<textarea contentEditable spellCheck style={props.style} ref={editorParent} />

But that didn't help.

[CalebJohn]

It's actually much easier than that, I should have included the fix in my previous comment.
You just need to edit one of the cmOptions

		const cmOptions = Object.assign({}, {
			value: props.value,
			screenReaderLabel: props.value,
			theme: props.codeMirrorTheme,
			mode: props.mode,
			readOnly: props.readOnly,
			autoCloseBrackets: props.autoMatchBraces,
+			inputStyle: 'contenteditable',
-                       inputStyle: 'textarea', // contenteditable loses cursor position on focus change, use textarea instead
			lineWrapping: true,
			lineNumbers: false,
			indentWithTabs: true,
			indentUnit: 4,
			spellcheck: true,
			allowDropFileTypes: [''], // disable codemirror drop handling
			keyMap: props.keyMap ? props.keyMap : 'default',
		}, userOptions);

[laurent22]

Thanks @CalebJohn, I got spell checking working with inputStyle: "contentEditable" however this mode is very buggy so I don't think we can use it. Selection breaks, note content sometimes is not saved, cursor movements don't work well, etc. I found this thread about it: https://discuss.codemirror.net/t/what-are-the-pitfalls-of-using-inputstyle-contenteditable-in-2020/2389

So I think for now we'll have to stick with "textarea" mode, but it means spell checking, underlining words and finding dictionary suggestions will have to be done manually. I think I'll leave that aside for now and maybe look at it later.

Work for this is in this branch: https://github.com/laurent22/joplin/tree/spellckecking_codemirror

[CalebJohn]

Thanks for looking into it, those are some of the same issues I had originally encountered with contentEditable, but for some reason wasn't able to duplicate them the other day.

That's unfortunate, but it shouldn't be too hard to integrate. Others have done it and the code looks fairly simple.

[laurent22]

Yes I expect it can be done, although it doesn't look like there's an easy drop-in solution.

[laurent22]

Merging this for now because the branch is a bit of a mess due to some TypeScript refactoring, so we can start clean if we add support for CodeMirror.

[CalebJohn]

I spent more time looking into adding spellcheck for codemirror. It seems that everyone does it by integrating typo.js or some other spellchecker. I can't see a way to do it with the integrated spellcheck and use CodeMirror.

However, we do have options.

1. We can add a setting to make codemirror contenteditable and thus have spellcheck. Lately I wasn't able to duplicate the bugs with it so perhaps it will be stable for some users. (I don't think this is a good option)
2. Spell checking mode like in old school spell checkers. When users initiate spellcheck it will pop them over to a plain editor component that highlights misspelled words and jumps between them. (This might work nicely had a plugin).
3. Integrate a seperate spell checker for codemirror that relies on typo.js or something similar.

@laurent22 what are your thoughts? Do you see an alternative? Do you think any of these methods would work in Joplin?

[fosslinux]

Hooray! Thanks so much @laurent22 and @CalebJohn for this development on Joplin, really appreciate the work you guys do

[pr0n1x]

Thank you so much for Spell Checker!

[CalebJohn]

@laurent22 I've been thinking more about making the spellchecker available for the markdown editor. First off, I found a couple of things on the codemirror side that suggests contenteditable is getting more stable. This prompted me to do more testing on my local build, where again I couldn't duplicate any issues that I had experience in the past (Aside from a variant of this issue which is fairly minor).

What I'm thinking might be the best path forward is that I'll create a Joplin plugin that adds a toggle spellcheck command which toggles contenteditable. This keeps the less stable code out of the main app and makes it more clear that the issue comes from spellcheck.

The only hitch with this plan is that it would probably require additional code on the app side to support the context menu word replacement. Would you accept a pull request that adds that structure?

[laurent22]

@CalebJohn, your approach is reasonable, however with the kind of bugs I had when I've tried contentEditable, I feel it's risky to use that mode. People will enable spellchecking and leave it on, and they'll have many issues, including loss of data, so it won't matter to them that they are in a different mode.

The two issues I remember for example is that I make some changes and nothing is saved, as if the "onChange" handler is not triggered. The cursor was also broken so I would type something and it would write it elsewhere. I'm not sure why it seems to be more stable for you, maybe it's OS specific, but I'll give it another try to be sure.

[laurent22]

Yes I can still replicate it consistently by creating a new note, then typing some text. It seems only the first letter is saved and the rest is ignored. And although CodeMirror is displaying all this text, internally it's clear it only knows about the first letter "d" because I can't move past it afterwards.

So it's not just the onChange handler not being triggered but something internal to CodeMirror that doesn't track the text being typed properly. This was with my test branch here is that makes a difference: 305e4c8 With Windows WLS and X-Server (so equivalent to Linux)

[CalebJohn]

@laurent22 Thanks for checking into it again. That image is really bad and I agree we wouldn't really want to release anything that does that.

I went into your branch to see if maybe there was a difference between how you had set it up and I was able to duplicate the above issues on your branch but not mine. The issue being that you had made the parent div contenteditable when only the codemirror needs to be. I have a branch that works well for me. Do you mind testing there to see if you still have some of the same issues?

One thing I was thinking I might do if we still think contenteditable mode is too unstable would be to toggle readonly when contentedible is turned on, this would allow users to see spelling errors and correct them via the context menu, but they wouldn't run into any editing related bugs.

[laurent22]

@CalebJohn, right that makes sense, thanks for checking.

One thing I was thinking I might do if we still think contenteditable mode is too unstable would be to toggle readonly when contentedible is turned on, this would allow users to see spelling errors and correct them via the context menu, but they wouldn't run into any editing related bugs.

I see what you mean but I feel it wouldn't be great in terms of UX. I wonder if instead we could add a checkbox in the settings to allow CodeMirror spellchecker, and there we can make it clear it's still a beta feature. Like "Enable spell checker in Markdown editor (BETA)" with a description below like "Spell checker in the Markdown editor was previous unstable however it appears to be more reliable now. If you notice any issue, please report it on the forum or GitHub.". Users are more receptive to this and that might help us learn about various edge cases. And if it turns out the feature is stable, we could enable it permanently. What do you think?

By the way, what issue did you previously have with contentEditable mode? Perhaps we should mention that in the setting description.

[CalebJohn]

@laurent22 I like that idea, I'll place the setting under the advanced options to hopefully make it even more clear.

I'll follow up with a pull request in the next couple days.

[IllyaYalovyy]

Thank you guys for this. The best note taking tool just became perfect! Really appreciate the work you are doing!