search

francoischalifour / medium-zoom

🔎🖼 A JavaScript library for zooming images like Medium
https://medium-zoom.francoischalifour.com
MIT License
3.62k stars 165 forks source link

Not vertically centered properly with <!DOCTYPE html> set #4

Closed karimhossenbux closed 7 years ago

karimhossenbux commented 7 years ago

Found a bug when having doctype declared.

I was losing my mind to find why it wasn't working!

When the doctype is set, document.body.clientHeight returns a bigger value than expected, the same height as the body instead of the height of the actual viewport.

Maybe the solution here is to replace document.body.clientHeightwith document.documentElement.clientHeight ?

francoischalifour commented 7 years ago

Thanks for opening the issue.

Since, as you pointed out, document.body.clientWidth doesn't work as expected when the doctype is declared, a simpler solution would be to directly replace document.body.clientHeight in favor of window.innerHeight (document.documentElement.clientHeight doesn't work when the doctype is not declared):

-const windowWidth = document.body.clientWidth || window.innerWidth
-const windowHeight = document.body.clientHeight || window.innerHeight
+const windowWidth = window.innerWidth
+const windowHeight = window.innerHeight

This way, it would work with or without the doctype declared.

What do you think?

karimhossenbux commented 7 years ago

Yes sure, if that works in both case, that's better!