thewca / worldcubeassociation.org

All of the code that runs on worldcubeassociation.org
https://www.worldcubeassociation.org/
GNU General Public License v3.0
326 stars 175 forks source link

Browser integrated spell checking not working #7043

Open Nanush7 opened 2 years ago

Nanush7 commented 2 years ago

Describe the bug I don't know if this is a bug actually. The browser won't do spell checking at all when filling markdown text fields on the website.

Expected behavior When editing a delegate report, creating a competition or filling an incident log, it would be nice to have spell checking (you know, the red line below incorrectly spelled words). This feature is usually handled by the web browser, but the problem is that for some reason the browser is not doing it at all in the WCA website (it works on other sites, like GitHub).

To Reproduce Steps to reproduce the behavior:

  1. Create a new incident log.
  2. Enable your web browser's integrated spell checking.
  3. Write some words incorrectly.
  4. You won't see the red line.

Desktop:

Additional context It would be nice to have this feature working, specially for non-native English speakers.

Nanush7 commented 2 years ago

Addition: The problem occurs only in the fields that have the buttons to write markdown.

imagen

dunkOnIT commented 1 year ago

Also mentioned by Levi in #7306.

gregorbg commented 1 year ago

Looking at the Git history, it seems like the original Markdown editor was added in https://github.com/thewca/worldcubeassociation.org/pull/664. That PR intentionally disables spellchecking, but unfortunately I couldn't find any documentation as to why. My guess (!) would be that it caused conflicts because we generally expect texts to be written in English and in the local language in the same text box. But spell checking can only ever focus on one of those two languages.

I'm not opposed to simply turning spell check back on, although it may cause discomfort for non-native English communities where one of the two versions in the textbox will automatically be underlined like crazy.

For reference, it's a one-line fix at https://github.com/thewca/worldcubeassociation.org/blob/master/WcaOnRails/app/webpacker/lib/markdown-editor.js#L101. Be warned though that there might be some disussion / controversy when anyone submits a PR, for the reasons outlined above.

dunkOnIT commented 1 year ago

This raises the question of whether we want some kind of community feedback mechanism for potentially controversial changes like this.

Personally, I'd be in favour of pushing the change as even for people writing in multiple languages, I can imagine spellchecker would be helpful when writing in English.

Nanush7 commented 1 year ago

There are browsers (i.e. Firefox, Brave) that support spell-checking for multiple languages at the same time. As a non-native English speaker, I'm in favor of enabling it. Anyway, would it be possible to enable it by default and add an option or something for the user to manually disable spell-checking?

dunkOnIT commented 1 year ago

I would say we should go ahead and re-enable spell checking. If users don't want it, they can disable at the browser level. I would worry that making it a setting for the user to change on the WCA website adds unnecessary complexity.

Anyone reading this: We will accept contributions on this issue.

Nanush7 commented 1 year ago

@gregorbg @dunkOnIT

For reference, it's a one-line fix at https://github.com/thewca/worldcubeassociation.org/blob/master/WcaOnRails/app/webpacker/lib/markdown-editor.js#L101.

I just did what Gregor suggested, but this is what I got:

Screenshot

The text box does not use the integrated browser spell checking, instead it just highlights the word like that (no suggestions nor "add to dictionary" options). I guess this cannot be fixed on our side.

From this, it looks like we are using NextStepWebs/simplemde-markdown-editor, but that repo is empty.

dunkOnIT commented 1 year ago

I can't give techincal input unfortunately. Gregor is out of action until the 12th of Feb, but @FinnIckler might be able to advise?

gregorbg commented 1 year ago

We use easymde as per our package.json. It's a JS package. I don't have the capacity to dig into this, but the documentation of the package itself might help: https://github.com/Ionaru/easy-markdown-editor

Nanush7 commented 1 year ago

Solution: spellChecker: false disable library spell checker (as it has always been). nativeSpellcheck: true enable native spell check. inputStyle: 'contenteditable'. As per the documentation, this option "Defaults to textarea for desktop and contenteditable for mobile. contenteditable option is necessary to enable nativeSpellcheck" (see here).

Regarding inputStyle, I don't really know what it does at a lower level, but it seems that they want to make 'contenteditable' the only option in the future (see here). However, it looks like the contenteditable option is "terribly buggy" in older browsers (as said in this post from January, 2016).

I have briefly tested it and it seems to work properly on Firefox 110.0 and Brave 1.48.164, and kinda weird in Safari 16.3. So I guess this shouldn't be touched for now :(