Consistent rows/numberOfLines prop on both Android and iOS

[chrispader]

Feature Request

The rows/numberOfLines prop on the TextInput component is currently only available on Android. @Szymon20000 has been working on a PR that adds this prop on iOS and provides a consistent approach on how this prop affects the TextInput.

At the moment, on iOS there is no way to explicitly set a TextInput to a specific number of lines without manually setting the actual height in style. Creating a TextInput which should have a specific number of lines is therefore only possible, by somehow calculating the line height by font size, spacing, etc....

Problems

The rows/numberOfLines props currently behave different on Android/Web and between different architectures as described in this comment.

Additionally, there are two props for the same use-case: rows and numberOfLines. We should reduce these to only one prop rows. There already is an ongoing discussion about replacing numberOfLines with rows completely: facebook/react-native#35703 (comment)

Proposal

On all platforms, we want to have 2 props:

• rows: Sets the fixed number of rows on a TextInput.
  • The TextInput will always have these number of lines even if the content is less
  • If the number of rows is higher than maxRows, it will be capped to the defined maximum
• maxRows: Sets the maximum number of rows/lines up to which a TextInput can grow.
  • If rows is not set, the TextInput will start with only 1 row and can grow up to the maximum number of rows
  • If rows is set, the TextInput will always have the defined rows.

Note

I'm ignoring numberOfLines/maxNumberOfLines props, as we want to remove them anyway

In this comment @necolas also stated that we'll want to unify this prop on all platforms at some point.

Internal testing

@cipolleschi mentioned here that this change will affect internal (screenshot) testing. I think in order to achieve this interface, we'll eventually want to update the internal tests. Up to this point though, we can also instead name these props displayedRows and maxDisplayedRows, to prevent internal tests from failing.

What's next?

@Szymon20000 has already implemented this on iOS and also improved the Android code. We'll want to continue reviews on the first (iOS) PR and possibly split it up even more (between Fabric and Paper?).

---

friendly bump @Szymon2000 @roryabraham @cipolleschi @necolas

[cipolleschi]

As stated, the two existing props have different behaviors across platforms.
We cannot reuse them:

• this will require too much work for any Meta engineer to update the internal system
• we cannot outsource that to some external contributor because they won't have access to our code base
• we don't have capacity to follow that effort.

On top of those practical things, changing the existent behavior of existing props is a huge breaking change. It will not only break MEta code, but every other app that uses those properties expecting the current behavior.

Personally, I'm okay with a flow like this:

• create an (or two) additional props with the desired behavior.
• if we think that the behavior of old props is wrong, we can deprecate them.

Disclaimer: I will not have capacity to follow this effort. Somebody else @ Meta should champion this.

[chrispader]

Hmm, i'm personally okay with having this functionality in a separate component to avoid conflicts with other codebases, but it would be cool to ship and maintain this alongside the react-native core library.

[cipolleschi]

Why, If I can ask?
Historically, there have been an effort called lean-core, for which we are moving out of the core those extra components. Bringing a new one in, for a feature we don't use internally, seem in direct opposition to that effort and to the direction of the framework. 🤔

[fabOnReact]

@cipolleschi regarding Meta effort for lean-core for Text/TextInput, would they be open to add more types for third-party libraries that want to work with raw text as children prop? thanks a lot (related Issue facebook/react-native#25136)

https://github.com/fabriziobertoglio1987/react/blob/96740fbff5de90b3899db8d9c7df668c675c632b/packages/react-native-renderer/src/ReactNativeHostConfig.js#L200-L210

https://github.com/fabriziobertoglio1987/react-native/blob/385473522cbc525aad08500f5a752dea734c14c3/packages/react-native/Libraries/Renderer/implementations/ReactNativeRenderer-dev.js#L5373

[cipolleschi]

Uhm, good question.
I don't have much context on this, but perhaps @sammy-SC or @javache can shed more light on why this is happening and also if that is still true for Fabric.

In the Fabric example from the documentation it is possible to render some text without using a Text component or any one specified in the HostConfig.

[sebazelonka]

My 2 cents here. Having TextArea as part of lean-core is the way to go, as Textarea is an everyday use case and part of the web platform. It is the expected behavior when you need something like that on your app.