Add support for multiline TextInput via UITextView

[brentvatne]

@nicklockwood - Could I get a review of this?

Just took RCTTextField and ported it from UITextField to UITextView as you mentioned in another discussion, and removed any UITextField specific attributes.

• How do you think this should behave when there are subviews?
• Do you know how we can respond to the UIControlEventEditingDidEndOnExit event to respond to submit? Because UITextView isn't a UIControl we can't just use addTarget with UIControlEventEditingDidEndOnExit.
• Any other feedback?

Still going to look over the UITextView docs in more detail and make sure we expose all important options, and add it to the UIExplorer example, just putting this out here for feedback.

[nicklockwood]

Can you add this to the RCTText lib instead of the main React lib? Otherwise it will clash with our private FBTextView implementation.

[nicklockwood]

(I know that's rather inconsistent with the RCTTextField - we should move that too, but I'll do that in a separate diff.)

[nicklockwood]

This looks fantastic, thank you!

The only thing I can see that's missing is placeholder text support. There's no built-in support for this in UITextView, so it will need to be implemented as a private UILabel inside RCTTextView that shows and hides automatically when the text is edited.

[brentvatne]

@nicklockwood - having a few issues with getting it to respect padding, both on the placeholder (UILabel as you suggested) and the UITextView.

On the UITextView, padding works as expected only when there is text, otherwise it goes a bit nuts:

On UILabel, I think all we need to do is have it factor in the contentInsets of the the UITextView, of course UILabel doesn't support and I was wondering what your approach would be.

Thanks!

[nicklockwood]

OK, so there's a couple of things happening here: The contentInset property you declare in the interface is currently redundant because that property is declared already on UITextView and inherited.

You won't find it in the docs because it's not a property of UITextView itself, but is inherited from UIScrollView, which is UITextView's superclass.

Here's a good writeup of the problem, and the only viable solution, which seems to be to wrap the control in another view and apply the padding to that outer view instead: http://stackoverflow.com/questions/18560412/uitextview-content-inset - in React we could do that wrapping either in the native or the JS code, but I think native makes more sense in this case.

Probably easiest to make RCTTextView a subclass of RCTView, and add a regular UITextView as a private subview that it controls. The contentInset property would then be used to adjust the frame of the subview, not the inset of the scrollView.

To fix the placeholder label, just adjust it's frame to match tht inner textView's (it's frame may need to be inset by a few pixels to make the label text perfectly align with the textView text though).

[brentvatne]

@nicklockwood - thanks, have that working now, ran into another issue with setting fonts that seems to be an existing bug with RCTTextField:

RCTConvert.m
RCTTextFieldManager.m

Each time you set one of the font styles it obliterates the others, (UIFont *)UIFont:(UIFont *)font withFamily:(id)family size:(id)size weight:(id)weight style:(id)style doesn't seem to behave as you would expect (ignore nil values, create new UIFont with the non-nil options applied, return it). I'll look at resolving this too.

[brentvatne]

@nicklockwood - quickly hacked this together, fixes the issue, will need to circle back later to make sure it's robust:

+ (UIFont *)UIFont:(UIFont *)font withFamily:(id)family
              size:(id)size weight:(id)weight style:(id)style
{
  // Defaults
  NSString *const RCTDefaultFontFamily = @"Helvetica Neue";
  const RCTFontWeight RCTDefaultFontWeight = UIFontWeightRegular;
  const CGFloat RCTDefaultFontSize = 14;
  CGFloat fontSize;

  // Get existing properties
  BOOL isItalic = NO;
  BOOL isCondensed = NO;
  RCTFontWeight fontWeight = RCTDefaultFontWeight;
  if (font) {
    if (!family) {
      family = font.familyName;
    }

    isItalic = RCTFontIsItalic(font);
    isCondensed = RCTFontIsCondensed(font);
  }

  // Get font style
  if (style) {
    isItalic = [self RCTFontStyle:style];
  }

  // Get font size
  if (!size && font) {
    fontSize = font.pointSize;
  } else {
    fontSize = [self CGFloat:size] ?: RCTDefaultFontSize;
  }

  // Get font family
  NSString *familyName = [self NSString:family] ?: RCTDefaultFontFamily;


  // Get font weight
  if (!weight && font) {
    fontWeight = RCTWeightOfFont(font);
  } else {
    fontWeight = [self RCTFontWeight:weight];
  }

  if ([UIFont fontNamesForFamilyName:familyName].count == 0) {
    font = [UIFont fontWithName:familyName size:fontSize];
    if (font) {
      // It's actually a font name, not a font family name,
      // but we'll do what was meant, not what was said.
      familyName = font.familyName;
      NSDictionary *traits = [font.fontDescriptor objectForKey:UIFontDescriptorTraitsAttribute];
      fontWeight = [traits[UIFontWeightTrait] doubleValue];
    } else {
      // Not a valid font or family
      RCTLogError(@"Unrecognized font family '%@'", familyName);
      familyName = RCTDefaultFontFamily;
    }
  }

  // Get closest match
  UIFont *bestMatch = [UIFont fontWithName:font.fontName size: fontSize];
  CGFloat closestWeight;
  if (font && [font.familyName isEqualToString: familyName]) {
    closestWeight = RCTWeightOfFont(font);
  } else {
    closestWeight = INFINITY;
  }
  for (NSString *name in [UIFont fontNamesForFamilyName:familyName]) {
    UIFont *match = [UIFont fontWithName:name size:fontSize];
    if (isItalic == RCTFontIsItalic(match) &&
        isCondensed == RCTFontIsCondensed(match)) {
      CGFloat testWeight = RCTWeightOfFont(match);
      if (ABS(testWeight - fontWeight) < ABS(closestWeight - fontWeight)) {
        bestMatch = match;
        closestWeight = testWeight;
      }
    }
  }

  // Safety net
  if (!bestMatch) {
    RCTLogError(@"Could not find font with family: '%@', size: %@, \
                weight: %@, style: %@", family, size, weight, style);
    bestMatch = [UIFont fontWithName:[[UIFont fontNamesForFamilyName:familyName] firstObject]
                                size:fontSize];
  }

  return bestMatch;
}

This fixes a bug with the regular RCTTextField where multiple font attributes can't be set as well.

[nicklockwood]

Yes, that is indeed how the UIFont stuff is supposed to work. If you can provide some examples of it not working, I'll dig in and see if I can figure out the problem.

[brentvatne]

@nicklockwood - try setting fontWeight: bold and fontFamily: 'Superclarendon' on a TextInput styles, only one will work at a time

[nicklockwood]

Hah, you were too quick for me ;-)

[brentvatne]

😄

[brentvatne]

Heading out for a run, will finish this off and push code up later tonight! Edit: Actually, tomorrow 😄 Edit Edit: Actually, Monday 😄

[brentvatne]

Possible API for more placeholder customization - probably for a separate diff but just documenting here: #1011 (comment)

[josebalius]

Looking for to this getting merged guys! Great job.

[brentvatne]

@nicklockwood - Changes up now, seems to be working well!

I'm curious - I didn't notice tests for RCTConvert, do you think it's worth setting that up here to test the changes I made to it? If so, any chance you could pull the branch and add some scaffolding for the tests so I can be sure I'm following your preferred style?

[nicklockwood]

It looks like the <Text> example is totally broken - I think the changes to +[RCTConvert UIFontXX] may need some more work :-)

[brentvatne]

@nicklockwood - woops, I'll check that out, thanks

[brentvatne]

@nicklockwood - I've reverted those changes, I think they are better suited for their own diff as they have implications beyond multiline text.

[brentvatne]

@nicklockwood - Scratch that, I fixed the issue and added an example that includes a styled multiline text input. Everything in the example app is working fine now with that fix! Still some work to go to get all props supported though.

[brentvatne]

@nicklockwood - finished adding support for remaining attributes, ready for review again 😄

[nicklockwood]

This is great, thanks! I've merged it internally.

[lukasredev]

I just compared the implementation of RCTTextView with RCTText. Is there a reson why the TextView functionality is completely implemented in RCTTextView and not separated into a RCTTextView and RCTShadowTextView class?

I'am asking because I think I'd be nice to have the RCTTextView automatically measure it's height like implemented in the RCTShadowText object. I forked the repository and moved most of the code from RCTTextView into a RCTShadowTextView class and implemented the RCTMeasure function.

What do you think about this approach?

[nicklockwood]

@lukasreichart UITextView isn't thread safe, so creating and setting one up on the shadow thread might be problematic.

It would be good if it was possible to calculate the height though. Maybe it can be done by emulating the TextView layout using an NSTextContainer in the shadow view?

[ide]

@lukasreichart that sounds really good in that the default height (0px) is useless at the moment.

Following the line of thinking in @nicklockwood's suggestion, you might want to take a look at AsyncDisplayKit's ASTextNode/ASEditableTextNode to see how they support background-thread layout and also other features like text selection. It may make sense to jettison UITextView and use a reimplemented version altogether, which might sound crazy but ASEditableTextNode did it on top of TextKit.

[ide]

setting one up on the shadow thread might be problematic.

Yeah - if my memory serves, UITextView is indeed a UIKit class that needs to do its layout on the main thread.

[nicklockwood]

@ide "which might sound crazy" - yep, sounds pretty crazy :-)

At least as a first step, I think given the padding, font and text, it should be possible to calculate the correct size using the NSString size functions, or an NSTextContainer. It might not be pixel perfect, but it would be a hell of a lot simpler than re-implementing all the editing functions.

It would certainly be a cool project to try to write UITextView from scratch, but apart from anything else, we're trying to keep the total code size of the library down by re-using standard UIKit controls as much as possible. If we're going to build completely bespoke stuff, it's preferable to do it in JS so it's cross-platform.

[lukasredev]

@nicklockwood Thanks for clearing this up.
Your approach sounds reasonable to me. I'll try it and let you know as soon as I got something to work.

@ide 👍 Thanks for the input, I'll check it out. ( lots of work )

[lukasredev]

@nicklockwood Quick question: What's the best practise to send an event from a RCTTextView instance to it's counter part on the shadow thread?
I need to update the shadow object, when the text of the UITextView changes ...

[nicklockwood]

@lucasreichart Interesting... I don't think we use that pattern anywhere. There's two problems - the first is getting a reference to the shadow queue, and the second is getting a reference to the shadowView. The former can be done with:

dispatch_async(self.bridge.uiManager.methodQueue, ^{ ... });

The latter is a bit tricker. Do you need that information to be updated live as the user types, or is it sufficient to get it whenever layout is performed?

[lukasredev]

Situation: If the user is typing to the TextView the RCTTextView calculates if it's necessary to update the layout. When the layout is updated the fillCSSNode of the RCTShadowTextView is called, which sets the measure function for the css node.
In the measure function I need to be able to access the current text displayed by the UITextView to calculate the height.

You're probably right I don't need live updates, but I need to access the text attribute of the UITextView whenever layout is performed.

[nicklockwood]

TextView should probably be a managed component, so that a re-render clears the text unless the text state has been updated. If that was the case, you could ensure that the shadowView always had the correct text value simply by adding RCT_EXPORT_SHADOW_PROPERTY(text, NSString) to the RCTViewManager.

@vjeux am I understanding the concept of managed components correctly here?

[vjeux]

We call them controlled component and yes they do what you described.

The tricky part with react native is because of the asynchronous nature having them controlled is harder to implement correctly. Cc @sahrens who did most of the work there

[HananeAlSamrout]

hello, please fix also the issue of multiline with textAlign, try textAlign right with multiline true, it is not working for the 'placeHolder' , but for the text there is no problem.