bgrins / ExpandingTextareas

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

Calling $elem.expandingTextarea() before $elem has been inserted into the DOM doesn't behave correctly #29

Closed iamdanfox closed 10 years ago

iamdanfox commented 11 years ago

The invisible element adjusts its size correctly, but the textarea doesn't expand.

bgrins commented 10 years ago

Sorry for missing this originally. This seems to be fixed now. Tested with: http://jsfiddle.net/bgrins/Xd5WB/1/

$("<textarea />").expanding().parent().appendTo("body");
domchristie commented 10 years ago

I’m not sure if this is quite working as expected. (Test by adding a few line-break: the textarea expands beyond what it should do.)

The clone’s styles rely on the computed textarea styles. These will differ when inserted into the DOM.

Adding an expanding textarea requires knowledge of the resultant markup (i.e. having to know to insert the textarea’s parent node), so isn’t ideal.

We could check if the DOM does not contain the textarea, and reinitialise when it’s inserted (using: http://www.backalleycoder.com/2012/04/25/i-want-a-damnodeinserted/ ?)

iamdanfox commented 10 years ago

Agreed, it's not quite perfect yet. JSFiddle demo of incorrect clone styles: http://jsfiddle.net/Xd5WB/2/

As a temporary solution, perhaps we should write a short demo of 'best-practise' DOM insertion, including any necessary hacks: see my 2 minute attempt: http://jsfiddle.net/Xd5WB/3/. That way, developers can at least get the desired behaviour easily.

Obviously, it would be nicer if this didn't require a style-syncing hack, but that's another issue.

bgrins commented 10 years ago

What if we exposed the _copyTextareaStylesToClone function to content with elem.expanding("refresh") or similar (maybe "cloneStyles" instead)? Then we could add this case to the documentation.

I don't think we need to detect the insertion (though this CSS frames hack is quite clever), if we just document how to deal with this case. Ideally you just wait to call expanding until after it is inserted into the DOM. If for some reason you don't, you could call this helper function we provide that puts everything in sync.

iamdanfox commented 10 years ago

The "cloneStyles" idea sounds good to me!

On Mon, Feb 24, 2014 at 5:30 PM, Brian Grinstead notifications@github.com wrote:

Reopened #29.

Reply to this email directly or view it on GitHub: https://github.com/bgrins/ExpandingTextareas/issues/29

domchristie commented 10 years ago

What about if we only initialise the plugin if the given textarea is in the DOM? That way there’d be no need to expose another method, users would just have to call .expanding() when it’s inserted.

bgrins commented 10 years ago

@domchristie that seems fine with me, since users could always just call expanding() again. And since we have the wrap call in the constructor, it really doesn't make sense to proceeed if there is no parent. We could probably just check $textarea.parent().length and bail out if 0.

And I agree with @iamdanfox that we should come up with some documentation about this. Maybe just a section on the home page on how to deal with with non-inserted DOM nodes?

domchristie commented 10 years ago

I’ve made a start here: https://github.com/domchristie/ExpandingTextareas/commits/outside-dom

Would it be sensible to add a console warning (or similar) if a user attempts to call expanding() on a textarea outside the DOM?

domchristie commented 10 years ago

I also went with checking presence of the element in document.body, rather than checking the parent node, because there may be additional manipulations (outside the DOM) that cause the textarea to have a parent node.

bgrins commented 10 years ago

This is looking pretty good. And yeah, it shouldn't hurt to add in something like:

if (window.console && console.warn) {
  console.warn("ExpandingTextareas: attempt to initialize textarea not inside of body.  Call expanding() again once it has been inserted into the page"); 
}