[RenderEditable] Dont paint caret when selection is invalid#79607
Conversation
flutter-dashboard
bot
added
the
framework
justinmc
left a comment
justinmc
left a comment
There was a problem hiding this comment.
LGTM 👍
Maybe run the Google tests on this before merge? If you think there's any risk of a cursor no longer being painted in a visual test.
| expect(editable, paints..paragraph()); | ||
| // No longer paints the caret. | ||
| expect(editable, isNot(paints..rect(color: Colors.red[500])), | ||
| ); |
There was a problem hiding this comment.
Nit: This parenthesis should be on the previous line I think.
|
Not going to merge this just yet. I'll wait for #79656 to be rolled and do a TGP. |
|
The behavior after this patch is consistent with the native behavior(desktop and mobile), right? |
|
@xu-baolin I have not tested on native platforms but not painting the caret seems to be the only approach that makes sense. I'll run all customer tests to figure out whether this is a breaking change. This PR depends on a breaking change #79656 so I'll have to wait for that commit to get rolled first. |
|
It does break internal tests: internal link. I'm creating a migration guide. |
…lutter#79607)" (flutter#81076) This reverts commit 0f8148e.
…81076 (#81155) * Revert "Replace some `dynamic` to `Object?` type (#80772)" (#80965) This reverts commit 12a2e68. * Add frontend_server_client to dependency allowlist (#80912) * Revert "[RenderEditable] Dont paint caret when selection is invalid (#79607)" (#81076) This reverts commit 0f8148e. * Convert AnimatedSize to a StatefulWidget (#80554) Co-authored-by: Jenn Magder <[email protected]> Co-authored-by: Jason Simmons <[email protected]>
Fixes #79495 This is basically a reland of #79607. Currently when the cursor is invalid, arrow key navigation / typing / backspacing doesn't work since the cursor position is unknown. Showing the cursor when the selection is invalid gives the user the wrong information about the current insert point in the text. This is going to break internal golden tests.
4 participants
Fixes #79495
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.