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

Desktop: Fixes #11261 : Ensure spell-check toggle works on macOS #11388

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

Conversation

dhakarRaghu
Copy link

Desktop: Fixes #11261 : Ensure spell-check can be toggled off on macOS

This pull request addresses an issue where spell-check could not be fully disabled on the macOS desktop app, even when explicitly turned off in the settings.

Problem

On macOS, users encountered a problem when trying to disable spell-check. Although the setting was turned off, spell-check continued to underline words, suggesting that it wasn't fully disabled.

Solution

The fix ensures that:

  • When users toggle spell-check off, the language list is set to an empty array, and the spell-checker is explicitly disabled in the session.
  • When re-enabling, the spell-checker applies the correct languages and confirms that it is enabled in the session.

Technical Summary

  • Updated the SpellCheckerServiceDriverNative.ts file to explicitly call setSpellCheckerEnabled(false) when disabling and setSpellCheckerEnabled(true) when enabling.
  • Improved logging in setLanguages to track the session's state after each toggle and confirm that the changes have taken effect.

Testing

  • Manually tested the toggling of spell-check on and off on the macOS desktop app to confirm that spell-check does not underline words when disabled.
  • Verified that the correct language settings are applied upon re-enabling.

This PR resolves the underlying issue and provides a more consistent spell-check toggle experience on the macOS platform.

Reference: Fixes #11261

demo :

Screen.Recording.2024-11-13.at.10.33.59.PM.mov

Copy link
Contributor

github-actions bot commented Nov 13, 2024

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@dhakarRaghu
Copy link
Author

I have read the CLA Document and I hereby sign the CLA

@laurent22
Copy link
Owner

Too many white space changes, deleted comments and log statements in this pull request. We can't merge it as it is

Comment on lines 55 to 62
if (effectiveLanguages.length === 0) {
this.session().setSpellCheckerLanguages([]);
this.session().setSpellCheckerEnabled(false);
return;
}

this.session().setSpellCheckerLanguages(effectiveLanguages);
this.session().setSpellCheckerEnabled(true);
Copy link
Owner

Choose a reason for hiding this comment

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

Isn't it the same as this?

this.session().setSpellCheckerLanguages(effectiveLanguages);
this.session().setSpellCheckerEnabled(effectiveLanguages.length > 0);

Copy link
Author

Choose a reason for hiding this comment

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

yes, The changes i have done and
this.session().setSpellCheckerLanguages(effectiveLanguages); this.session().setSpellCheckerEnabled(effectiveLanguages.length > 0);
are same.
Should i change my commit to this?

Copy link
Owner

Choose a reason for hiding this comment

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

yes

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.

Spell checker cannot be disabled in RTE
2 participants