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

Cannot set maxHeight using vH #930

Closed
lukesmurray opened this issue Mar 18, 2021 · 4 comments
Closed

Cannot set maxHeight using vH #930

lukesmurray opened this issue Mar 18, 2021 · 4 comments

Comments

@lukesmurray
Copy link
Contributor

Describe the bug
Cannot set maxHeight option to vH

To Reproduce

Set the maxHeight option to 100vh. When you look at the iframe you'll see the style tag for the iframe does not have a maxHeight set.

Expected behavior
I would expect to be able to set the iframe height to units other than px. vH is incredibly useful since it is automatically responsive if the user changes the screen size.

Additional context

      function addStyle(style) {
        if (
          Infinity !== settings[iframeId][style] &&
          0 !== settings[iframeId][style]
        ) {
          iframe.style[style] = settings[iframeId][style] + 'px'
          log(
            iframeId,
            'Set ' + style + ' = ' + settings[iframeId][style] + 'px'
          )
        }
      }

I believe the issue is due to the addStyle function add a px suffix to the iframe styles.
Not sure how to fix while retaining backwards compatibility.

Obviously these same issues would apply to vw as well if my reasoning is correct.

@davidjbradshaw
Copy link
Owner

I guess we should do isNumber() and only add px if true.

Would welcome a PR

@lukesmurray
Copy link
Contributor Author

That sounds reasonable and was similar to what I was thinking. I'll use lodash to check for isNumber since that's already a dependency of the lib.

@davidjbradshaw
Copy link
Owner

Can you just cut and paste the function, the built code has no dependencies in it or we can just write it

function isNumber(value) {
  return typeOf value === ‘Number’
}

@davidjbradshaw
Copy link
Owner

Fixed in v4.3.2.

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

2 participants