Skip to content
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

feat: upgrade Text and TextInput to 2716f53 #227

Open
wants to merge 143 commits into
base: master
Choose a base branch
from
Open

feat: upgrade Text and TextInput to 2716f53 #227

wants to merge 143 commits into from

Conversation

aleclarson
Copy link
Collaborator

@aleclarson aleclarson commented Feb 14, 2019

🚧 Work in progress

Opening this PR now that its changes are compiled without error. I'll be testing it today, so I decided to share my progress in case anyone wants to help out.

2/27/2019: I've fixed all of the issues that will affect my current project. I don't have time to fix the remaining bugs, so it would be great if anyone could lend a hand! Simply direct any PRs toward the text branch of this repository. 👍 Once all of the known bugs are fixed, we can merge this! 🚀

 

Details

The ./React, ./ReactCommon and ./Libraries/Text directories have been updated to this commit: facebook/react-native@2716f53 (from 01/09/2018 to 01/24/2018)

The goals are as follows:

  • Upgrade to the "new" rewrite of text-related native components
  • Fix all visual bugs in <TextInput>
  • Make the "tab" key focus the next editable <TextInput>
  • Make multiline <TextInput> scrollable when a maxHeight is used
  • Make <TextInput> placeholders more memory-efficient
  • Bunch of small fixes, probably

 

Known bugs

  • password prop not working
  • Styled Text inside multiline TextInput not working
  • Special fonts may have incorrect line height
  • focusRingType prop not working
  • selection prop not working
  • Tabbing to an offscreen TextInput does not scroll its enclosing scroll views
  • Scrollable TextInput jumps to top when a new line is added to the end via the "return" key
  • Caret is slightly thinner when the TextInput is empty

 

Related issues

Closes #216
Closes #148

 

Skipped commits

The following commits were skipped and may be cherry-picked in the future:

 

Cherry-picked commits

I also picked some commits ahead of facebook/react-native@2716f53 that fix some issues:

@ptmt
Copy link
Owner

ptmt commented Feb 15, 2019

Wow, this one is hard. Text inputs are particularly hard to keep up with RN for iOS due to differences between UIKit and AppKit.

@aleclarson
Copy link
Collaborator Author

Haha yep. It's been... a learning experience. 😆

The "mouseEnter" event in browsers does not bubble, while "mouseOver" does.
The "mouseLeave" event in browsers does not bubble, while "mouseOut" does.
By subclassing NSWindow and overriding the "sendEvent:" method, we can avoid using the NSGestureRecognizer class, which seems to cause issues with NSTextView event handling.

The RCTWindow class reimplements input event handling, which solves the following issues:

  - Skip "touchStart" events that target a focused NSTextView

  - Emit "mouseOut" event when the mouse leaves the RCTWindow

  - Emit "touchCancel" when the mouse leaves the RCTWindow

  - Support "mouseMove" events (which can be coalesced)

  - Emit "mouseMove" event right after "mouseUp" events

  - Blur the focused NSTextView when clicking outside it

  - Support "contextMenu" events

  - Add "altKey", "ctrlKey", "metaKey", and "shiftKey" properties to JS mouse events

  - Use "convertPoint:toView:" to compute the relative mouse location
shergin and others added 19 commits February 27, 2019 15:29
Differential Revision: D6641403

fbshipit-source-id: f478810413aa49b44c060db898e7e8698bddb8e1
Reviewed By: emilsjolander

Differential Revision: D6682933

fbshipit-source-id: 0fd90fdaf5ca4f9b7a11cbd15d8c54c7d0ce8a03
Reviewed By: emilsjolander

Differential Revision: D6675111

fbshipit-source-id: 884659fabb05033b4d43d3aa6629e22481d39b7e
Summary:
Currently, we can dirty leaf nodes with `measure` function, we also can get `dirty` status for any node, but we cannot handle a moment when this change happen. This diff introduces a new call-back-manner handler for it.
We need this to plug Yoga inside and outside other layout systems without maintaining own dirty propagation infrastructure.
Consider using Yoga for flex-box layout in React Native where we can have deeply nested layout like `<View><Text><View><Text/></View></Text></View>` where all content of all <Text> nodes are laid out using native text/inline (not flex-box!) layout system. In this case, when some change dirties some deeply nested Yoga node, we have to propagate the dirty state down to outer one. Having this handler makes possible to wire up `on-dirty` handler on the root node and `setDirtied` for the leaf node.
Removing custom dirting mechanism from React Native should drastically simplify rendering layer and bring a huge performance win.

Reviewed By: emilsjolander

Differential Revision: D6597856

fbshipit-source-id: 6588cd712f9c1dede4af32f3d326f90103e48ff0
Reviewed By: emilsjolander

Differential Revision: D6682929

fbshipit-source-id: 3607aab1544b62b1126c5d75b2f6fb8f5ca2d45f
Reviewed By: emilsjolander

Differential Revision: D6682956

fbshipit-source-id: 31c60e0eae906e1434a6969f3cd786fcaf9097a5
Reviewed By: emilsjolander

Differential Revision: D6683190

fbshipit-source-id: c37e57d02cc4475eb8181a2bb003c555bdb0aaea
Reviewed By: emilsjolander

Differential Revision: D6683205

fbshipit-source-id: d30003d90d634c644d92c833e58165b073d4d13e
Reviewed By: emilsjolander

Differential Revision: D6683270

fbshipit-source-id: a26663006419e13cb783e9849183e3c665f59b3c
Reviewed By: emilsjolander

Differential Revision: D6683313

fbshipit-source-id: 5ee458c2f4698768724901df0e3f5d8805c7c8f5
Reviewed By: emilsjolander

Differential Revision: D6683387

fbshipit-source-id: 83f64101faa700933771c69b222056ec2a6b8d1e
Reviewed By: johnislarry

Differential Revision: D6701327

fbshipit-source-id: 17630f336e2b275c1de30ebfa32d1cbfbc1b9634
…re passed to nativeRequire

Differential Revision: D6695769

fbshipit-source-id: b578b9d52ed711fb5a3e51717ac555fa8a232d7a
Summary: Because setting `intrinsicContentSize` for `RCTSurfaceRootView` doesn't have much sense.

Reviewed By: mmmulani

Differential Revision: D6701107

fbshipit-source-id: 259cdd27339bba3e8c9f98b6ca34affeb87f298c
Summary: Now it actually works.

Reviewed By: mmmulani

Differential Revision: D6701105

fbshipit-source-id: 16f3f4e319f874f9a08867b784d13aad4fa22aeb
Summary: Trivial.

Reviewed By: sahrens

Differential Revision: D6715229

fbshipit-source-id: 13ae84920c98e0d8e8f1b64aeadfa770b64ea3b4
Reviewed By: sahrens

Differential Revision: D6688488

fbshipit-source-id: da020b3510ac7163f63cb5cebc27ec4306b1136c
Summary: Trivial.

Reviewed By: sahrens

Differential Revision: D6690929

fbshipit-source-id: 82906cd4a0eec320f998661ed48b9352b9b72670
Reviewed By: sahrens

Differential Revision: D6690930

fbshipit-source-id: a6ce5f006b4e6d63feef0f9c0743fb19b0e546fa
The `selectedRange` is modified by the "mouseDown" event (after the call to "becomeFirstResponder").

To ensure that `selectTextOnFocus` is respected, we delay the "textFieldDidFocus" call until the next event loop.
The cursor still goes where the user clicked. This change affects programmatic focus and tabbing.
Between invalidating a bridge and suspending its JS thread, native modules may have their methods called.

Only warn when a native module has been invalidated, which happens right before its JS thread is suspended.

Avoid initializing a native module's instance if its bridge is invalidated.
This gives each RCTUITextField its own field editor.

This is required in order to fix paste detection and the blurOnSubmit prop.
The plan to preserve line numbers did not work out due to the amount of required changes,
so there's really no point in keeping this whitespace around.
When truthy, the example is not rendered.

This allows for less conflicts when merging examples from upstream.
Instead, replace the backedTextInputView based on the "password" prop.
@lhecker
Copy link

lhecker commented Feb 27, 2019

It's so nice seeing my old dusty project (NSLabel) being used like that... 🥰
Thanks again @aleclarson for the PRs you sent me! I already released it as version 1.1.0 on Cocoapods.
If you fix any other issues it'd be very kind of you if you could send me PRs for these as well. 🙂

@aleclarson
Copy link
Collaborator Author

@lhecker Of course! Thanks again for open-sourcing your work in 2015. 😉

@aleclarson
Copy link
Collaborator Author

The remaining bugs are up to the community to fix; unless I ever need them fixed for my own project, that is! Until then, this PR will remain unmerged. 😝

@aleclarson aleclarson added this to the 0.20.0 milestone Mar 3, 2019
@aleclarson aleclarson mentioned this pull request Mar 14, 2019
@aleclarson aleclarson mentioned this pull request Mar 25, 2019
@ptmt
Copy link
Owner

ptmt commented Mar 25, 2019

If only we have enough time to revive CI and tests. :(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Incorrect Multiline TextInput Bounds/Scroll TextInput has not correct border