bgrins / ExpandingTextareas

jQuery plugin for elegant expanding textareas
http://bgrins.github.com/ExpandingTextareas/
MIT License
260 stars 73 forks source link

Copy minHeight to mirror, add an optional throttle/delay to the update binding. #53

Closed vincekd closed 9 years ago

domchristie commented 9 years ago

Is it necessary to copy the minHeight property to the mirror?

(The minHeight value is taken from the initial height of the target textarea)

vincekd commented 9 years ago

I'm using another library in conjunction with angular that dynamically sets the height of vertically stacked elements, which was causing issues with the mirror. Might be a rare use case but it doesn't hurt anything to include it, right?

On Wed, Dec 10, 2014, 6:21 PM Dom Christie notifications@github.com wrote:

Is it necessary to copy the minHeight property to the mirror?

(The minHeight value is taken from the initial height of the target textarea)

— Reply to this email directly or view it on GitHub https://github.com/bgrins/ExpandingTextareas/pull/53#issuecomment-66543442 .

domchristie commented 9 years ago

it doesn't hurt anything to include it, right?

It may do. The clone’s min-height is set by getting the target textarea’s height: https://github.com/bgrins/ExpandingTextareas/blob/98a489fbfb6c975ec3fc8705d311504e46a8fa47/expanding.js#L126

And this change would overwrite that.

vincekd commented 9 years ago

Oughtn't the textarea respect the minHeight property regardless? For instance, if the textarea is pre-filled with content, it should be able to shrink below that if the user removes or alters the text. Of course, you could set the height property in the css and then let the plugin expand it, but isn't that more counter-intuitive?

On Thu, Dec 11, 2014 at 5:08 AM, Dom Christie notifications@github.com wrote:

it doesn't hurt anything to include it, right?

It may do. The clone’s min-height is set by getting the target textarea’s height: https://github.com/bgrins/ExpandingTextareas/blob/98a489fbfb6c975ec3fc8705d311504e46a8fa47/expanding.js#L126

And this change would overwrite that.

— Reply to this email directly or view it on GitHub https://github.com/bgrins/ExpandingTextareas/pull/53#issuecomment-66597394 .

domchristie commented 9 years ago

Oughtn't the textarea respect the minHeight property regardless?

It does :) When the page loads (and before the plug in is initialised) the textarea’s height will be based on the relevant CSS, as well as the rows attribute. The plugin grabs the textarea’s outer height at this point. This ensures that the clone’s min height is as accurate as possible.

For example, if the rows=100, but CSS of min-height: 1px, you’d want the clone’s min-height to obey the rows attribute, not the CSS. (A bit of a contrived example, I know!) It is for this reason that the clone’s min-height is based on the actual height value of the textarea on load.

Have a play: http://jsbin.com/tabocijoso/1/edit?html,css,output

vincekd commented 9 years ago

OK, that makes sense. Thanks.

On Thu, Dec 11, 2014 at 4:33 PM, Dom Christie notifications@github.com wrote:

Oughtn't the textarea respect the minHeight property regardless?

It does :) When the page loads (and before the plug in is initialised) the textarea’s height will be based on the relevant CSS, as well as the rows attribute. The plugin grabs the textarea’s outer height at this point. This ensures that the clone’s min height is as accurate as possible.

For example, if the rows=100, but CSS of min-height: 1px, you’d want the clone’s min-height to obey the rows attribute, not the CSS. (A bit of a contrived example, I know!) It is for this reason that the clone’s min-height is based on the actual height value of the textarea on load.

Have a play: http://jsbin.com/tabocijoso/1/edit?html,css,output

— Reply to this email directly or view it on GitHub https://github.com/bgrins/ExpandingTextareas/pull/53#issuecomment-66692763 .

bgrins commented 9 years ago

@vincekd @domchristie thanks for thinking through this, it does get a little tricky