chore: move members of magnifier feature into a separate files when possible (not final yet) #2251
Conversation
…ossible (not final yet)
| @@ -1114,8 +1116,7 @@ class QuillRawEditorState extends EditorState | |||
| ? null | |||
| : (context) => | |||
| widget.configurations.contextMenuBuilder!(context, this), | |||
| magnifierConfiguration: widget.configurations.magnifierConfiguration ?? | |||
| TextMagnifier.adaptiveMagnifierConfiguration, | |||
| magnifierConfiguration: _magnifierConfiguration, | |||
There was a problem hiding this comment.
The expression:
widget.configurations.magnifierConfiguration ??
TextMagnifier.adaptiveMagnifierConfigurationhas been extracted into _magnifierConfiguration (quill_raw_editor_state_magnifier_ext.dart).
The name is not quite descriptive and needs to be changed or at least documented.
| if (_selectionOverlay == null) return; | ||
| _selectionOverlay?.hideMagnifier(); | ||
| } | ||
| void hideMagnifier() => _hideMagnifier(); |
There was a problem hiding this comment.
The _hideMagnifier is in the new file quill_raw_editor_state_magnifier_ext.dart. I would generally introduce this in a different way but since I'm not sure why it's done this way, I had to extract the implementation of those three methods into a private method that can be accessed only from this file. The main goal is to clean up the code, in general, this change needs to be improved as well, should be documented in a clear way.
Same for showMagnifier and updateMagnifier.
| // TODO: Are those specific to the magnifier? Is this a feature specific to the native Android | ||
| // and iOS apps (outside of the browser)? If yes, need to use isAndroidApp | ||
| // and isIosApp, if no, use isAndroid and isIos for clarity | ||
|
|
There was a problem hiding this comment.
Need to be addressed.
| void showMagnifier(Offset positionToShow); | ||
|
|
||
| void updateMagnifier(Offset positionToShow); | ||
|
|
||
| void hideMagnifier(); | ||
|
|
There was a problem hiding this comment.
Why those methods added here?
There was a problem hiding this comment.
Probably bad design to be here
|
Haven't tested the changes yet. |
|
I have extracted some parts of this PR into release/v11. Closing this PR as I don't see it as a good time investment. |
Description
Cleanup #2026 by extracting the new methods, members, and checks to separate files when possible
Not for merge yet, I'm uncertain about some of the changes made, waiting response from the contributor, also the new TODOs introduced in this PR need to be cleaned up before merging.
This PR does separate anything related to the magnifier feature in one manageable place in a simple way. Still need more improvements.
Even after the cleanup, the new code is still not qualified enough as I'm unable to understand some of the changes made. There might be inconsistency issues.
Related Issues
Type of Change