Skip to content
This repository has been archived by the owner on Sep 5, 2024. It is now read-only.

feat(textarea): textarea shrinking and resizing #7991

Closed
wants to merge 1 commit into from

Conversation

crisbeto
Copy link
Member

@crisbeto crisbeto commented Apr 11, 2016

  • Changes the textarea behavior to consider the "rows" attribute as the minimum, instead of the maximum, when auto-expanding the element.
  • Adds a handle for vertically resizing the textarea element.
  • Simplifies the logic for determining the textarea height.
  • Makes the textarea sizing more accurate by using scrollHeight directly, instead of depending on the line height and amount of rows.
  • Avoids potential issues where the textarea wouldn't resize when adding a newline.
  • Adds the option to specify a maximum number of rows for a textarea via the max-rows attribute.

Fixes #7649.
Fixes #5919.
Fixes #8135.

BREAKING CHANGE
This changes the behavior from considering the "rows" attribute as the maximum to considering it as the minimum.

@ThomasBurleson @EladBezalel @devversion These are the textarea changes that we talked about and specced in the Google Doc.

@crisbeto crisbeto added the needs: review This PR is waiting on review from the team label Apr 11, 2016
@EladBezalel
Copy link
Member

@crisbeto is it auto growing/shrinkning according the content?

@crisbeto
Copy link
Member Author

It is.

@EladBezalel
Copy link
Member

@topherfangio please take a look as well

@topherfangio
Copy link
Contributor

@crisbeto This should be noted as a breaking change in the commit message (and it should have a label).

Just to confirm, this means that there is no longer a maximum size for the text area? Will this break any apps that depend on it having a maximum size?

.on('$md.drag', onDrag)
.on('$md.dragend', onDragEnd);

scope.$on('$destroy', function() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haven't used it much; do we need to deregister the drag gesture above (i.e. undo the $mdGesture.register?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd say so. I remember trying to find if there was a deregister method, but I couldn't find one.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The deregister function will be returned from the register call.

@crisbeto
Copy link
Member Author

Yes @topherfangio, this is a breaking change. Mainly because the old behavior was: start at 1 row or fit any of the text and go until the rows are reached. The new behaviour treats the rows as a minimum.

@topherfangio
Copy link
Contributor

@crisbeto Okay, awesome. Can you make sure to do a git commit --amend to add the BREAKING CHANGE notice to the commit message? It's important because we automatically generate a list of breaking changes when we build a new release, and this is what that script looks for :-)

@crisbeto
Copy link
Member Author

Done, hopefully it's not too picky with the placement.

@topherfangio
Copy link
Contributor

I just tested this locally and it looks pretty good, but I think there might be a small issue with the height calculation.

Particularly, when you have 5 rows of text (some might be newlines), there is about 20px of extra whitespace at the bottom. As soon as you start typing the 6th row, it does indeed grow, but only by a tiny amount so that there's only 8px of whitespace at the bottom. This was on both Chrome and Firefox, so I don't think it's browser-specific.

5 Rows
text-5-rows

6 Rows
text-6-rows

Also, is it true that once a user resizes the textarea, it no longer does any automatic resizing? If so, I think that's fine, but we may want to mention this in the docs. In fact, we may want to just add a Textarea Usage section to the docs with some additional info about how users should expect it to work and what options are available.

@crisbeto
Copy link
Member Author

crisbeto commented Apr 14, 2016

Regarding the height, I believe it's because 5 rows in the textarea don't necessarily translate to 5 lines of text. We use offsetHeight to measure the height of 1 row of text, but that measurement includes padding, which means that the more padding we have, the more the measurement will be offset. E.g. here's an example of 5 rows, but with 20px of padding:

angular_material_-demos__input-_google_chrome_2016-04-14_18-28-11

I wouldn't say it's too much of an issue in our case, because the design doesn't have a lot of padding by default. I've tried using getComputedStyle to remove the padding, but that was also problematic because:

  1. It didn't behave consistently between IE and other browsers.
  2. We'd have to either call it on each keypress or assume that it would't change.

Otherwise I've also tried resizing the textarea purely with the rows attribute, but it wasn't reliable on IE either. And the other alternative is having a whole separate DOM element that mirrors the text and is being used for measurements, but that can turn into a big mess.


As for the resizing, I'll add some info in the docs.
Docs have been added.

@crisbeto crisbeto added the in progress Mainly for in progress PRs, but may be used for issues that require multiple PRs label Apr 14, 2016
@crisbeto
Copy link
Member Author

crisbeto commented Apr 15, 2016

I managed to work around those height/padding issues. Also updated the PR to include a max-rows attribute.

@topherfangio
Copy link
Contributor

LGTM 👍

@topherfangio topherfangio added pr: merge ready This PR is ready for a caretaker to review and removed needs: review This PR is waiting on review from the team labels Apr 18, 2016
@ThomasBurleson ThomasBurleson added this to the 1.1.1 milestone Apr 20, 2016
@crisbeto crisbeto removed the in progress Mainly for in progress PRs, but may be used for issues that require multiple PRs label Apr 20, 2016
@ThomasBurleson ThomasBurleson modified the milestones: 1.1.1, Backlog Apr 21, 2016
@ThomasBurleson ThomasBurleson modified the milestones: Backlog, 1.1.1 Apr 21, 2016
@cmacdonnacha
Copy link

Just wondering if this will be merged for the next release?

@crisbeto crisbeto force-pushed the textarea branch 2 times, most recently from 1762ce2 to ac50ed4 Compare May 26, 2016 19:56
…he minimum, instead of the maximum, when auto-expanding the element.

* Adds a handle for vertically resizing the textarea element.
* Simplifies the logic for determining the textarea height.
* Makes the textarea sizing more accurate by using scrollHeight directly, instead of depending on the line height and amount of rows.
* Avoids potential issues where the textarea wouldn't resize when adding a newline.
* Adds the option to specify a maximum number of rows for a textarea via the `max-rows` attribute.

Fixes angular#7649.
Fixes angular#5919.
Fixes angular#8135.

BREAKING CHANGE
This changes the behavior from considering the "rows" attribute as the maximum to considering it as the minimum.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
- Breaking Change pr: merge ready This PR is ready for a caretaker to review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants