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

textSelectionHandleColor taken from parent's context. Fixes #20219. #22026

Closed
wants to merge 1 commit into from
Closed

textSelectionHandleColor taken from parent's context. Fixes #20219. #22026

wants to merge 1 commit into from

Conversation

ajinasokan
Copy link

textSelectionHandleColor taken from the parent widget context instead of app's overlay context. Fixes #20219.

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

@ajinasokan
Copy link
Author

I signed it!

@googlebot
Copy link

CLAs look good, thanks!

@ajinasokan ajinasokan changed the title textSelectionHandleColor uses parent's context. Fixes #20219. textSelectionHandleColor taken from parent's context. Fixes #20219. Sep 19, 2018
OverlayEntry(builder: (BuildContext context) => _buildHandle(context, _TextSelectionHandlePosition.start)),
OverlayEntry(builder: (BuildContext context) => _buildHandle(context, _TextSelectionHandlePosition.end)),
OverlayEntry(builder: (_) => _buildHandle(context, _TextSelectionHandlePosition.start)),
OverlayEntry(builder: (_) => _buildHandle(context, _TextSelectionHandlePosition.end)),
Copy link
Contributor

Choose a reason for hiding this comment

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

this won't work reliably. When you look up the Theme with Theme.of, it causes the relevant Theme to register the context so that when the Theme changes, the context is marked dirty. But that won't work here because that would mark the widget whose context was passed to TextSelectionOverlay dirty, but not the handles. So the handles wouldn't change color.

(When you use the build context of the OverlayEntry, as in the original code, then when the relevant Theme changes, it causes the overlay entry to rebuild, rebuilding the handle.)

@Hixie
Copy link
Contributor

Hixie commented Oct 16, 2018

This would need a test. The most important test would be one where the color of the Theme is changed while the handles are showing; we should verify that the handles change colour. I don't believe they would, with this patch.

In general there isn't a good way to make something higher in the tree depend on a Theme elsewhere in the tree, as the PR is attempting to do here.

@zoechi zoechi added a: text input Entering text in a text field or keyboard related problems framework flutter/packages/flutter repository. See also f: labels. labels Nov 28, 2018
@goderbauer
Copy link
Member

Closing as there hasn't been any activity for a while.

@goderbauer goderbauer closed this Jan 7, 2019
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
a: text input Entering text in a text field or keyboard related problems framework flutter/packages/flutter repository. See also f: labels.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

textSelectionHandleColor is not working/changing.
5 participants