Skip to content
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

When there is only one line, why the height is not my css height? #336

Closed
x1nGoo opened this issue Jul 11, 2017 · 22 comments
Closed

When there is only one line, why the height is not my css height? #336

x1nGoo opened this issue Jul 11, 2017 · 22 comments

Comments

@x1nGoo
Copy link

x1nGoo commented Jul 11, 2017

When there is only one line, why the height is not my css height?

@jackmoore
Copy link
Owner

CSS height is not factored in (autosize sets the CSS height itself), the size will be dependent on the min-height, max-height, and rows attribute. So instead of (or in addition to) your height value, add an equivalent min-height value.

@x1nGoo
Copy link
Author

x1nGoo commented Jul 11, 2017

oh. I have set a min-height, but if I add a padding-top, the one line height is also not the min-height;
then how to set the word in the middle(vertical) when there is one line.

@bdteo
Copy link

bdteo commented Jul 12, 2017

I have the exactly same problem. The proposed workaround does not work,

@bdteo
Copy link

bdteo commented Jul 12, 2017

There is some bug actually I found. It adds 1 em more at init no matter what. I read through the code and It's not probably something complicated but I will search for another component now.

@jackmoore
Copy link
Owner

Sorry, I gave all the information I have based on the provided info. Post a demo and I'll have a look.

@jackmoore
Copy link
Owner

@x1nGoo I don't follow what you are saying. What does vertical alignment have to do with this project?

@bdteo
Copy link

bdteo commented Jul 12, 2017

Jack, I implemented simpler version of it for my particular case. It goes like that

function autoheightTxtMsgInput(ev) {
  var $ = jQuery;
  var e = $(this);  
      
  e.css('height', '');

  var contentBoxMode = true,
      offset = 0;

  if (contentBoxMode) {
    offset = e.outerHeight(true) - e.innerHeight();
  }

  e.css('height', offset + e[0].scrollHeight + 'px');  
}

Setting height to auto at the beginning results in the same behaviour as I just found out.

It would take me too much time rn to give you a fiddle with my exact code.

I think however the assumption that the default height of the element is "auto" is completely wrong.
For example i could have defined .my-textarea {height: 11px} and your code will fail in this case.

@jackmoore
Copy link
Owner

Autosize does not account for your CSS height, it matches the height of your content (that's it's purpose). If you need a minimum, use the rows attribute or min-height style.

@bdteo
Copy link

bdteo commented Jul 12, 2017

Yeah, try it. It has nothing to do with min-height and you override the real height that way. Good night from me ! It's that simple but it has really nothing to do with min-height. (the problem is not that the element collapses too much but that it expands too much needlessly). Even if you used min-height in your code to hack around this in some way it would be super funky.

Please do some experiments and take it into consideration so other people could use your component without hassle.

@bdteo
Copy link

bdteo commented Jul 12, 2017

I hope you understood me well. height = 'auto' is just completely wrong assuption. It should be at least height = ''. It should account for the default height since textareas could be too high by default and this is unwanted behaviour ... having 60px textarea for single line ,.. and min-height does not affect that. At least in Chrome.

@jackmoore
Copy link
Owner

Again, you should be providing a demonstration of your problem because what you are saying doesn't mean anything to me. You are probably getting a height that is too high due to the innate rows attribute value on your textarea element. That has nothing to do with CSS height, auto or otherwise. That is covered in the documentation. Autosize uses getComputedStyle to find the height of the textarea element, so it only knows what the browser tells it about what the height actually is. It doesn't know what styles are in your stylesheet or what number of rows the textarea is supposed to have.

It should account for the default height since textareas could be too high by default and this is unwanted behaviour

Default is too high by default? What?

@jackmoore
Copy link
Owner

On your textarea element, try settings rows='1', as the default value is 2. This can lead it the textarea element being higher than expected.

@jackmoore
Copy link
Owner

I'm assuming you are using the current version of Autosize, and not the jQuery plugin from 2 years ago. It worked in a completely different manner, and there was an append option that added an extra line as a buffer to smooth out CSS transition animations in some browsers.

@bdteo
Copy link

bdteo commented Jul 12, 2017

Jack, I don't have anything to prove here. I just wrote to help you. I am really tired and I don't have time to dispute rn. If this is vague to you ... I am referring to the desired height and the default height. By "default" I mean not only browser defaults but defaults in css frameworks and so on ...

What you've just suggested might help actually (using height = auto to measure the scroll height). I hope the op gives some feedback too If it is impossible for you to reproduce it right now. I think however I was super detailed in my explanation.

To be more detailed ... here is what happens specificly in my case. I will depict it with concrete numbers. Giving height: auto (it is the highest priority css rule, I checked) results in my textarea extending to 50px of height. You use that to measure the scroll height. The actual scroll height is not 50px though. It's 20px. Since I have sth like that .my-textarea { height: 20px; min-height: 20px; max-height: none; } the height is 20px, not 50px. You measure 50px. That's all.

I don't know if you still support and update this project but if you do I recommend you to reconsider your approach according to scenarios like this.

Cheers !

I am unsubscribing from this thread. I have nothing to do here as I said you. Just tried to help you and the users of your repo.

@bdteo
Copy link

bdteo commented Jul 12, 2017

Yes, I used the latest version. I did not bother to install it trough npm or clone the repo. I just downloaded the dist/*.js directly ... so I am sure.

@jackmoore
Copy link
Owner

@bdteo My apologies, understand what you are getting at now. You are right about 'auto' causing it to be too big if you set a style height that lowers the height below what it would normally have been, and that replacing it with '' would be better.

@bdteo
Copy link

bdteo commented Jul 12, 2017

I am glad I helped. I am a big supporter of the open source idea and I think eveyone should contribute. When I comment on some issue on github people usually see that I have no repos on my account here and don't take me seriously. Too bad, actually, cause I've found tons of bugs. :D Most people here actually don't even read through their issues. In the best case scenario usually they say "make a pull request" and they never merge it. My apologies, too if I sounded rude. That's the reason I become angry sometimes. 👍

@bdteo
Copy link

bdteo commented Jul 12, 2017

Thanks for your time !

@x1nGoo
Copy link
Author

x1nGoo commented Jul 13, 2017

@jackmoore sorry, my english is not good..
I have updated now, It has been solved. Thanks :D

@KNedelec
Copy link

KNedelec commented Jul 13, 2017

My solution is to set rows="1" so that the browser compute a correct "auto" height and you don't have the micro lag.
Anyway, thank you for your great work

@bdteo
Copy link

bdteo commented Jul 13, 2017

Nedelec, it works partnered with some non-inline styling, e.g .my-text { height: 10px; line-height: 8px; box-sizing: border-box; padding: 6px; ... } is defined. It won't work for inline defined height of course. Attributes like cols and rows should be deprecated anyways if they are not already.

Let's thank Jack for the good support anyway !

@dobriai
Copy link

dobriai commented Jul 18, 2017

Thanks to @x1nGoo, @bdteo and @jackmoore for resolving this! I had been meaning to raise the issue, but was too slow to act :-) Nice touch!

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

No branches or pull requests

5 participants