Auto-detect note language (for spelling and fonts)

[mirka]

Closes #949
Closes #788

This enhances the spellchecker by adding language auto-detection. It's working pretty well in my testing, and hopefully it will be a better experience than having to manually select a language from a dropdown.

Bonus: Better fonts on Windows

As a bonus, I'm using the auto-detected language to set a lang attribute to the note body. This will help Electron in Windows to select language-appropriate fonts when the primary OS language is not set to that (#788). One limitation to this is that it's only for the Note Editor, and not the NoteList, since that would be too costly. Still, it's an improvement over the current behavior.

To test

These changes only affect Electron on Windows and Linux. (Don't know why, but the language detection feature in electron-spellchecker is disabled on Mac. It's strange because it works if I just comment out that line.)

You can set localStorage.debug="electron-spellchecker:*" in the devtools console and reload to see the language switching in action. Language will be determined every time a different note is selected, or whenever the spelling/auto-detection menu items are toggled. If a language cannot be detected (either by being too short or ambiguous), it will use the OS language.

Test instructions on Linux/Windows

1. Enable Check Spelling in the Edit menu.
2. Grab some dummy text in several spell-checkable languages (English, French, German, etc.) and make a new note for each language text. Assuming that your system language is set to English, non-English words should be considered spelling errors and have squiggly lines.
3. Enable Auto-Detect Language in the Edit menu.
4. Now see the non-English notes again. Squiggly lines should be gone. (It may take a few seconds because the spelling dictionaries need to be downloaded first.)
5. Delete some characters from a word or two to make spelling errors, and see that they show squiggly lines.

Test instructions on Mac

1. See that the Auto-Detect Language option in the Edit menu is hidden. (Feature is disabled on Mac)

[dmsnell]

hey! fun fact - we don't ned to create imperative thunk-style action creators for toggle-actions like this. reducers are great at keeping things simpler and then our actions remain a basic data object representing the action our users want to take.

// state/action-types
export type ToggleLanguageDetection = Action<'TOGGLE_LANGUAGE_DETECTION'>;

// …/settings/actions.ts
import * as A from '../action-types';

export const toggleLanguageDetection: A.ActionCreator<A.ToggleLanguageDetection> = () => ( {
	type: 'TOGGLE_LANGUAGE_DETECTION',
} );

// …/settings/reducer.ts
const languageDetection: A.Reducer<boolean> = (state = false, action) => {
	switch (action.type) {
		case 'SET_LANGUAGE_DETECTION':
			return action.languageDetectionEnabled;

		case 'TOGGLE_LANGUAGE_DETECTION':
			return !state;

		default:
			return state;
	}
}

Note here that we never need to base our intentions/actions off of the current app state - all we have to do is explain what we're trying to accomplish and then the reducers govern their own data type. It gets murkier when we mash together multiple data values into a single reducer, which is where combineReducers can help (you can see how things were prior to #1216 - sometimes the boiler plate is valuable for other reasons, here in keeping separate data types isolated from each other and making it easier to reason about changes from the user's perspective).

It may also be the case that when we're working this way we don't need the SET_ action at all. This is the css with the edit/preview mode toggle, because the app starts in one mode and all that matters from the user's perspective is the intention to toggle the mode, not necessary to set it.

[mirka]

Yes, that makes sense. And I see the benefits of the combineReducers approach better now. Thanks for the tip!

[dmsnell]

@mirka it's so awesome to see you updating this PR - thanks so much!

[dmsnell]

could we instead incorporate the language into the key itself so that we don't need to track an increasing number and force a re-render?

getEditorKey = () => {
  const { spellCheckEnabled } = this.props;
  const { lang } = this.state;

  return `${ spellCheckEnabled ? 'spellcheck' : 'no_spellcheck' }-${ lang || 'unknown_lang' }`;
}

render() {
  …
    key={ this.editorKey() }
  …
}

With this I believe we should be able to get rid of the force-update cycles and allow the prop-changes to trigger the editor update naturally.

[dmsnell]

replaced in ef9fcb7

[dmsnell]

we might as well add this inside of <NoteContentEditor /> so we don't have to pass it through this component.

[dmsnell]

updated in ef9fcb7

[dmsnell]

Besides disabling language-detection (and switching back to English?) is there any way to manually choose the language?

I can imagine that if someone wants to write in one language about another language the detection could be consistently wrong but then I'd want to manually fix it.

For example, writing in German about French poetry.

[mirka]

It is technically feasible, but I think that use case probably isn't common enough to warrant the clutter of an additional menu item. However, I did initially post a UI suggestion for a global manual select (as well as some other, per-note possibilities) in the original issue.

This is mostly a UI design decision, so my thinking was to start out with the most simplest design, which is auto detection. If that works well enough for the majority of our users, great. If not, we can revisit.

[dmsnell]

I did initially post a UI suggestion for a global manual select

Yeah that's what I meant. It's a good idea.

start out with the most simplest design

Also a great idea. I wouldn't want to hold up this PR for not adding more but I wanted to ask if it was something already available we might have just been overlooking, for example, in the right-click menu of the text view if all we had to do were set some boolean toggle…

[mirka]

I wanted to ask if it was something already available we might have just been overlooking

I'm afraid not 😅 Also, I wrote "technically feasible" but I overlooked one possible complication we might run into: Getting a definitive list of available spell checker languages. I haven't looked into it too deeply, but it doesn't seem like a language list is exposed through electron-spellchecker. If Chromium doesn't expose a list, it might get complicated. (Just a heads up in case you consider it in the future.)

[dmsnell]

@mirka I have rebased this PR against the latest develop and pushed some changes from my comments. I tested it in macOS with the disable commented out but haven't yet tested it in Windows or Linux

[mirka]

@dmsnell Thanks for the review and improvements! Feel free to move forward however the team wishes 👍

[belcherj]

Closing for now. Too out of date. We can take a look when we are ready to work on this.