Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

now using clientHeight to detect when to show/hide scrollbar #327

Closed
wants to merge 1 commit into from

Conversation

srobilliardfln
Copy link

@srobilliardfln srobilliardfln commented Jan 31, 2017

This fixes an issue in IE 11 where the scrollbar is always showing, even when the height of the text area is less than the max height.
More specifically it seems that in IE11, the computed height from window.getComputedStyle is less than the actual height of the element. clientHeight appears to be a more reliable source for determining the height of the DOM element. This fixes issue #226

@srobilliardfln
Copy link
Author

Hi there,
hoping someone may be able to review my pull request? It's been a few months now.

@jackmoore
Copy link
Owner

@srobilliardfln Thanks for the pull request, my apologies for the late response. At first glance this does not work as you cannot substitute style height for clientHeight, as clientHeight includes padding. Your fix probably only worked for your specific CSS and would not be appropriate to merge into master.

I tested anyway and was not able to verify that it fixed anything. I still get an IE11 scrollbar on the same instances as prior to incorporating your change. But I was able to see that there is an issue and agree that it should be fixed.

@srobilliardfln
Copy link
Author

Thanks for reviewing @jackmoore 👍

@jackmoore
Copy link
Owner

Looks like this is a bug with how IE11 handles window.getComputedStyle; it does not account for changes to the box-sizing property. I'm assuming you set your textarea box-sizing to border-box. The default box-sizing would not be hit by this and hides scrollbars as expected.

@srobilliardfln
Copy link
Author

Ahhh yes you are right. I will see if I can get back on master by removing that style.

@jackmoore
Copy link
Owner

See my referenced pull request for a potential fix. It uses offsetHeight as a replacement for the computed style height for a border-box element. This needs more testing because I'm unsure of the browser support range for offsetHeight. Quick test showed it working in modern browsers.

@jackmoore
Copy link
Owner

It would probably be better form to add up the individual computed styles (height, paddingTop/Bottom, borderTopWidth/BottomWidth) rather than relying on offsetHeight, since it's just Working Draft. It would require adding in a test to see if the computed style for height is correct or not to determine if those additional styles should be added to the computed height or not.

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

Successfully merging this pull request may close these issues.

2 participants