Misspell wavy line#162
Conversation
…tings Wires the sdkjs CSpellCheckSettings.WavyLine flag through to the document editor's Advanced Settings panel, following the same localStorage -> InternalSettings -> CSpellCheckSettings pattern already used for the ignore-uppercase/ignore-numbers spellcheck options. Defaults to on (wavy). Signed-off-by: Peter P. Lupo <pplupo@gmail.com>
1770ffb to
6fdc8c8
Compare
|
This is also required: Euro-Office/sdkjs#59 |
Alex-Arsys
left a comment
Alex-Arsys
left a comment
There was a problem hiding this comment.
1 · locale — missing i18n key — blocking (convention)
The new label adds strSpellcheckWavyLine as a JS default, but I don't see the corresponding DE.Views.FileMenuPanels.Settings.strSpellcheckWavyLine key in apps/documenteditor/main/locale/en.json (the sibling strIgnoreWordsWithNumbers is there). As-is the checkbox label shows in English via the JS fallback but stays untranslated in every locale. Could you add the key to en.json (and flag the other locales for translation) to match the project convention?
2 · scope — DE only vs SSE/PE — question / blocking
The rendering change in sdkjs#59 lives in the common engine (Graphics.js, IsSpellCheckWavyLine defaults to true), so Spreadsheet and Presentation will render the wavy underline by default — but this PR only wires the toggle into DocumentEditor. Is DE-only intended (the source issue is DesktopEditors), or should we replicate the checkbox in SSE/PE so users there can turn it off too?
3 · cross-repo dependency — non-blocking
Note this depends on put_WavyLine from sdkjs#59; if web-apps ships against an sdkjs without it, spprops.put_WavyLine is undefined and applying spell-check settings throws. Worth merging / deploying the two together and cross-linking the PRs.
Adds the DE.Views.FileMenuPanels.Settings.strSpellcheckWavyLine locale key that was missing from DocumentEditor's en.json, and wires the same Advanced Settings checkbox into SpreadsheetEditor and Presentation Editor, following each app's existing ignore-uppercase/ignore-numbers spellcheck pattern. Defaults to on (wavy). Signed-off-by: Peter P. Lupo <pplupo@gmail.com>
SpreadsheetEditor's spellcheck only scans cell values, never shape/text-box content, so nothing in a sheet is ever marked misspelled outside that scan and DrawSpellingLine is unreachable there. The checkbox added in 33ac075 had no way to visibly do anything. Reverting until SSE's spellcheck engine actually walks shape/drawing text. PresentationEditor's equivalent checkbox is kept, since PE's SpellCheckManager does cover shape text. Signed-off-by: Peter P. Lupo <pplupo@gmail.com>
|
@Alex-Arsys thank you for pointing those out. You are absolutely right about all of them. 1- Fixed. 2- When I implemented it, I initially only thought about DE. However, you are absolutely right that it makes sense to have these in all other editors, so my fix wasn't to make it DE-only, but to implement it in the others as well. With one caveat: SSE doesn't have any sort of underline (wavy or straight). Or, at least, I didn´t know how to trigger it. I tried in a cell, in a cell comment, in a textbox, and inside a shape. There's no misspelling underline of any kind. So I just removed the settings from SSE. |
2 participants
Adding the option of the original straight line or a wavy line to underline mispelled words: Euro-Office/DesktopEditors#3