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

feature: enanble custom TextSelectionControls #398

Merged
merged 5 commits into from
Aug 11, 2024

Conversation

kfdykme
Copy link
Contributor

@kfdykme kfdykme commented Aug 11, 2024

Hello, there is a bug in Flutter's OverlayEntry handling related to TextSelectionTheme.of(context). Specifically, the issue causes the color of MaterialTextSelectionControls to be lost when buildHandle is called. This can be seen in the Flutter source code at https://github.com/flutter/flutter/blob/master/packages/flutter/lib/src/material/text_selection.dart#L77.

A simple way to fix this issue would be to allow developers to use a custom TextSelectionControls implementation.

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.

Thanks for doing it. LGTM. Can you also take care of adding a test for this please?

@kfdykme
Copy link
Contributor Author

kfdykme commented Aug 11, 2024

Thanks for doing it. LGTM. Can you also take care of adding a test for this please?

I tried adding a test, please check the following

@amantoux
Copy link
Member

@kfdykme can you please format the changes using dart format .? and we should be good to go

@kfdykme
Copy link
Contributor Author

kfdykme commented Aug 11, 2024

@kfdykme can you please format the changes using dart format .? and we should be good to go

Thanks for the reminder, I have formatted it now.

@amantoux amantoux requested a review from Amir-P August 11, 2024 13:04
@amantoux
Copy link
Member

@kfdykme you still have a analysis issue
info • Use 'const' with the constructor to improve performance • test/widgets/editor_text_selection_controllers_test.dart:58:29 • prefer_const_constructors

@kfdykme
Copy link
Contributor Author

kfdykme commented Aug 11, 2024

@kfdykme you still have a analysis issue info • Use 'const' with the constructor to improve performance • test/widgets/editor_text_selection_controllers_test.dart:58:29 • prefer_const_constructors

sorry , fixed

Copy link

codecov bot commented Aug 11, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.91%. Comparing base (e7c7a9a) to head (8879fad).

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #398      +/-   ##
==========================================
+ Coverage   87.90%   87.91%   +0.01%     
==========================================
  Files          62       62              
  Lines       10329    10330       +1     
==========================================
+ Hits         9080     9082       +2     
+ Misses       1249     1248       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

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. Thanks.

@Amir-P Amir-P merged commit fff4b87 into fleather-editor:master Aug 11, 2024
3 checks passed
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