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

spellchecker: Use Electron 8 built-in spellchecker. #967

Merged
merged 5 commits into from
Jun 18, 2020

Conversation

manavmehta
Copy link
Collaborator

@manavmehta manavmehta commented May 23, 2020

What's this PR do?
#928 takes away with it the context menu as so this PR implements a custom context menu
Inherits commit from #928 as it was needed
Screenshots?

You have tested this PR on:

  • macOS Catalina 10.15.2
  • Linux/Ubuntu
  • Windows

Also fixes: #504

@akashnimare
Copy link
Member

Just had a quick run - this works well.

Copy link
Collaborator Author

@manavmehta manavmehta left a comment

Choose a reason for hiding this comment

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

2 bugs to keep updated

app/renderer/js/components/context-menu.ts Outdated Show resolved Hide resolved
app/renderer/js/components/context-menu.ts Outdated Show resolved Hide resolved
Copy link
Collaborator Author

@manavmehta manavmehta left a comment

Choose a reason for hiding this comment

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

The code would've been much shorter if the package ISO-639-1 had corresponding names for some of the locale codes from electron.

app/renderer/js/pages/preference/general-section.ts Outdated Show resolved Hide resolved
app/renderer/js/pages/preference/general-section.ts Outdated Show resolved Hide resolved
@manavmehta manavmehta force-pushed the contextmenu branch 2 times, most recently from c1b20e4 to f342fa0 Compare June 6, 2020 15:09
@abhigyank
Copy link
Collaborator

Functionality is working for me. Almost mergable I suppose.

@manavmehta manavmehta changed the title [WIP] contextMenu: Create custom context menu spellchecker: Create custom context menu and input tags to pass language to spellchecker Jun 6, 2020
@abhigyank abhigyank requested a review from andersk June 6, 2020 16:04
app/renderer/js/pages/preference/general-section.ts Outdated Show resolved Hide resolved
app/renderer/css/tagify.css Outdated Show resolved Hide resolved
app/renderer/js/components/context-menu.ts Outdated Show resolved Hide resolved
app/renderer/js/pages/preference/general-section.ts Outdated Show resolved Hide resolved
app/renderer/js/pages/preference/general-section.ts Outdated Show resolved Hide resolved
app/renderer/js/pages/preference/general-section.ts Outdated Show resolved Hide resolved
app/renderer/js/pages/preference/general-section.ts Outdated Show resolved Hide resolved
typings.d.ts Outdated Show resolved Hide resolved
@manavmehta manavmehta force-pushed the contextmenu branch 2 times, most recently from fee38c2 to 8e15134 Compare June 7, 2020 16:15
app/renderer/preference.html Outdated Show resolved Hide resolved
app/renderer/js/pages/preference/general-section.ts Outdated Show resolved Hide resolved
app/renderer/js/pages/preference/general-section.ts Outdated Show resolved Hide resolved
app/renderer/js/pages/preference/general-section.ts Outdated Show resolved Hide resolved
andersk and others added 4 commits June 17, 2020 12:19
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
* It performs the appropriate operations
* Provides suggestions for misspellings caught by electron's built-in spellchecker
* Will be maintained by the organisation
* Tag-input in Setings -> General -> Functionality to select max 3 languages (stored on disk)
* Pass the languages to spellchecker, after fetching from disk

Fixes: zulip#504
* Better type annotations
* Use DOM manipulation instead of HTML generation
* Internationalize strings
* Use .map and .filter to avoid quadratic loop
* source tagify.css from node_modules
* declare @yaireo/tagify
* Proper internationalization
* Passing correct parameters including contextMenuParams
* Typecasting event: Event
@manavmehta manavmehta force-pushed the contextmenu branch 3 times, most recently from 462db7a to 9a279e9 Compare June 17, 2020 20:09
* Bug fix: broken customCSS
* Improvement: set default spellcheck lang
* Improvement: ensure spellchecker is set for every session
* Bug fix: json.parse() cannot parse empty array string when all the langs are deselected
@akashnimare akashnimare changed the title spellchecker: Create custom context menu and input tags to pass language to spellchecker spellchecker: Use Electron 8 built-in spellchecker. Jun 18, 2020
@akashnimare akashnimare merged commit 0fff633 into zulip:master Jun 18, 2020
@akashnimare
Copy link
Member

Merged. Thanks, everyone for working on this 🚀

@akashnimare
Copy link
Member

This works well. There are a couple of minor things which we'll be adding as a follow-up.

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

Successfully merging this pull request may close these issues.

Please add usage of default context menu on MacOS
5 participants