-
Notifications
You must be signed in to change notification settings - Fork 345
Textfield focus issue workaround #9091
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
Conversation
@@ -101,15 +102,15 @@ class _PropertiesListState extends State<_PropertiesList> { | |||
@override | |||
void initState() { | |||
super.initState(); | |||
// Workaround for https://github.com/flutter/devtools/issues/8929. | |||
// Workaround for https://github.com/flutter/flutter/issues/155265. | |||
setUpTextFieldFocusFixHandler(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we still need this since we now have it at the root of DevToolsApp?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, removed!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have we done thorough testing to make sure that we are not causing any unforeseen side effects on other parts of the DevTools app? (different screens, embedded and non-embedded, vs code & intelliJ, devtools extensions, etc.)
I've done the following testing, will need to finish testing tomorrow morning:
|
Adds the workaround for flutter/flutter#155265 to ALL of DevTools, not just the Property Editor.
Incorporates @ditman's suggestions as well to check if we are in a
<flutter-view>
first before callingblur
(thank you!)FYI @DanTup @jwren
Also FYI @johnpryan (Dartpad has the same issue, so adding this workaround there might be a good idea until it's fixed upstream in Flutter)