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

Upgrade to electron 8.0.1 and implement spellchecking #30

Merged
merged 1 commit into from Feb 26, 2020

Conversation

t3chguy
Copy link
Member

@t3chguy t3chguy commented Feb 24, 2020

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>
@t3chguy t3chguy marked this pull request as ready for review February 24, 2020 17:21
@t3chguy t3chguy requested a review from a team February 24, 2020 17:22
Copy link
Member

@dbkr dbkr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉 thanks! Looking forward to finally having spell checking!

Does electron support viewing the list of words you've added to the custom dictionary? Either way we should probably file a bug to add support for this as it's a little cruel to not be able to remove a mis-spelling if you accidentally add it to the dictionary.

Did you find why Riot crashed when Ryan tried to upgrade to Electron 8 a couple of releases back?

options.push({
label: word,
click: (menuItem, browserWindow) => {
browserWindow.webContents.replaceMisspelling(word);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this knows what to replace by what the user has highlighted since it's an editing command? The docs seem pretty scarce - either way it look like you're doing the correct thing.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

indeed, and empirically it does work. When you right click a misspelt word it highlights the entire word before showing the context menu

@t3chguy
Copy link
Member Author

t3chguy commented Feb 25, 2020

Does electron support viewing the list of words you've added to the custom dictionary?

Nope I don't see an API to even remove words from the custom dict at this time.

Did you find why Riot crashed when Ryan tried to upgrade to Electron 8 a couple of releases back?

is there any log of this? @jryans

@dbkr
Copy link
Member

dbkr commented Feb 25, 2020

Does electron support viewing the list of words you've added to the custom dictionary?

Nope I don't see an API to even remove words from the custom dict at this time.

OK - we should probably file a collection of bugs around this (dictionary words should probably be stored on your account data, they won't be removed when you log out). Do you want to or shall I? Related: electron/electron#22161

@t3chguy
Copy link
Member Author

t3chguy commented Feb 25, 2020

Related electron/electron#22368

@t3chguy
Copy link
Member Author

t3chguy commented Feb 25, 2020

dictionary words should probably be stored on your account data, they won't be removed when you log out

Are you suggesting we store custom words in Matrix and re-hydrate client spellcheckers when the user logs in and sync them between clients (not sure how possible this will be on the mobile apps) or that we simply wipe custom dictionary when the user logs out?

@jryans
Copy link
Contributor

jryans commented Feb 25, 2020

is there any log of this? @jryans

Unfortunately no log... 😭 Electron 8.0.0 crashed on startup for macOS at least, so I abandoned it at the time.

@dbkr
Copy link
Member

dbkr commented Feb 25, 2020

dictionary words should probably be stored on your account data, they won't be removed when you log out

Are you suggesting we store custom words in Matrix and re-hydrate client spellcheckers when the user logs in and sync them between clients (not sure how possible this will be on the mobile apps) or that we simply wipe custom dictionary when the user logs out?

Probably at least that we wipe the custom dictionary when you log out, although ideally it should probably somehow be stored on your account so you don't have to add all the words again if you log out & log in again. That does imply some level of syncing, yeah, but I don't think we need to worry about mobile apps, at least not for now.

@jryans
Copy link
Contributor

jryans commented Feb 25, 2020

Electron 8.0.0 crashed on startup for macOS at least, so I abandoned it at the time.

Electron 8.0.1 appears to work here, so maybe we're safe now!

@t3chguy t3chguy merged commit c644f66 into master Feb 26, 2020
@BloodyIron
Copy link

Woot! \o/

@geckolinux
Copy link

geckolinux commented Apr 12, 2020

@t3chguy I'm on 1.5.15 for Linux (Tumbleweed), and I have no spellchecking. I assume that c644f66 hasn't been incorporated into that release? Thanks.

@turt2live
Copy link
Member

@geckolinux this only applies to the Nightly package at the moment. 1.16 (not to be confused with 1.15.16) will have this, but not earlier.

@t3chguy
Copy link
Member Author

t3chguy commented Apr 13, 2020

1.6*

@dandv
Copy link

dandv commented May 6, 2020

Is there a way to disable spell checking?

@t3chguy
Copy link
Member Author

t3chguy commented May 6, 2020

This is not a support channel.

element-hq/element-web#13543

@t3chguy t3chguy deleted the t3chguy/spellcheck branch May 3, 2022 09:57
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.

No spell check on the electron app version
7 participants