Skip to content

Commit

Permalink
TextInput: Refined contentSize calculation
Browse files Browse the repository at this point in the history
Summary: This fixes pretty bad issue when contentSize is calculated based on an intrinsic horizontal (width) limitation, not on a real/current horizontal (width) one.

Reviewed By: mmmulani

Differential Revision: D5422114

fbshipit-source-id: 0eb582aeb59d29530990d4faabf2f41baa79c058
  • Loading branch information
shergin authored and facebook-github-bot committed Jul 18, 2017
1 parent f5d9b52 commit 603cc48
Show file tree
Hide file tree
Showing 5 changed files with 46 additions and 5 deletions.
1 change: 1 addition & 0 deletions Libraries/Text/RCTBackedTextInputViewProtocol.h
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
@property (nonatomic, assign) UIEdgeInsets textContainerInset; @property (nonatomic, assign) UIEdgeInsets textContainerInset;
@property (nonatomic, strong, nullable) UIView *inputAccessoryView; @property (nonatomic, strong, nullable) UIView *inputAccessoryView;
@property (nonatomic, weak, nullable) id<RCTBackedTextInputDelegate> textInputDelegate; @property (nonatomic, weak, nullable) id<RCTBackedTextInputDelegate> textInputDelegate;
@property (nonatomic, readonly) CGSize contentSize;


// This protocol disallows direct access to `selectedTextRange` property because // This protocol disallows direct access to `selectedTextRange` property because
// unwise usage of it can break the `delegate` behavior. So, we always have to // unwise usage of it can break the `delegate` behavior. So, we always have to
Expand Down
8 changes: 4 additions & 4 deletions Libraries/Text/RCTTextInput.m
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -168,10 +168,10 @@ - (void)textInputDidEndEditing


- (CGSize)contentSize - (CGSize)contentSize
{ {
CGSize contentSize = self.intrinsicContentSize; CGSize contentSize = self.backedTextInputView.contentSize;
UIEdgeInsets compoundInsets = self.reactCompoundInsets; UIEdgeInsets reactPaddingInsets = self.reactPaddingInsets;
contentSize.width -= compoundInsets.left + compoundInsets.right; contentSize.width -= reactPaddingInsets.left + reactPaddingInsets.right;
contentSize.height -= compoundInsets.top + compoundInsets.bottom; contentSize.height -= reactPaddingInsets.top + reactPaddingInsets.bottom;
// Returning value does NOT include border and padding insets. // Returning value does NOT include border and padding insets.
return contentSize; return contentSize;
} }
Expand Down
8 changes: 8 additions & 0 deletions Libraries/Text/RCTUITextField.m
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -137,6 +137,12 @@ - (void)paste:(id)sender


#pragma mark - Layout #pragma mark - Layout


- (CGSize)contentSize
{
// Returning size DOES contain `textContainerInset` (aka `padding`).
return self.intrinsicContentSize;
}

- (CGSize)intrinsicContentSize - (CGSize)intrinsicContentSize
{ {
// Note: `placeholder` defines intrinsic size for `<TextInput>`. // Note: `placeholder` defines intrinsic size for `<TextInput>`.
Expand All @@ -145,11 +151,13 @@ - (CGSize)intrinsicContentSize
size = CGSizeMake(RCTCeilPixelValue(size.width), RCTCeilPixelValue(size.height)); size = CGSizeMake(RCTCeilPixelValue(size.width), RCTCeilPixelValue(size.height));
size.width += _textContainerInset.left + _textContainerInset.right; size.width += _textContainerInset.left + _textContainerInset.right;
size.height += _textContainerInset.top + _textContainerInset.bottom; size.height += _textContainerInset.top + _textContainerInset.bottom;
// Returning size DOES contain `textContainerInset` (aka `padding`).
return size; return size;
} }


- (CGSize)sizeThatFits:(CGSize)size - (CGSize)sizeThatFits:(CGSize)size
{ {
// All size values here contain `textContainerInset` (aka `padding`).
CGSize intrinsicSize = self.intrinsicContentSize; CGSize intrinsicSize = self.intrinsicContentSize;
return CGSizeMake(MIN(size.width, intrinsicSize.width), MIN(size.height, intrinsicSize.height)); return CGSizeMake(MIN(size.width, intrinsicSize.width), MIN(size.height, intrinsicSize.height));
} }
Expand Down
16 changes: 15 additions & 1 deletion Libraries/Text/RCTUITextView.m
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ - (void)setContentOffset:(CGPoint)contentOffset animated:(__unused BOOL)animated


- (CGFloat)preferredMaxLayoutWidth - (CGFloat)preferredMaxLayoutWidth
{ {
// Returning size DOES contain `textContainerInset` (aka `padding`).
return _preferredMaxLayoutWidth ?: self.placeholderSize.width; return _preferredMaxLayoutWidth ?: self.placeholderSize.width;
} }


Expand All @@ -167,6 +168,18 @@ - (CGSize)placeholderSize
return placeholderSize; return placeholderSize;
} }


- (CGSize)contentSize
{
CGSize contentSize = super.contentSize;
CGSize placeholderSize = self.placeholderSize;
// When a text input is empty, it actually displays a placehoder.
// So, we have to consider `placeholderSize` as a minimum `contentSize`.
// Returning size DOES contain `textContainerInset` (aka `padding`).
return CGSizeMake(
MAX(contentSize.width, placeholderSize.width),
MAX(contentSize.height, placeholderSize.height));
}

- (void)layoutSubviews - (void)layoutSubviews
{ {
[super layoutSubviews]; [super layoutSubviews];
Expand All @@ -179,6 +192,7 @@ - (void)layoutSubviews


- (CGSize)intrinsicContentSize - (CGSize)intrinsicContentSize
{ {
// Returning size DOES contain `textContainerInset` (aka `padding`).
return [self sizeThatFits:CGSizeMake(self.preferredMaxLayoutWidth, INFINITY)]; return [self sizeThatFits:CGSizeMake(self.preferredMaxLayoutWidth, INFINITY)];
} }


Expand All @@ -187,7 +201,7 @@ - (CGSize)sizeThatFits:(CGSize)size
// Returned fitting size depends on text size and placeholder size. // Returned fitting size depends on text size and placeholder size.
CGSize textSize = [self fixedSizeThatFits:size]; CGSize textSize = [self fixedSizeThatFits:size];
CGSize placeholderSize = self.placeholderSize; CGSize placeholderSize = self.placeholderSize;
// Returning size DOES contain `textContainerInset`. // Returning size DOES contain `textContainerInset` (aka `padding`).
return CGSizeMake(MAX(textSize.width, placeholderSize.width), MAX(textSize.height, placeholderSize.height)); return CGSizeMake(MAX(textSize.width, placeholderSize.width), MAX(textSize.height, placeholderSize.height));
} }


Expand Down
18 changes: 18 additions & 0 deletions RNTester/js/TextInputExample.ios.js
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -823,6 +823,24 @@ exports.examples = [
placeholder="Placeholder defines intrinsic size" placeholder="Placeholder defines intrinsic size"
/> />
</View> </View>
<View>
<TextInput
style={{
fontSize: 16,
backgroundColor: '#eeeeee',
borderColor: '#666666',
borderWidth: 5,
borderTopWidth: 20,
borderRadius: 10,
borderBottomRightRadius: 20,
padding: 10,
paddingTop: 20,
}}
testID="multiline_textinput_with_flex"
multiline={true}
placeholder="Placeholder defines intrinsic size"
/>
</View>
</View> </View>
); );
} }
Expand Down

4 comments on commit 603cc48

@eliperkins
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this needs to get cherry picked into 0.47.0-stable to fix issues like #15326. Otherwise, multiline autoexpanding TextInputs will always return the same height for their content.

@shergin
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@eliperkins Great! I agree! Could you advocate requesting the cherry peak (probably here: #15318)?

@eliperkins
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shergin Done!

@abeltje1
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any news on this getting cherry-picked? Would be great!

Please sign in to comment.