Skip to content

feat(textview): added maxLines property #7943

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

Merged
merged 10 commits into from
Jan 16, 2020

Conversation

surdu
Copy link
Contributor

@surdu surdu commented Oct 10, 2019

PR Checklist

What is the current behavior?

TextView doesn't have a way to limit the maximum number of lines

What is the new behavior?

TextView has a way to limit the maximum number of lines by specifying the maxLines property.

Fixes #4315

@cla-bot cla-bot bot added the cla: yes label Oct 10, 2019
@surdu surdu force-pushed the textview-maxlines branch 4 times, most recently from 18cd36f to a3b25c8 Compare October 15, 2019 17:03
@surdu surdu force-pushed the textview-maxlines branch from a3b25c8 to 974bdb1 Compare October 22, 2019 15:36
@MartoYankov
Copy link
Contributor

@surdu The EditableTextBase class is a base class for both the TextView and the TextField. The TextField component is a single line editable component, so it doesn't make much sense to have this property. Further, on iOS the UITextField doesn't have a textContainer property, so trying to set the property on TextField for iOS will cause a crash.

I think you should move this property to the TextView component specifically.

@surdu surdu force-pushed the textview-maxlines branch 2 times, most recently from c3af9e1 to 25f55d2 Compare November 6, 2019 12:06
@surdu surdu force-pushed the textview-maxlines branch 2 times, most recently from 870edfb to 97b2244 Compare November 6, 2019 18:15
@surdu
Copy link
Contributor Author

surdu commented Nov 6, 2019

@MartoYankov done

@MartoYankov
Copy link
Contributor

Hi, @surdu , sorry for the delayed response. This looks good, however I see that the feature behaves differently on iOS and Android right now.

  • On iOS the values -1 and 0 work and reset the value allowing infinite lines.
  • On Android -1 crashes the app and 0 doesn't allow even one line.

The -1 value on Android is also the default one, so it's important to not crash when resetting CSS rules for example.

Also the 0 value should reset to the default behavior. I see that on Android they are doing this by setting it to integer max value.

@MartoYankov
Copy link
Contributor

Also, the tns-core-modules-package/bundle-entry-points.js shouldn't be under source control. We have missed to add it to the git ignore. You can remove it from this PR.

@vakrilov
Copy link
Contributor

Hey @surdu
Do you think you can exclude the tns-core-modules-package/bundle-entry-points.js from the PR as Marto suggested.
Also as this PR is changing the Public API of the package - can you run npm run api-extractor and commit the changes in the api-reports/NativeScript.api.md file

@surdu surdu force-pushed the textview-maxlines branch from 7dcc40c to 1975774 Compare December 12, 2019 13:42
@surdu
Copy link
Contributor Author

surdu commented Dec 12, 2019

@vakrilov I did the 2 changes you asked for, but I don't have time right now to look over the default value issue mentioned by Martin Yankov. I'll do my best to also fix that problem by Monday.

Sorry, I was really busy these past weeks 😞

@surdu surdu force-pushed the textview-maxlines branch from 1ead5f2 to ca7e6c4 Compare December 20, 2019 21:45
@surdu
Copy link
Contributor Author

surdu commented Dec 20, 2019

@MartoYankov Now when you set the value to 0 or -1 works the same on android as well. Here is the commit that changed the behavior on Android.

@dtopuzov
Copy link
Contributor

test

@dtopuzov
Copy link
Contributor

test --ignore

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TextView Support For maxLines and minLines
6 participants