-
Notifications
You must be signed in to change notification settings - Fork 24.4k
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
Include existing attributes in newly typed text #47018
Conversation
This pull request was exported from Phabricator. Differential Revision: D64352310 |
Summary: This change makes it so that newly typed text in a TextInput will include the existing attributes present based on cursor position. E.g. if tou type after an inner fragment with blue text, the next character will be blue (or, an event emitter specific to an inner fragment will also be expanded). This is a behavior change for the (admittedly rare) case of uncontrolled TextInput with initially present children AttributedText, but more often effect controlled components, before state update (we are after, less likely to need to reset AttributedString because of mismatch). Originally included this in D64121570, but it's not needed to fix the common case since we include paragraph-level event emitter as part of default attributes, and has some of its own risk, so I decided it is better separate. Changelog: [iOS][Changed] - Include existing attributes in newly typed text Differential Revision: D64352310
7c0965f
to
e871a87
Compare
This pull request was exported from Phabricator. Differential Revision: D64352310 |
Summary: This change makes it so that newly typed text in a TextInput will include the existing attributes present based on cursor position. E.g. if tou type after an inner fragment with blue text, the next character will be blue (or, an event emitter specific to an inner fragment will also be expanded). This is a behavior change for the (admittedly rare) case of uncontrolled TextInput with initially present children AttributedText, but more often effect controlled components, before state update (we are after, less likely to need to reset AttributedString because of mismatch). Originally included this in D64121570, but it's not needed to fix the common case since we include paragraph-level event emitter as part of default attributes, and has some of its own risk, so I decided it is better separate. Changelog: [iOS][Changed] - Include existing attributes in newly typed text Reviewed By: cipolleschi Differential Revision: D64352310
e871a87
to
ad9e8c3
Compare
This pull request was exported from Phabricator. Differential Revision: D64352310 |
Summary: This change makes it so that newly typed text in a TextInput will include the existing attributes present based on cursor position. E.g. if tou type after an inner fragment with blue text, the next character will be blue (or, an event emitter specific to an inner fragment will also be expanded). This is a behavior change for the (admittedly rare) case of uncontrolled TextInput with initially present children AttributedText, but more often effect controlled components, before state update (we are after, less likely to need to reset AttributedString because of mismatch). Originally included this in D64121570, but it's not needed to fix the common case since we include paragraph-level event emitter as part of default attributes, and has some of its own risk, so I decided it is better separate. Changelog: [iOS][Changed] - Include existing attributes in newly typed text Reviewed By: cipolleschi Differential Revision: D64352310
ad9e8c3
to
76f04c0
Compare
This pull request was exported from Phabricator. Differential Revision: D64352310 |
…n controlled single line TextInput on iOS (New Arch) (facebook#46970) Summary: Fixes facebook#44157 This one is a bit of a doozy... During auto-correct in UITextField (used for single line TextInput) iOS will mutate the buffer in two parts, non-atomically. After the first part, after iOS triggers `textFieldDidChange`, selection is in the wrong position. If we set full new AttributedText at this point, we propagate the incorrect cursor position, and it is never restored. In the common case, where we are not mutating text in the controlled component, we shouldn't need to be setting AttributedString in the first place, and we do have an equality comparison there currently. But it is defeated because attributes are not identical. There are a few sources of that: 1. NSParagraphStyle is present in backing input, but not the AttributedString we are setting. 2. Backing text has an NSShadow with no color (does not render) not in the AttributedText 3. Event emitter attributes change on each update, and new text does not inherit the attributes. The first two are part of the backing input `typingAttributes`, even if we set a dictionary without them. To solve for them, we make attribute comparison insensitive to the attribute values in a default initialized control. There is code around here fully falling back to attribute insensitive comparison, which we would ideally fix to instead role into this "effective" attribute comparison. The event emitter attributes being misaligned is a real problem. We fix in a couple ways. 1. We treat the attribute values as equal if the backing event emitter is the same 2. We set paragraph level event emitter as a default attribute so the first typed character receives it After these fixes, scenario in facebook/react-native-website#4247 no longer repros in new arch. Typing in debug build also subjectively seems faster? (we are not doing second invalidation of the control on every keypress). Changes which do mutate content may be susceptible to the same style of issue, though on web/`react-dom` in Chrome, this seems to not try to preserve selection at all if the selection is uncontrolled, so this seems like less of an issue. I haven't yet looked at old arch, but my guess is we have similar issues there, and could be fixed in similar ways (though, we've been trying to avoid changing it as much as possible, and 0.76+ has new arch as default, so not sure if worth fixing in old impl as well if this is very long running issue). Changelog: [iOS][Fixed] - Fix cursor moving in iOS controlled single line TextInput on Autocorrection (New Arch) Reviewed By: javache, philIip Differential Revision: D64121570
Summary: This change makes it so that newly typed text in a TextInput will include the existing attributes present based on cursor position. E.g. if you type after an inner fragment with blue text, the next character will be blue (or, an event emitter specific to an inner fragment will also be expanded). This is a behavior change for the (admittedly rare) case of uncontrolled TextInput with initially present children AttributedText, but more often effect controlled components, before state update (we are after, less likely to need to reset AttributedString because of mismatch). Originally included this in D64121570, but it's not needed to fix the common case since we include paragraph-level event emitter as part of default attributes, and has some of its own risk, so I decided it is better separate. Changelog: [iOS][Changed] - Include existing attributes in newly typed text Reviewed By: cipolleschi Differential Revision: D64352310
76f04c0
to
690076a
Compare
This pull request was exported from Phabricator. Differential Revision: D64352310 |
Summary: This change makes it so that newly typed text in a TextInput will include the existing attributes present based on cursor position. E.g. if you type after an inner fragment with blue text, the next character will be blue (or, an event emitter specific to an inner fragment will also be expanded). This is a behavior change for the (admittedly rare) case of uncontrolled TextInput with initially present children AttributedText, but more often effect controlled components, before state update (we are after, less likely to need to reset AttributedString because of mismatch). Originally included this in D64121570, but it's not needed to fix the common case since we include paragraph-level event emitter as part of default attributes, and has some of its own risk, so I decided it is better separate. Changelog: [iOS][Changed] - Include existing attributes in newly typed text Reviewed By: cipolleschi Differential Revision: D64352310
This pull request has been merged in 557e344. |
Summary: Pull Request resolved: #47018 This change makes it so that newly typed text in a TextInput will include the existing attributes present based on cursor position. E.g. if you type after an inner fragment with blue text, the next character will be blue (or, an event emitter specific to an inner fragment will also be expanded). This is a behavior change for the (admittedly rare) case of uncontrolled TextInput with initially present children AttributedText, but more often effect controlled components, before state update (we are after, less likely to need to reset AttributedString because of mismatch). Originally included this in D64121570, but it's not needed to fix the common case since we include paragraph-level event emitter as part of default attributes, and has some of its own risk, so I decided it is better separate. Changelog: [iOS][Changed] - Include existing attributes in newly typed text Reviewed By: cipolleschi Differential Revision: D64352310 fbshipit-source-id: 90ef8c49f50186eadf777e81cf6af57e1aada207
Summary:
This change makes it so that newly typed text in a TextInput will include the existing attributes present based on cursor position. E.g. if tou type after an inner fragment with blue text, the next character will be blue (or, an event emitter specific to an inner fragment will also be expanded). This is a behavior change for the (admittedly rare) case of uncontrolled TextInput with initially present children AttributedText, but more often effect controlled components, before state update (we are after, less likely to need to reset AttributedString because of mismatch).
Originally included this in D64121570, but it's not needed to fix the common case since we include paragraph-level event emitter as part of default attributes, and has some of its own risk, so I decided it is better separate.
Changelog:
[iOS][Changed] - Include existing attributes in newly typed text
Differential Revision: D64352310