Spelling error decorations and suggestions (Resolves #388) #2324
Conversation
|
@miguelcmedeiros @brian-superlist - Do you guys wanna try out this PR and report back with any issues? |
| return viewModel; | ||
| } | ||
| if (viewModel is! TextComponentViewModel) { | ||
| // TODO: log error about component type |
There was a problem hiding this comment.
Should this log be added?
| } | ||
| } | ||
|
|
||
| class TextError { |
There was a problem hiding this comment.
How about a dart doc for this class?
There was a problem hiding this comment.
The new golden isn't showing an underline. This seem to be the case for other goldens as well.
There was a problem hiding this comment.
Do you intend to commit these .run files?
|
|
||
| final _styler = SpellingAndGrammarStyler(); | ||
|
|
||
| final _selectedWordLink = LeaderLink(); |
There was a problem hiding this comment.
Maybe we could add a dart doc for this.
| setState(() { | ||
| // If the selection was sitting in an ignored spelling error, and | ||
| // now the selection is somewhere else, reset the ignored error. | ||
| if (_ignoredSpellingErrorRange != null) { |
There was a problem hiding this comment.
How about returning early if _ignoredSpellingErrorRange is null?
| WidgetsBinding.instance.addPostFrameCallback((_) { | ||
| if (_suggestionToolbarOverlayController.isShowing) { | ||
| _suggestionToolbarOverlayController.hide(); | ||
| } | ||
| }); |
There was a problem hiding this comment.
Can you add a comment explaining why we hide the overlay here?
| return const SizedBox(); | ||
| } | ||
|
|
||
| return OverlayPortal( |
There was a problem hiding this comment.
Can you add a comment explaining why we need an OverlayPortal, since the widget returned here is already displayed over the content? I'm assuming it's because of the Follower, but it would be good to explain that here.
| /// Clears all spelling suggestions for text within the node with the given [nodeId]. | ||
| void clearNode(String nodeId) { | ||
| _suggestions.remove(nodeId); | ||
| } | ||
|
|
||
| /// Clears all spelling suggestions for all text in the document. | ||
| void clear() { | ||
| _suggestions.clear(); | ||
| } |
There was a problem hiding this comment.
Since we are notifying listeners when adding suggestions, shouldn't we also notify when clearing?
|
|
||
| class SpellingErrorSuggestion { | ||
| const SpellingErrorSuggestion( | ||
| {required this.word, required this.nodeId, required this.range, required this.suggestions}); |
There was a problem hiding this comment.
How about end with , to re-format?
There was a problem hiding this comment.
@matthew-carroll noticed a small issue (see comment) with the code.
Also tested the feature. Here's my initial feedback:
- We should be able to pass a
tapRegionGroupIdto the suggestion toolbar so that it plays well with aTapRegionancestor - Tested on android for potential regression and the following is thrown:
I/flutter ( 5003): (12.427) PlatformException(channel-error, Unable to establish connection on channel: "dev.flutter.pigeon.super_editor_spellcheck.SpellCheckMac.checkSpelling"., null, null)
I/flutter ( 5003): #0 SpellCheckMac.checkSpelling (package:super_editor_spellcheck/src/platform/messages.g.dart:277:7)
I/flutter ( 5003): <asynchronous suspension>
I/flutter ( 5003): #1 SuperEditorSpellCheckerMacPlugin.checkSpelling (package:super_editor_spellcheck/src/platform/spell_checker_mac.dart:37:20)
I/flutter ( 5003): <asynchronous suspension>
I/flutter ( 5003): #2 SpellingAndGrammarReaction._findSpellingAndGrammarErrors (package:super_editor_spellcheck/src/super_editor/spelling_and_grammar_plugin.dart:200:21)
I/flutter ( 5003): <asynchronous suspension>
| }) : super(nodeId: nodeId, maxWidth: maxWidth, padding: padding) { | ||
| this.composingRegion = composingRegion; | ||
| this.showComposingRegionUnderline = showComposingRegionUnderline; | ||
|
|
||
| this.spellingErrorUnderlineStyle = spellingErrorUnderlineStyle; | ||
| this.spellingErrors = spellingErrors; | ||
|
|
||
| this.grammarErrorUnderlineStyle = grammarErrorUnderlineStyle; | ||
| this.grammarErrors = spellingErrors; |
There was a problem hiding this comment.
| this.grammarErrors = spellingErrors; | |
| this.grammarErrors = grammarErrors; |
| parentNode: widget.editorFocusNode, | ||
| child: Container( | ||
| decoration: BoxDecoration( | ||
| color: Colors.white, |
There was a problem hiding this comment.
The current implementation looks fine on light mode, but not on dark mode.
Also, it would be great to be able to customize the suggestion overlay. Is it possible to expose this in the API?
Spelling error decorations and suggestions (Resolves #388)
Screen.Recording.2024-09-17.at.7.09.40.PM.mov