-
Notifications
You must be signed in to change notification settings - Fork 6.8k
feat(input): autosize sets default amount of rows to one #4906
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
Conversation
src/lib/input/autosize.ts
Outdated
* Browsers by default set the rows property to two. Therefore setting the minRows and maxRows | ||
* binding to something below two doesn't work. | ||
*/ | ||
private _setInitialRows() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any reason this can't be a simple host binding of {'rows': '1')
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not at all. I totally forgot about this 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. Not sure why the screenshot tests fail. The images don't seem to be different.
f2ba6b2
to
fad1aa2
Compare
src/lib/input/autosize.ts
Outdated
@@ -10,6 +10,9 @@ import {Directive, ElementRef, Input, AfterViewInit} from '@angular/core'; | |||
exportAs: 'mdTextareaAutosize', | |||
host: { | |||
'(input)': 'resizeToFitContent()', | |||
// Textarea elements that have the directive applied should have a single row by default. | |||
// Browsers normally show two rows by default and this makes the minRows and maxRows unusable. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe a bit overstated: without this it's just a problem for minRows=1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In theory it could be also for maxRows
because if someone sets maxRows=1
it would be still at two rows I think.
But yeah, who would set maxRows=1
if he wants autosize.
* The autosize directive now sets the rows property of the textarea to one by default. * This is necessary because browsers by default set the rows property to two and therefore setting the minRows and maxRows binding to something below two doesn't work. Fixes angular#4852
a74a1cf
to
89b378e
Compare
@jelbourn Updated the comment. Let me know if I should change anything else. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Note: The test looks more complex than you'd expect. I wanted to have the test also confirm that the textarea can still grow (even if rows is set to
1
always)Fixes #4852