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

Wrong height calculation #6217

Closed
SteffenMangold opened this issue Jan 8, 2017 · 4 comments
Closed

Wrong height calculation #6217

SteffenMangold opened this issue Jan 8, 2017 · 4 comments

Comments

@SteffenMangold
Copy link

@SteffenMangold SteffenMangold commented Jan 8, 2017

Expected behaviour

Container defined height should be respected by highchart

Actual behaviour

Container height less then 20px only works when chart height is set in options. Without chart high is set to default.

Live demo with steps to reproduce

http://jsfiddle.net/kakvbfns/2/

Test 1: Remove 'height: 16,' from options
Test 2: Set container height to 20px or more

Affected browser(s)

All

@TorsteinHonsi
Copy link
Collaborator

@TorsteinHonsi TorsteinHonsi commented Jan 9, 2017

There's a reason for it - see https://github.com/highcharts/highcharts/blob/v5.0.6/js/parts/Chart.js#L590. However it can be discussed whether we should respect an IE7 bug in 2017...

@rudovjan
Copy link

@rudovjan rudovjan commented Jan 9, 2017

Sorry I did PR faster than I should :-)

@rudovjan
Copy link

@rudovjan rudovjan commented Jan 9, 2017

This patch should solve that issue.

@TorsteinHonsi
Copy link
Collaborator

@TorsteinHonsi TorsteinHonsi commented Jan 10, 2017

Actually I can't replicate the scenario in IE7 where this happened. I've tried the following script with different doctypes and it always alerts 0:

<html>
	<head>
	</head>
	<body>
		<div id="container">
			<!-- comment -->
		</div>

		<script>
		alert (document.getElementById('container').offsetHeight);
		</script>
	</body>
</html>

So I think it is safe to assume that this only happened in the environment that Highcharts was first developed, and probably as a consequence of some CSS setting there. I'm publishing a commit where the 19px threshold is removed.

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

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.