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

Built in support for spell check suggestions #11

Closed
jwheare opened this issue Jul 19, 2016 · 20 comments · Fixed by #94
Closed

Built in support for spell check suggestions #11

jwheare opened this issue Jul 19, 2016 · 20 comments · Fixed by #94
Labels
enhancement 🎁 Rewarded on Issuehunt This issue has been rewarded on Issuehunt help wanted

Comments

@jwheare
Copy link
Contributor

jwheare commented Jul 19, 2016

Issuehunt badges

Would make this a more attractive replacement for https://github.com/mixmaxhq/electron-editor-context-menu and should be simple enough to add.

e.g. https://github.com/mixmaxhq/electron-editor-context-menu/blob/d1703f7b7e4523af8c9c088d4a1efc06e2dc3b12/src/index.js#L93


IssueHunt Summary

nautatva nautatva has been rewarded.

Backers (Total: $100.00)

Submitted pull Requests


Tips


IssueHunt has been backed by the following sponsors. Become a sponsor

@sindresorhus
Copy link
Owner

sindresorhus commented Jul 20, 2016

👍 Interested in doing a pull request?

Should try to require electron-spellchecker and if it's a dependency, use it. We don't want that module as a direct dependency, as it's heavy and not everyone needs it. So we instead let people add it as a dependency if they need and it will just work.

@jwheare
Copy link
Contributor Author

jwheare commented Jul 20, 2016

Yup, currently working on splicing it in with prepend. Will see about a more integrated PR once I've got that working. There's a bit of inter process fiddling required I think.

@jwheare
Copy link
Contributor Author

jwheare commented Jul 20, 2016

I fetched up ripping out what I needed and removing this dependency in the end.

https://github.com/irccloud/irccloud-desktop/blob/54174cc142db1bf236f0196e55acaaf986f976b0/app/context_menu.js

The spelling suggestions integration is done with an ipcRenderer.send from the renderer process (where the spellcheck provider has to be registered)
https://github.com/irccloud/irccloud-desktop/blob/54174cc142db1bf236f0196e55acaaf986f976b0/app/preload.js#L17


A few other things to note that I added, in case you fancy extending this package in future with any of them, or if others are interested (not all possible with the current append/prepend hooks)

@sindresorhus
Copy link
Owner

sindresorhus commented Jul 20, 2016

Cool. Thanks for the ideas. Some of that does make sense.

To make this module more easily extendable I'm thinking of a system where you can mix custom items with the provided ones:

require('electron-context-menu')({
    menu: x => [
        x.COPY,
        {
            title: 'foo'
        },
        x.INSPECT
    ]
});

Where x.COPY is a Symbol.

Continued in #14

Regardless of you using it, what do you think about it?

@jwheare
Copy link
Contributor Author

jwheare commented Jul 20, 2016

Yeah that would be handy. If I could reuse your cut/copy/paste symbols and build the whole menu myself, while still using your separator filtering that'd be better. Just having more exports for the different stages would be a good start.

@djalmaaraujo
Copy link

There's no easy way to integrate https://github.com/electron-userland/electron-spellchecker with this module.

@sindresorhus
Copy link
Owner

sindresorhus commented Jul 21, 2017

@djalmaaraujo It is possible, it's just that electron-spellchecker is not very well documented. You could use: https://github.com/electron-userland/electron-spellchecker/blob/d457b6722b0748a6c2aa23f9a0fdac7d6d296fe0/src/spell-check-handler.js#L471

Relevant issue: electron-userland/electron-spellchecker#80

@djalmaaraujo
Copy link

@sindresorhus The docs are very bad indeed. This method you mentioned would check if a word is misspelled, but return the suggestions to this the context-menu, is a different thing. It's not related to this issue anyway. tks

@djalmaaraujo
Copy link

@sindresorhus I figured out by using one context menu for all the things (this module) and the other just for spellcheck:

In the electron-context-menu config, you can use:

shouldShowMenu: (event, params) => !params.misspelledWord,

And on the electron-spellchecker you can use:

let contextMenuListener = new ContextMenuListener((info) => {
  if (!info.misspelledWord) return;

  contextMenuBuilder.showPopupMenu(info);
});

Doing this one can exclude the other in the correct context.

@jwheare

@djalmaaraujo
Copy link

@sindresorhus With my answer above, I think you can call "built-in support" something that is true. :). I would close this issue.

@sindresorhus
Copy link
Owner

@djalmaaraujo That's not a proper solution though. I plan on having spell-checking integrated into this context menu.

@snene
Copy link

snene commented Oct 26, 2017

@djalmaaraujo Were you actually able to use both electron-context-menu and electron-spellchecker together and display the correct context menu using the solution you mentioned? I tried that and I am not able to.

When I right click on the misspelled word then first I get the application context menu and then I have to click again to get the spell checker context menu. Any help or pointers?

@djalmaaraujo
Copy link

djalmaaraujo commented Oct 27, 2017

@snene I used both, if I click on the text, I get the suggestions and IGNORE the next context-menu, so I don't end up having 2.

shouldShowMenu: (event, params) => !params.misspelledWord,

This will prevent electron-context-menu from opening the context-menu. Try to read again my previous comment and I think you will get the idea.

@snene
Copy link

snene commented Oct 27, 2017

Thanks @djalmaaraujo ! It worked for me too. It's a great workaround. I just needed to get the latest version of electron-context-menu. Thanks again!

@IssueHuntBot
Copy link

@IssueHunt has funded $100.00 to this issue.


@issuehunt-oss issuehunt-oss bot added the 💵 Funded on Issuehunt This issue has been funded on Issuehunt label May 10, 2019
@janat08
Copy link

janat08 commented Jun 13, 2019

So the original plan stands, detect if spell checker is loaded, and then show one or other?

@sindresorhus
Copy link
Owner

sindresorhus commented Feb 5, 2020

This should be easier to implement in Electron 8. There's now a spellcheck option in BrowserWindow (which indicates misspelled words with a red squiggly line) and https://github.com/electron/electron/pull/20897/files#diff-9b078be8f43e00e3b99edb5be31fa353R573 to get the suggested replacements.

Here's how it should look:

Screenshot 2020-02-05 at 15 43 48

See: https://www.electronjs.org/blog/electron-8-0#highlight-features

@djalmaaraujo
Copy link

@buzinas @philfreo ☝️

@SterlingChin
Copy link

Watching this issue

@issuehunt-oss
Copy link

issuehunt-oss bot commented Apr 14, 2020

@sindresorhus has rewarded $90.00 to @nautatva. See it on IssueHunt

  • 💰 Total deposit: $100.00
  • 🎉 Repository reward(0%): $0.00
  • 🔧 Service fee(10%): $10.00

@issuehunt-oss issuehunt-oss bot added 🎁 Rewarded on Issuehunt This issue has been rewarded on Issuehunt and removed 💵 Funded on Issuehunt This issue has been funded on Issuehunt labels Apr 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement 🎁 Rewarded on Issuehunt This issue has been rewarded on Issuehunt help wanted
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants