Rounding up calculated height value to non-decimal number to prevent scrollbar from showing

[RenoLooijmans]

Describe the bug
When, for example, the height is calculated by using "heightCalculationMethod: 'lowestElement'", and because of css the calculated value is a decimal number (say 1436.3348343943), the iframe height seems to be rounded to 1436 instead of 1437. Because it is rounded down a scrollbar is shown inside the iframe (when the iframe height is 100%). This scrollbar scrolls 1px or something, which renders it completely useless and annoying (because the parent page cannot be scrolled).

To Reproduce
See above. I am unsure what css cause a non-rounded value but most likely this is line height of font size related.

Expected behavior
Rounding up the calculated height value so a scrollbar is not shown.

Desktop (please complete the following information):

• Version: 4.3.6

Smartphone (please complete the following information):

• Version: 4.3.6

Additional context
Possible fix.

Change file iframeResizer.contentWindow.js, line 980:

lowestElement: function getBestHeight() { return Math.max( getHeight.bodyOffset() || getHeight.documentElementOffset(), getMaxElement('bottom', getAllElements()) ) },

to

lowestElement: function getBestHeight() { return Math.ceil(Math.max( getHeight.bodyOffset() || getHeight.documentElementOffset(), getMaxElement('bottom', getAllElements()) )) },

Please note the added Math.ceil()-function.
Perhaps this should also be applied to the other results, and instead fix it on line 1053:

currentHeight = undefined === customHeight ? getHeight[heightCalcMode]() : customHeight

to

currentHeight = undefined === customHeight ? Math.ceil(getHeight[heightCalcMode]()) : customHeight

I'm happy to provide additional details.

[davidjbradshaw]

This will be fix in The upcoming V5. Would love some feedback if you want to give it a go. #1212

[jlarmstrongiv]

@davidjbradshaw if you have an npm alpha version published for this package and iframe-resizer-react, I would be glad to give v5 a test run 😄

[RenoLooijmans]

Thanks for your quick reply! I've just seen the implemented fix for V5. Looks like it's better than my suggestion - so no additional feedback there. Any timeframe when V5 is being officially released or "it's ready when it's ready"?

[davidjbradshaw]

@RenoLooijmans sorry for slow response, have been unwell for the last few days.

I've just published the alpha of version 5 to NPM. It is now split into two different packages.

@iframe-resizer/parent
@iframe-resizer/child

in addition I have created jQuery and React plugins for the parent page as well. If you want to try either of these they can be used instead of the @iframe-resizer/parent

@iframe-resizer/react
@iframe-resizer/jquery

I'm still putting together the first draft of a new documentation website, but you can find details of all the changes here.
https://github.com/davidjbradshaw/iframe-resizer-docs/blob/master/src/content/docs/upgrade.md

A number of the config options have been revised, but if your not using any then the basic setup is unchanged.

Please let me know if you have any questions.

[RenoLooijmans]

@davidjbradshaw no worries there! I'm already happy it's being dealt with :-).

I'm away next week myself, so I'll definitely have a look at it when I am back.

Get well soon.

[davidjbradshaw]

Thanks, very rough docs now online at https://iframe-resizer.github.io/docs/

[jlarmstrongiv]

Wonderful! @davidjbradshaw one example that would great to add to the docs would be loading the @iframe-resizer/child script from a cdn. That’s how I currently load the script in the content window

[davidjbradshaw]

That's a fair point, currently I plan to add examples for CommonJS and ES Modules, so can do a CDN example as well.

[davidjbradshaw]

Just moved doc site to http://iframe-resizer.github.io/docs/

[davidjbradshaw]

@jlarmstrongiv added jsDeliver CDN

[jlarmstrongiv]

Hey @davidjbradshaw what’s this about

To use iframe-resizer in a closed source website or application, you need to purchase a one time license key. See the pricing page for details.

Where can I see the pricing? I saw ua-parser-js do the same thing recently. I’m building a small app (probably won’t make money, but you never know), so I guess I would need to look into a business license.

One thing I would like to see clarified is I’m planning to make webpages that can be embedded in other websites, so clarification that that use-case is included in the license is important to me. That would include, for example, me building a simple react library or wordpress plugin to help non-technical users with my webapp-specific embeds and options.

I can’t legally try out v5 until all of that clarified.

[davidjbradshaw]

Hey @jlarmstrongiv if you would like to help me test this and provide feedback, then I'll be happy to give you a free license, once we have a full release version.. For now you can use the key ALPHA-TEST with the fifth alpha build.

Still need to build the licensing bit out, but yes your use case will be covered, that was one of the original design goals of this project.

[jlarmstrongiv]

Still need to build the licensing bit out, but yes your use case will be covered, that was one of the original design goals of this project.

Thank you for clarifying @davidjbradshaw

Hey @jlarmstrongiv if you would like to help me test this and provide feedback, then I'll be happy to give you a free license, once we have a full release version.. For now you can use the key ALPHA-TEST with the fifth alpha build.

That sounds good to me! I’ll update the react and cdn versions and let you know how it goes

For now you can use the key ALPHA-TEST with the fifth alpha build

Just a couple followup questions about the key actually.

I assume the answer is no from the code I read in the v5-dev branch, but will the libraries phone home with the API Key? If so, I’d have privacy concerns for European users. I assume that it’s mainly included to highlight the license requirements.

I assume from the usage examples that the API key does not need to be kept secret?

[davidjbradshaw]

This is going to be pretty simple and I couldn't afford the server infrastructure to have it phone home. The current version is used on a surprisingly large amount of the world's top websites.

[jlarmstrongiv]

The current version is used on a surprisingly large amount of the world's top websites.

I sure hope they will pay to help maintain it then! I’m curious how your pricing experiment with iframe-resizer and how Faisal Salman’s experiment with ua-parser-js will go

---

In the meantime, I received this error trying to use @iframe-resizer/react:

File '/Users/user/Desktop/projects/project/node_modules/@iframe-resizer/react/iframe-resizer.react.d.ts' is not a module.ts(2306)

I did see that the types were included in the node_modules as the docs said, though they seem to be structured in a way that ts doesn’t like.

[davidjbradshaw]

Yeah those types were my best guess, I'm not much of a TS coder. Any chance you could have a quick look and make a PR on the v5-dev branch?

[davidjbradshaw]

I had a chat with the guy behind fullpage.js, who was able to turn that into a full time job. But I expect his market is mainly WordPress templates.

[jlarmstrongiv]

Yeah those types were my best guess, I'm not much of a TS coder. Any chance you could have a quick look and make a PR on the v5-dev branch?

I took a stab at it. These days, I code 95% of my projects in TypeScript and ESM so the PR reflects that. I wasn’t able to get it to play nicely with the module or namespace syntax.

---

I had a chat with the guy behind fullpage.js, who was able to turn that into a full time job. But I expect his market is mainly WordPress templates.

I can see that. I like that he still offers one-time payments. Everything turning to subscriptions these days is rather painful.

[davidjbradshaw]

Thanks for the PR, I left a comment on it, looking at the changes I spotted an issue in the old version that might have been the issue. I had not changed the module name from iframe-resizer-react to @iframe-resizer/react. I think it was just lifted from the old module, with the deprecated options removed.

I still need to build a test page in the example folder for react and write some tests for it. I'm glad to hear it worked for you as I have literally never even ran it yet! It was there to test the build and deploy scripts.

---

The full page guy said the subscription option was pretty much unused. I don't think it makes a lot of sense for things like this.

[jlarmstrongiv]

Thanks for the PR, I left a comment on it, looking at the changes I spotted an issue in the old version that might have been the issue. I had not changed the module name from iframe-resizer-react to @iframe-resizer/react. I think it was just lifted from the old module, with the deprecated options removed.

Oh! 😂 Why didn’t I see that? Must not have been awake yet first thing this morning when I looked at it. You’re right—updating the module name @iframe-resizer/react fixes it (I tested it locally).

I still need to build a test page in the example folder for react and write some tests for it. I'm glad to hear it worked for you as I have literally never even ran it yet! It was there to test the build and deploy scripts.

I wouldn’t go that far 😅 I’m more of a one-error-at-a-time kind of guy. Once the types are fixed, I’ll move onto the next error (or hopefully it works)

---

The full page guy said the subscription option was pretty much unused. I don't think it makes a lot of sense for things like this.

I agree, nice to hear a real-world anecdote

[davidjbradshaw]

@jlarmstrongiv Hopefully this should now work in Alpha.8 release

[jlarmstrongiv]

@davidjbradshaw I tried out the alpha release:

I’m getting the TypeScript error Property 'license' does not exist on IframeResizer

In the console, I’m getting:

Warning: Failed prop type: The prop `forwardRef` is marked as required in `l2`, but its value is `undefined`.

As well as

[iframeResizer][Host page: null] Ignored: [object Object]

Apart from the other miscellaneous logs, it seems to work so long as I pass it the license key (fails without it)

[davidjbradshaw]

@jlarmstrongiv

Good spot on license, added to types for both Parent and React. These two type files are really very different and both came from PRs by different people. I don't really understand the difference. My guess is the one on Parent should be more like the one on react.

The strange ignore message was due to it seeing something come in via postMessage that it was not expecting. I've change this so it will display objects, as we non longer have to worry about IE not supporting this and lowered the level so it will only show in dev builds.

The above release as Alpha.9

The forwardRef one is down to it being marked as required via the PropTypes library. However, I'm not really sure why it is even there. I expect it comes from a PR at some point in the past. It's been a few years since I did a lot of React and I'm bit unsure why it is there. Do you have any ideas?

Will dig in more when I get time to go through this backlog of issues
https://github.com/davidjbradshaw/iframe-resizer-react/issues

[davidjbradshaw]

Figured out the PropType thing, seems that was down to me following esLint's incorrect advice.... I've removed it and also added an old PR that I had missed from iframe-resizer-react to add a getIframeElement()` method.

[jlarmstrongiv]

@davidjbradshaw here’s an example on how react-aria implemented an optional useRef adobe/react-spectrum#834, which includes several example implementations. The idea is being able to pass an existing ref if it’s given, or create one if not.

[jlarmstrongiv]

@davidjbradshaw incoming warning on the previous major version of iframe-resizer-react:

Warning: IframeResizer2: Support for defaultProps will be removed from function components in a future major release. Use JavaScript default parameters instead.

With the recently released react-dom@18.3.0 (released 6 hours ago), I checked out the v5-dev branch and it may still be used with prop-types?

[davidjbradshaw]

I just saw that yesterday, I’ve updated it to remove DefaultProps and I guess PropTypes could go as well as it is not really being used anyway.