TextEditingDelta support for iOS#28418
Conversation
justinmc
left a comment
justinmc
left a comment
There was a problem hiding this comment.
Looks super straightforward, identical to the Android approach except even simpler because no pending deltas, right? I'm on board, the only thing is the duplication of the FlutterTextEditingDelta class with the Android one that I mentioned in the comments. We should verify that it's not doable in Dart somehow if you haven't already.
| @@ -0,0 +1,125 @@ | |||
| // Copyright 2013 The Flutter Authors. All rights reserved. | |||
There was a problem hiding this comment.
It would be so nice if we could figure out a way to do this sort of stuff in Dart and not duplicate all of this logic and the tests for each platform, but I'm still not sure if it's really possible.
There was a problem hiding this comment.
The logic that I was referring to has been moved to the framework since I last reviewed 🎉
There was a problem hiding this comment.
Alas, it is Flutter's burden to carry. The more platform stuff we do, the less the developer has to worry as things will "just work"
| [self setSelectedTextRangeLocal:[FlutterTextRange | ||
| rangeWithNSRange:[self clampSelection:selectedRange | ||
| forText:self.text]]]; | ||
| _currentTextEditingDelta = [[FlutterTextEditingDelta alloc] |
There was a problem hiding this comment.
Is this here just for updating the selection etc. with an equality delta?
There was a problem hiding this comment.
Yeah an equality delta is one where no text has changed, but the selection and/or composing regions have potentially been updated, so here we just set the current delta to an equality one for a selection update.
There was a problem hiding this comment.
I think you probably need to release the current _currentTextEditingDelta object before you assign a new one?
| if (_enableDeltaModel) { | ||
| [self updateEditingStateWithDelta]; | ||
| } else { | ||
| NSLog(@"We should never be here"); |
There was a problem hiding this comment.
Left over logs you can ignore them.
Renzo-Olivares
force-pushed
the
text_editing_deltas_ios
branch
from
6df00c2 to
041dd40
Compare
| [self updateEditingState]; | ||
|
|
||
| if (_enableDeltaModel) { | ||
| [self updateEditingStateWithDelta:[[FlutterTextEditingDelta alloc] initWithNonText:self.text]]; |
There was a problem hiding this comment.
The new FlutterTextEditingDelta will leak if you don't autorelease it. You can either:
- change the initializer to be a static method with a different name that returns an
autoreleasedFlutterTextEditingDelta(new,allocimplies you own the object so the ref count is going to be +1). - call autorelease every time you alloc a new
FlutterTextEditingDelta.
Since the deltas are turned into json immediately I'd recommend approach 1.
| newText:(NSString*)newTxt | ||
| deltaStart:(NSInteger)newStart | ||
| deltaEnd:(NSInteger)newEnd { | ||
| _oldText = oldText; |
There was a problem hiding this comment.
Oh since the delta objects are immediately turned into json, not claiming the ownership of these strings should be fine.
| } | ||
|
|
||
| NSDictionary* deltaToFramework = @{ | ||
| @"oldText" : [NSString stringWithString:delta.oldText], |
There was a problem hiding this comment.
these are already copies I think? You should be able to directly use delta.oldText?
Renzo-Olivares
added
the
waiting for tree to go green
5 participants
Description
Design Doc
This change aims to collect the changes made to the editing value in the iOS text input plugin, and send them to the framework through a platform channel.
Framework PR: flutter/flutter#88477
Related Issues
Partially fixes flutter/flutter#87972
Tests
Pre-launch Checklist
writing and running engine tests.
///).