Skip to content

Conversation

@zaynetro
Copy link
Contributor

@zaynetro zaynetro commented Jan 25, 2023

Copied the changes from similar changes in simplistic editor. Link points to a specific file. It might take a while to download the diffs though.

Tried example app on Android, Mac and iOS.

@amantoux
Copy link
Member

@zaynetro
Copy link
Contributor Author

'ToolbarOptions' is deprecated and shouldn't be used. Use contextMenuBuilder instead. This feature was deprecated after v3.3.0-0.5.pre

I will check if I can refactor it quickly.

@zaynetro
Copy link
Contributor Author

Seems like TextSelectionOverlay should be used instead of custom OverlayEntry? _toolbar in EditorTextSelectionOverlay. It means though quite a few changes :/

Let me know how we can proceed. Luckily, deprecated ToolbarOptions still works.

@zaynetro
Copy link
Contributor Author

Arrow keys and backspace buttons do not work on Mac when running example app on Mac...

@zaynetro
Copy link
Contributor Author

Looks like arrows and backspace are reported in performSelector now.

flutter: performSelector deleteBackward:
flutter: performSelector moveLeft:
flutter: performSelector moveRight:

@zaynetro
Copy link
Contributor Author

OK. Found what Flutter does internally, copied it and things started to work again.

@Amir-P
Copy link
Member

Amir-P commented Jan 28, 2023

Thanks for your PR. I'll try to review it in the next few days. @zaynetro

Copy link
Member

@Amir-P Amir-P left a comment

Choose a reason for hiding this comment

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

LGTM. Just fix the 4 unnecessary_non_null_assertion warnings. Also let's keep ToolbarOptions for now. I think it's better to address it in another PR. @zaynetro
Do you have anything to add @amantoux ?

@zaynetro
Copy link
Contributor Author

Removed.

@amantoux
Copy link
Member

Would you mind ignoring deprecation messages so as to have a green CI?
Otherwise, all good on my end

@codecov
Copy link

codecov bot commented Jan 30, 2023

Codecov Report

Merging #89 (47ae8fd) into master (68ae54f) will decrease coverage by 0.07%.
The diff coverage is 35.71%.

@@            Coverage Diff             @@
##           master      #89      +/-   ##
==========================================
- Coverage   75.45%   75.38%   -0.07%     
==========================================
  Files          57       57              
  Lines        8644     8650       +6     
==========================================
- Hits         6522     6521       -1     
- Misses       2122     2129       +7     
Impacted Files Coverage Δ
...kages/fleather/lib/src/widgets/editor_toolbar.dart 0.29% <0.00%> (ø)
packages/fleather/lib/src/widgets/editor.dart 48.08% <11.11%> (-0.47%) ⬇️
...kages/fleather/lib/src/widgets/text_selection.dart 68.48% <100.00%> (+0.09%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@zaynetro
Copy link
Contributor Author

Ignored.

@amantoux
Copy link
Member

Code coverage diff acceptable:

  • macOS specific code not covered in CI
  • No testing of didChangeInputControl is ok

@amantoux amantoux merged commit 271167a into fleather-editor:master Jan 30, 2023
@zaynetro zaynetro deleted the flutter-v3.7 branch January 31, 2023 10:02
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.

3 participants