Skip to content
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

disable spellcheck #1856

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

disable spellcheck #1856

wants to merge 1 commit into from

Conversation

Malmik
Copy link

@Malmik Malmik commented Nov 19, 2021

This is confirmed working on my local build
Would be a temporary workaround for #727

I've read code of conduct of this repo, also this and I think this pr is fine, but it's my first one so sorry for any mistakes.

This is confirmed working on my local build
Would be a temproary workaround for #727
@vercel
Copy link

vercel bot commented Nov 19, 2021

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/athens-research/athens/2aXtTxjHQoEDhELn6gP2oY8xdfTh
✅ Preview: Failed

@CLAassistant
Copy link

CLAassistant commented Nov 19, 2021

CLA assistant check
All committers have signed the CLA.

@neotyk
Copy link
Collaborator

neotyk commented Nov 19, 2021

  • This issue is not a simple as just turning spellchecker off for everyone.
  • Users without TouchBar probably don't see spellcheck suggestions.
  • On my Mac I get spellcheck suggestions on TouchBar, I'd still like to get those.
  • Therefore this should be a user preference, if they want spell check or not
    • This requires editing settings.
  • Changing spellcheck preferences will require restarting Athens to be activated.
    • This might be trickier then it sounds at first.
    • We manage application state in rendering part
    • BrowserWindow is started from main part of electron app
      • This part might not have easy access to data stored by renderer
      • If this is the case we'd have to communicate changes to spellchecker setting to main process, store it in main and read before we create BrowserWindow

@Malmik
Copy link
Author

Malmik commented Nov 21, 2021

I do understand that it isn't a fix for the issue and you are right with what you said.
I would find the decision of disabling spellcheck a compromise that will not solve current situation, but will improve it.

Now, you may say that majority of Athens users are english speaking people, and while that's true it doesn't mean they take personal notes in that language. I would be pretty sure that most of Athens users with their native language different from English do make notes in their own one, so it's apparent that spellchecking in it's current form is beneficial to few and detrimental for others, often to the such a degree that usage of Athens is impossible for them.

I understand you @neotyk would be in a dissadvantageous situation after merging this pr and I see your objections, but I hope that you will agree with me, especially that we didn't have any pr that tries to implement approach you propose, using preferences in settings, for at least 8 months ( #727 ) nor any other, even less viable solution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants