Move platform specific text selection behavior out of styled TextField classes #155774
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
089b8df to
7fee2d9
Compare
Renzo-Olivares
commented
Sep 27, 2024
| renderEditable.selectWordEdge(cause: SelectionChangedCause.tap); | ||
| if (previousSelection == editableText.textEditingValue.selection | ||
| && renderEditable.hasFocus | ||
| && !editableText.widget.readOnly) { |
There was a problem hiding this comment.
should I use renderEditable.readOnly instead?
There was a problem hiding this comment.
No strong opinion but I lean renderEditable.readOnly I guess? I'm thinking it's best to use the most narrowly scoped thing possible. Maybe in the future this method could only use renderEditable and not editableText, for example.
d3120ce to
b774441
Compare
justinmc
approved these changes
Sep 27, 2024
There was a problem hiding this comment.
Looks like a solid refactor, thanks for cleaning this up. Is there anything in Cupertino that can be similarly removed? I think originally iOS-specific behavior was put into Cupertino in places like _CupertinoTextFieldSelectionGestureDetectorBuilder, but really that platform-specific behavior should be in the widgets library.
| renderEditable.selectWordEdge(cause: SelectionChangedCause.tap); | ||
| if (previousSelection == editableText.textEditingValue.selection | ||
| && renderEditable.hasFocus | ||
| && !editableText.widget.readOnly) { |
There was a problem hiding this comment.
No strong opinion but I lean renderEditable.readOnly I guess? I'm thinking it's best to use the most narrowly scoped thing possible. Maybe in the future this method could only use renderEditable and not editableText, for example.
Comment on lines
+2482
to
+2483
| } | ||
| } |
There was a problem hiding this comment.
It's hard to tell from the diff but I think this indentation is wrong?
There was a problem hiding this comment.
I'll double check!
There was a problem hiding this comment.
I think this is right, the nested switch makes it look a little off.
|
@justinmc I didn't really see anything we could pull out of It mainly just has this, flutter/packages/flutter/lib/src/cupertino/text_field.dart Lines 111 to 121 in 3e4a57f which seems CupertinoTextField specific since it handles the clear button behavior, so I think that's okay to stay there.
There's also but i'm not quite sure what it's purpose is there for, and the git blame doesn't tell me much either. Removing it doesn't break any tests, do you have any thoughts? |
added 7 commits
September 27, 2024 16:51
13fe420 to
74a30e2
Compare
|
Talked offline about #155774 (comment) , will investigate at a later time. |
thejitenpatel
pushed a commit
to thejitenpatel/flutter
that referenced
this pull request
Oct 1, 2024
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Oct 1, 2024
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Oct 1, 2024
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Oct 1, 2024
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Oct 1, 2024
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Oct 1, 2024
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Oct 2, 2024
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Oct 2, 2024
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Oct 2, 2024
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Oct 2, 2024
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Oct 2, 2024
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Oct 2, 2024
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Oct 3, 2024
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Oct 3, 2024
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Oct 3, 2024
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Oct 3, 2024
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Oct 3, 2024
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Oct 3, 2024
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Oct 4, 2024
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Oct 4, 2024
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Oct 4, 2024
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Oct 5, 2024
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Oct 5, 2024
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Oct 6, 2024
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Oct 6, 2024
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Oct 7, 2024
auto-submit bot
pushed a commit
to flutter/packages
that referenced
this pull request
Oct 8, 2024
Manual roll Flutter from ead6b0d to 6bba08c (37 revisions) Manual roll requested by stuartmorgan@google.com flutter/flutter@ead6b0d...6bba08c 2024-10-01 a.a.ustinoff@gmail.com Feat: Add opportunity to change CupertinoTextField suffix alignment (flutter/flutter#154601) 2024-10-01 engine-flutter-autoroll@skia.org Roll Flutter Engine from da28db8ff41d to 3fdb546bf595 (2 revisions) (flutter/flutter#155993) 2024-10-01 engine-flutter-autoroll@skia.org Roll Flutter Engine from 6b21b796cc94 to da28db8ff41d (1 revision) (flutter/flutter#155985) 2024-10-01 engine-flutter-autoroll@skia.org Roll Flutter Engine from 259f56c6e91b to 6b21b796cc94 (1 revision) (flutter/flutter#155983) 2024-10-01 engine-flutter-autoroll@skia.org Roll Flutter Engine from 6ee04ed763d9 to 259f56c6e91b (1 revision) (flutter/flutter#155981) 2024-10-01 engine-flutter-autoroll@skia.org Roll Flutter Engine from bfb6dddb2b30 to 6ee04ed763d9 (3 revisions) (flutter/flutter#155978) 2024-10-01 engine-flutter-autoroll@skia.org Roll Flutter Engine from e61bc853acb2 to bfb6dddb2b30 (8 revisions) (flutter/flutter#155967) 2024-10-01 katelovett@google.com Disable flaky menu test (flutter/flutter#155968) 2024-10-01 robert.ancell@canonical.com Fix crash in Linux platform channel example. (flutter/flutter#155735) 2024-09-30 polinach@google.com Fix leak in input_decorator [prod-leak-fix] (flutter/flutter#155885) 2024-09-30 rmolivares@renzo-olivares.dev Move platform specific text selection behavior out of styled TextField classes (flutter/flutter#155774) 2024-09-30 engine-flutter-autoroll@skia.org Roll Flutter Engine from b466a0dd7834 to e61bc853acb2 (5 revisions) (flutter/flutter#155952) 2024-09-30 rmolivares@renzo-olivares.dev `RenderParagraph`s `_SelectableFragment.boundingBoxes` should consider max line height (flutter/flutter#155892) 2024-09-30 engine-flutter-autoroll@skia.org Roll Packages from 0321757 to 27c9853 (3 revisions) (flutter/flutter#155945) 2024-09-30 engine-flutter-autoroll@skia.org Roll Flutter Engine from f4507e7a4beb to b466a0dd7834 (1 revision) (flutter/flutter#155944) 2024-09-30 andrewrkolos@gmail.com when `ResidentRunner.tryInitLogReader` fails, only log warning on Android (flutter/flutter#155800) 2024-09-30 gordin.dan@gmail.com Move FlutterLogo from material to widget (flutter/flutter#155864) 2024-09-30 lizhuo.huang@outlook.com fix: support android 15 16k page size for template plugin_ffi (flutter/flutter#155508) 2024-09-30 engine-flutter-autoroll@skia.org Roll Flutter Engine from daf126b38b8f to f4507e7a4beb (1 revision) (flutter/flutter#155932) 2024-09-30 engine-flutter-autoroll@skia.org Roll Flutter Engine from 734205fbcd62 to daf126b38b8f (1 revision) (flutter/flutter#155929) 2024-09-30 engine-flutter-autoroll@skia.org Roll Flutter Engine from 338f09c4ea72 to 734205fbcd62 (1 revision) (flutter/flutter#155923) 2024-09-30 engine-flutter-autoroll@skia.org Roll Flutter Engine from 897f5caffe2d to 338f09c4ea72 (2 revisions) (flutter/flutter#155917) 2024-09-30 engine-flutter-autoroll@skia.org Roll Flutter Engine from 569abc4044b8 to 897f5caffe2d (1 revision) (flutter/flutter#155912) 2024-09-29 engine-flutter-autoroll@skia.org Roll Flutter Engine from c4784aa7eade to 569abc4044b8 (2 revisions) (flutter/flutter#155894) 2024-09-28 engine-flutter-autoroll@skia.org Roll Flutter Engine from ff4541712df4 to c4784aa7eade (2 revisions) (flutter/flutter#155889) 2024-09-28 50643541+Mairramer@users.noreply.github.com Fixes column text width calculation in CupertinoDatePicker (flutter/flutter#151128) 2024-09-28 engine-flutter-autoroll@skia.org Roll Flutter Engine from 380fd814448c to ff4541712df4 (1 revision) (flutter/flutter#155886) 2024-09-28 tessertaha@gmail.com Optimize `Overlay` sample to avoid overflow (flutter/flutter#155861) 2024-09-28 engine-flutter-autoroll@skia.org Roll Flutter Engine from f3b11bcd9c37 to 380fd814448c (1 revision) (flutter/flutter#155876) 2024-09-28 engine-flutter-autoroll@skia.org Roll Flutter Engine from 9c8e5cb226e4 to f3b11bcd9c37 (3 revisions) (flutter/flutter#155865) 2024-09-28 engine-flutter-autoroll@skia.org Roll Flutter Engine from f9e4ed28f103 to 9c8e5cb226e4 (1 revision) (flutter/flutter#155857) 2024-09-27 engine-flutter-autoroll@skia.org Roll Flutter Engine from f21f2b232b8a to f9e4ed28f103 (2 revisions) (flutter/flutter#155855) 2024-09-27 45459898+RamonFarizel@users.noreply.github.com Add magnificationScale to CupertinoMagnifier for Zoom Effect (flutter/flutter#155276) 2024-09-27 iNoles@users.noreply.github.com Fix typo on theme_data (flutter/flutter#155644) 2024-09-27 engine-flutter-autoroll@skia.org Roll Flutter Engine from 7c603de2dca7 to f21f2b232b8a (6 revisions) (flutter/flutter#155843) 2024-09-27 34871572+gmackall@users.noreply.github.com Turn the packages roller bot back on (flutter/flutter#155842) 2024-09-27 engine-flutter-autoroll@skia.org Roll Packages from f38b780 to 0321757 (4 revisions) (flutter/flutter#155832) 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 camillesimon@google.com,stuartmorgan@google.com on the revert to ensure that a human is aware of the problem. ...
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Dec 11, 2024
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Dec 12, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #129726
Pre-launch Checklist
///).