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

Allow for Text components to support a forced minimum number of lines (numberOfLines) #21407

Closed
B3rry opened this issue Sep 28, 2018 · 6 comments
Labels
Resolution: Locked This issue was locked by the bot. Type: Discussion Long running discussion.

Comments

@B3rry
Copy link

B3rry commented Sep 28, 2018

For Discussion

As of 0.57.0, the Text component's numberOfLines prop is used to define the maximum number of lines of a string that will be rendered before truncation occurs. This is a bit of a misnomer, as one may anticipate that this prop w

Should this prop be renamed to something such as maxNumberOfLines? Would the maintainers consider the possibility of adding an additional prop, such as minNumberOfLines, which would force the Text component to occupy the minimum space necessary to render the defined amount of lines?

Motivation

Both Android and iOS's interface guidelines specify that when producing a tappable list of items (such as a list of messages) these items should be of even height.

In order to maintain accessible support for font scaling, a cell's height should be inferred from the height of the content, however, it is currently impossible to achieve a consistent inferred height from Text's available props.

Current Behavior

These cells do not have a consistent height
simulator screen shot - iphone xs max - 2018-09-28 at 13 45 35

Anticipated Behavior

These cells have a consistent height
simulator screen shot - iphone xs max - 2018-09-28 at 13 52 45

@B3rry
Copy link
Author

B3rry commented Sep 28, 2018

As a workaround, concatenating a series of newline characters equivalent to your numberOfLines prop (terminated with an nbsp #21406 #21612 ) will help you achieve the anticipated behavior in the meantime. (I have not validated that this does not cause issues with screen readers)

@B3rry B3rry changed the title Allow for Text components to support a forced minimum number of lines. Allow for Text components to support a forced minimum number of lines (numberOfLines) Sep 28, 2018
@vovkasm
Copy link
Contributor

vovkasm commented Oct 3, 2018

I personally think that in the example, cell height should be fixed, and contained elements should layout according to available space...

@B3rry
Copy link
Author

B3rry commented Oct 9, 2018

@vovkasm, @dooboolab,

Cell heights cannot be fixed, due to accessibility concerns. Users who have font scaling set to a high value will have text clipping issues. The height must be dynamic. Below is an example of what happens when font scaling is set in a device's accessibility settings when using static heights:

img_0473

@vovkasm
Copy link
Contributor

vovkasm commented Oct 9, 2018

@B3rry By fixed I mean that cell height should be set on View, not that this value should be a constant hardcoded in code... Of course you can calculate this value depending on various system conditions. Truth is that yoga currently can't handle text at all and proposed minNumberOfLines property simple will not resolve this...
See, you set minNumberOfLines, but text not fit in width... you cell will be higher than others... how you set constraints that all cells has the same size and at the same time wrap their content?
Okey, you set minNumberOfLines and maxNumberOfLines (to same value, or cells again will not be same height)... how then you set size for you icons on left side? Etc... etc...

@B3rry
Copy link
Author

B3rry commented Oct 9, 2018

@vovkasm

cell height should be set on View

Are you saying a calculated height should be applied directly to the View to achieve this? Such as:

 <View style={{height: SomeCalculatedHeight}}>

Yes, this is an option, but at that point you're still not allowing for standard Flex principles to create a consistently sized text component in a dynamic layout. Is there any reason why consistent, inferred heights should only be available through calculation?

See, you set minNumberOfLines, but text not fit in width... you cell will be higher than others...

Unsure what you mean.

how you set constraints that all cells has the same size and at the same time wrap their content?

This is already a factor of numberOfLines. It does exactly that: defines a set height, wraps the text. Additionally, it truncates text when it runs over. I'd imagine setting a unique offset (bump up the padding?) on each text container that does not meet the calculated size of a text container with n lines could work.

minimumTextContainerHeight = heightOfSingleLine * minimumNumberOfLines + totalVerticalPadding

verticalPaddingToMeetMinimumHeight = minimumTextContainerHeight - (heightOfSingleLine * numberOfRenderedLines)

There's probably some considerations around how to take around how you get to some of these values, especially given font scaling and line-height inconsistencies between render in iOS and android. I'm unsure of the full scope of these.

Okey, you set minNumberOfLines and maxNumberOfLines (to same value, or cells again will not be same height)... how then you set size for you icons on left side? Etc... etc...

You could use the Flex, nativeEvent and Dimensions achieve this. That's how it's done now. Although normal behavior for iOS elements is to not scale icons and images when the text does (in most cases). If you really wanted to, you could use PixelRatio.getFontScale() to scale the Entire UI, or handle responsiveness issues.

This process seems rather complex to produce a consistently sized, responsive component. This is a very common UX pattern in iOS and Android, and I don't think relying on each developer to implement their own solution is best, in this case.

@cpojer
Copy link
Contributor

cpojer commented Mar 19, 2019

Thanks for starting this discussion. I don't think it makes sense to add minNumberOfLines but I'm open to accepting a PR that changes numberOfLines to maxNumberOfLines. I'm gonna close this issue but feel free to send us that PR :)

@cpojer cpojer closed this as completed Mar 19, 2019
@facebook facebook locked as resolved and limited conversation to collaborators Mar 19, 2020
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label Mar 19, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Resolution: Locked This issue was locked by the bot. Type: Discussion Long running discussion.
Projects
None yet
Development

No branches or pull requests

5 participants