Skip to content

Commit

Permalink
Fix incorrect height of single line TextInput without definite size (#…
Browse files Browse the repository at this point in the history
…48523)

Summary:
Pull Request resolved: #48523

Current AndroidTextInputShadowNode logic measures the height of the TextInput by fitting text into the constraints of the TextInput box. This results in the wrong height for single line TextInputs, since a single line TextInput is infinitely horizontally scrollable (whearas the outer TextInput component itself has a fixed width).

After this change, we measure text under single line textinputs with an infinite width constraint, then clamp to the final constraints of the TextInput, to better emulate what is happening under the hood.

iOS ended up solving this in a slightly different way, by measuring paragraph with `maximumNumberOfLines={1}` when not multiline, but think this is a bit more fraught. E.g. up until recently, it would have meant that the width could have been less than max width, depending on where line-breaking happened. I ended up duplicating the new logic to use for both instead (D66914447 will eventually deduplicate).

Changelog:
[Android][Fixed] - Fix incorrect height of single line TextInputs without definite size

Reviewed By: christophpurrer

Differential Revision: D67916827

fbshipit-source-id: b827185c4640835481794cb985c2b62dcf643abe
  • Loading branch information
NickGerleman authored and facebook-github-bot committed Jan 9, 2025
1 parent 8310d65 commit 9b646c8
Show file tree
Hide file tree
Showing 5 changed files with 85 additions and 40 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -234,17 +234,6 @@ TextAttributes BaseTextInputProps::getEffectiveTextAttributes(
return result;
}

ParagraphAttributes BaseTextInputProps::getEffectiveParagraphAttributes()
const {
auto result = paragraphAttributes;

if (!multiline) {
result.maximumNumberOfLines = 1;
}

return result;
}

SubmitBehavior BaseTextInputProps::getNonDefaultSubmitBehavior() const {
if (submitBehavior == SubmitBehavior::Default) {
return multiline ? SubmitBehavior::Newline : SubmitBehavior::BlurAndSubmit;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,6 @@ class BaseTextInputProps : public ViewProps, public BaseTextProps {
*/
TextAttributes getEffectiveTextAttributes(Float fontSizeMultiplier) const;

ParagraphAttributes getEffectiveParagraphAttributes() const;

#pragma mark - Props

/*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,15 +72,19 @@ class BaseTextInputShadowNode : public ConcreteViewShadowNode<
const LayoutContext& layoutContext,
const LayoutConstraints& layoutConstraints) const override {
const auto& props = BaseShadowNode::getConcreteProps();
auto textConstraints = getTextConstraints(layoutConstraints);

TextLayoutContext textLayoutContext{
.pointScaleFactor = layoutContext.pointScaleFactor};
return textLayoutManager_
->measure(
attributedStringBoxToMeasure(layoutContext),
props.getEffectiveParagraphAttributes(),
textLayoutContext,
layoutConstraints)
.size;
auto textSize = textLayoutManager_
->measure(
attributedStringBoxToMeasure(layoutContext),
props.paragraphAttributes,
textLayoutContext,
textConstraints)
.size;

return layoutConstraints.clamp(textSize);
}

void layout(LayoutContext layoutContext) override {
Expand Down Expand Up @@ -112,9 +116,7 @@ class BaseTextInputShadowNode : public ConcreteViewShadowNode<

AttributedStringBox attributedStringBox{attributedString};
return textLayoutManager_->baseline(
attributedStringBox,
props.getEffectiveParagraphAttributes(),
size) +
attributedStringBox, props.paragraphAttributes, size) +
top;
}

Expand Down Expand Up @@ -166,6 +168,30 @@ class BaseTextInputShadowNode : public ConcreteViewShadowNode<
return attributedString;
}

/*
* Determines the constraints to use while measure the underlying text
*/
LayoutConstraints getTextConstraints(
const LayoutConstraints& layoutConstraints) const {
if (BaseShadowNode::getConcreteProps().multiline) {
return layoutConstraints;
} else {
// A single line TextInput acts as a horizontal scroller of infinitely
// expandable text, so we want to measure the text as if it is allowed to
// infinitely expand horizontally, and later clamp to the constraints of
// the input.
return LayoutConstraints{
.minimumSize = layoutConstraints.minimumSize,
.maximumSize =
Size{
.width = std::numeric_limits<Float>::infinity(),
.height = layoutConstraints.maximumSize.height,
},
.layoutDirection = layoutConstraints.layoutDirection,
};
}
}

/*
* Returns an `AttributedStringBox` which represents text content that should
* be used for measuring purposes. It might contain actual text value,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,27 @@ AttributedString AndroidTextInputShadowNode::getMostRecentAttributedString()
: reactTreeAttributedString);
}

LayoutConstraints AndroidTextInputShadowNode::getTextConstraints(
const LayoutConstraints& layoutConstraints) const {
if (getConcreteProps().multiline) {
return layoutConstraints;
} else {
// A single line TextInput acts as a horizontal scroller of infinitely
// expandable text, so we want to measure the text as if it is allowed to
// infinitely expand horizontally, and later clamp to the constraints of the
// input.
return LayoutConstraints{
.minimumSize = layoutConstraints.minimumSize,
.maximumSize =
Size{
.width = std::numeric_limits<Float>::infinity(),
.height = layoutConstraints.maximumSize.height,
},
.layoutDirection = layoutConstraints.layoutDirection,
};
}
}

void AndroidTextInputShadowNode::updateStateIfNeeded() {
ensureUnsealed();

Expand Down Expand Up @@ -149,20 +170,24 @@ void AndroidTextInputShadowNode::updateStateIfNeeded() {
Size AndroidTextInputShadowNode::measureContent(
const LayoutContext& layoutContext,
const LayoutConstraints& layoutConstraints) const {
auto textConstraints = getTextConstraints(layoutConstraints);

if (getStateData().cachedAttributedStringId != 0) {
return textLayoutManager_
->measureCachedSpannableById(
getStateData().cachedAttributedStringId,
getConcreteProps().paragraphAttributes,
layoutConstraints)
.size;
auto textSize = textLayoutManager_
->measureCachedSpannableById(
getStateData().cachedAttributedStringId,
getConcreteProps().paragraphAttributes,
textConstraints)
.size;
return layoutConstraints.clamp(textSize);
}

// Layout is called right after measure.
// Measure is marked as `const`, and `layout` is not; so State can be updated
// during layout, but not during `measure`. If State is out-of-date in layout,
// it's too late: measure will have already operated on old State. Thus, we
// use the same value here that we *will* use in layout to update the state.
// Measure is marked as `const`, and `layout` is not; so State can be
// updated during layout, but not during `measure`. If State is out-of-date
// in layout, it's too late: measure will have already operated on old
// State. Thus, we use the same value here that we *will* use in layout to
// update the state.
AttributedString attributedString = getMostRecentAttributedString();

if (attributedString.isEmpty()) {
Expand All @@ -175,13 +200,14 @@ Size AndroidTextInputShadowNode::measureContent(

TextLayoutContext textLayoutContext;
textLayoutContext.pointScaleFactor = layoutContext.pointScaleFactor;
return textLayoutManager_
->measure(
AttributedStringBox{attributedString},
getConcreteProps().paragraphAttributes,
textLayoutContext,
layoutConstraints)
.size;
auto textSize = textLayoutManager_
->measure(
AttributedStringBox{attributedString},
getConcreteProps().paragraphAttributes,
textLayoutContext,
textConstraints)
.size;
return layoutConstraints.clamp(textSize);
}

Float AndroidTextInputShadowNode::baseline(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,12 @@ class AndroidTextInputShadowNode final
*/
AttributedString getMostRecentAttributedString() const;

/*
* Determines the constraints to use while measure the underlying text
*/
LayoutConstraints getTextConstraints(
const LayoutConstraints& layoutConstraints) const;

/*
* Creates a `State` object (with `AttributedText` and
* `TextLayoutManager`) if needed.
Expand Down

0 comments on commit 9b646c8

Please sign in to comment.