fix Ink not updating on TextField newline #140700
Conversation
github-actions
bot
added
a: text input
|
Golden file changes have been found for this pull request. Click here to view and triage (e.g. because this is an intentional change). If you are still iterating on this change and are not ready to resolve the images on the Flutter Gold dashboard, consider marking this PR as a draft pull request above. You will still be able to view image results on the dashboard, commenting will be silenced, and the check will not try to resolve itself until marked ready for review. For more guidance, visit Writing a golden file test for Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
|
Golden file changes are available for triage from new commit, Click here to view. For more guidance, visit Writing a golden file test for Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
justinmc
left a comment
justinmc
left a comment
There was a problem hiding this comment.
Thanks for the PR! This is an interesting bug. I want to make sure SizeChangedLayoutNotifier is the right approach.
@goderbauer Is it an oversight that we've never added SizeChangedLayoutNotifier to EditableText before? It seems that Ink doesn't update its position when the size of an EditableText changes:
 |
TextField(minLines: 1, maxLines: 4),
ListTile(
title: Text('moving text'),
shape: RoundedRectangleBorder(
side: BorderSide(
color: Colors.red
)
),
),
|
Or is there something more fundamental to solve here?
github-actions
bot
removed
the
f: material design
I am not entirely sure. My guess is that this was just an oversight and nobody thought about this before. |
Well then I say let's do it. It looks like the tests are passing. We can keep a close eye out in case this causes a performance regression or anything like that. |
justinmc
left a comment
justinmc
left a comment
There was a problem hiding this comment.
LGTM 👍
Just a nit in the test. Let's get one more reviewer and then we can merge.
bleroux
left a comment
bleroux
left a comment
There was a problem hiding this comment.
LGTM!
Thanks for your contribution.
Before we merge this thank you to address the last 2 nits.
Manual roll requested by [email protected] flutter/flutter@a8efa77...2f6fdf2 2024-01-26 [email protected] Start renaming by adding a new `bringup: true` as an Android emulator. (flutter/flutter#142257) 2024-01-26 [email protected] Instrument ImageInfo. (flutter/flutter#141411) 2024-01-26 [email protected] Fix `SegmentedButton` default size and default tappable size (flutter/flutter#142243) 2024-01-26 [email protected] Update name for android_defines_test. (flutter/flutter#142273) 2024-01-26 [email protected] Enable native compilation for windows-arm64 (flutter/flutter#141930) 2024-01-25 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Roll Flutter Engine from 4b145d041560 to 44f26274bfbe (6 revisions)" (flutter/flutter#142274) 2024-01-25 [email protected] Run a few mac tests only on arm. (flutter/flutter#142188) 2024-01-25 [email protected] fix Ink not updating on TextField newline (flutter/flutter#140700) 2024-01-25 49699333+dependabot[bot]@users.noreply.github.com Bump codecov/codecov-action from 3.1.4 to 3.1.5 (flutter/flutter#142259) 2024-01-25 [email protected] Roll Flutter Engine from 4b145d041560 to 44f26274bfbe (6 revisions) (flutter/flutter#142264) 2024-01-25 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Reverts "Rename `integration_tests/external_ui` but do not touch anything else..."" (flutter/flutter#142268) 2024-01-25 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Rename `integration_tests/external_ui` but do not touch anything else..." (flutter/flutter#142265) 2024-01-25 [email protected] Roll Flutter Engine from b2167a93c1a0 to 4b145d041560 (3 revisions) (flutter/flutter#142256) 2024-01-25 [email protected] Implementing `switch` expressions in the `cupertino/` directory (flutter/flutter#141591) 2024-01-25 [email protected] Rename `integration_tests/external_ui` but do not touch anything else... (flutter/flutter#142238) 2024-01-25 [email protected] Roll Flutter Engine from 55eefd5bd255 to b2167a93c1a0 (2 revisions) (flutter/flutter#142252) 2024-01-25 [email protected] Roll Flutter Engine from 3b4779324b44 to 55eefd5bd255 (6 revisions) (flutter/flutter#142245) 2024-01-25 [email protected] Fix incorrect zh-cn translation for Look Up Label in selection controls (flutter/flutter#142158) 2024-01-25 [email protected] PopScope example improvements (flutter/flutter#142163) 2024-01-25 [email protected] Roll Flutter Engine from 1d3f16b0d62e to 3b4779324b44 (1 revision) (flutter/flutter#142225) 2024-01-25 [email protected] Roll Packages from 8fbdf65 to 21b5abb (6 revisions) (flutter/flutter#142224) 2024-01-25 [email protected] Don't show legacy welcome message when analytics are disabled (flutter/flutter#140956) 2024-01-25 [email protected] Roll Flutter Engine from 7c4ed15cb271 to 1d3f16b0d62e (1 revision) (flutter/flutter#142223) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
4 participants
Summary
As documented in the Ink class, a layout LayoutChangedNotification needs to be dispatched.
When a
FormFieldreceives inputs it may change its size which causes box decorations (which are based on InkWells) on other elements not to update. To address this issue the text form field now dispatches the notification when it needs to be rebuild which must happen on a size change.This change is tested by adding an Ink below a text field to which a new line is added.
Fixes #138219
Old behavior:
New behavior:
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.