Stretchy HTML5 iframe height

[kollivier]

Summary

For some HTML5 content, there is enough height to render the content but the hardcoded size causes the content to be cut off and thus necessitates vertical scrollbars in the HTML5 content itself. This PR makes it so that the content can expand up to 70% of the page's virtual height, while retaining the old hardcoded height as the min-height to minimize issues with existing content.

Reviewer guidance

ProFuturo content in the Science section is a good test of what this fixes, but testing with a variety of HTML5 content is ideal. We may want this to have another more extensive HTML5 QA pass before merging into develop.

References

…

---

Contributor Checklist

PR process:

• PR has the correct target branch and milestone
• PR has 'needs review' or 'work-in-progress' label
• If PR is ready for review, a reviewer has been added. (Don't use 'Assignees')
• If this is an important user-facing change, PR or related issue has a 'changelog' label
• If this includes an internal dependency change, a link to the diff is provided

Testing:

• Contributor has fully tested the PR manually
• If there are any front-end changes, before/after screenshots are included
• Critical user journeys are covered by Gherkin stories
• Critical and brittle code paths are covered by unit tests

Reviewer Checklist

• Automated test coverage is satisfactory
• PR is fully functional
• PR has been tested for accessibility regressions
• External dependency files were updated if necessary (yarn and pip)
• Documentation is updated
• Contributor is in AUTHORS.md

[indirectlylit]

will test soon - just a quick review of the code itself

[indirectlylit]

vh has mediocre support on older iOS and Android devices.

We can do this, but if there is a way to avoid vh that would be preferable.

[kollivier]

Any ideas on alternatives? Doing percentage would make it relative to parent element, rather than expanding the height based on available screen space, wouldn't it?

[indirectlylit]

I'll take a look when I actually test it locally

[indirectlylit]

is this file packaged with Webpack, or is it unprocessed because it gets embedded in Hashi?

If the former, you can use an arrow function here:

Suggested change

	let self = this;
	window.addEventListener('DOMContentLoaded', function() {
	const data = {
	offsetHeight: window.document.body.offsetHeight,
	};
	const event = events.CONTENTLOADED;
	self.mediator.sendMessage({ nameSpace, event: event, data: data });
	});
	window.addEventListener('DOMContentLoaded', () => {
	const data = {
	offsetHeight: window.document.body.offsetHeight,
	};
	const event = events.CONTENTLOADED;
	this.mediator.sendMessage({ nameSpace, event: event, data: data });
	});

[kollivier]

It's packaged, I'll make the change! Didn't realize this got around the self = this problem.

[indirectlylit]

Yup - super convenient!

ref: https://hackernoon.com/javascript-es6-arrow-functions-and-lexical-this-f2a3e2a5e8c4

[indirectlylit]

let self = this; not necessary if you use an arrow function:

Suggested change

	let self = this;
	this.hashi.on(events.CONTENTLOADED, function(data) {
	this.hashi.on(events.CONTENTLOADED, (data) => {

[indirectlylit]

rather than using nested $refs and modifying .style.height, it would be a bit more conventional to add a prop to CoreFullscreen and pass in a height if possible.

For example:

<template>
  <div
    ref="fullscreen"
    :class="fullscreenClass"
    :style="{ height: `${height} px` }"
    allowfullscreen
  >
    <slot></slot>
  </div>
</template>

<script>
  // ...
    props: {
      height: { type: Number, required: false },
    }
   // ...

[indirectlylit]

I've been experimenting with this a bit on different content and seeing some odd results.

In ProFuturo (English) / Level Up / Motivation the height increases, but the width also does which causes horizontal scrolling:

For other content such as PraDigi and HP Life, the iframe doesn't resize at all, presumably because the content is rendered after the DOMContentLoaded event.

My recommendation would be to something like:

• update the iFrame client to poll maybe once per second for content size changes
• send content size change events through Hashi when the size changes
• if necessary, incorporate both width and height information to avoid horizontal scrolling
• with regard to the max-height: 70vh; question, perhaps we don't need a max height at all? It might be better to never scroll?

[kollivier]

There are too many cases to handle fully dynamic sizing, I think. There are a number of different layout strategies a page may use:

1. Fixed height
2. Scale by aspect ratio
3. Fixed min-height but scales by aspect ratio
4. Small / med / large canvas sizes, chosen based on container height
5. Some completely custom sizing logic written in code

The problem is that the only way to know for sure which it is would be to check the code.

Polling and resizing every second will lead to recursion for cases 2 and sometimes 3 and 5, because we have no idea how the content responds to size changes.

The Level Up content is actually a good case in point of that. For some reason, enlarging the canvas by 100-200 pixels causes it to zoom the content considerably. I'll see if there's a way to address that. Setting width instead of height could be an option, but I also want to make sure we don't end up shrinking the content too much.

I think ultimately what we want is to have the ability to specify a desired canvas size for the HTML5 app, and maybe have some auto-detection logic in ricecooker and Studio that can offer suggestions based on the content, but I believe that would require us to finish up the "extra_fields" support on ContentNodes?

[indirectlylit]

Maybe we could define a 'size' setting on the content node, which can be set by a sushi chef or, longer-term, in the studio UI? For example, it could be set to one of:

• fixed height (in px)
• fixed aspect ratio (fraction or float)
• dynamically resize height

In all cases, it would expand to 100% of the available width

[rtibbles]

To add my two cents - I think @kollivier and @indirectlylit are both right here. We should add a size attribute within extra_fields. As @kollivier notes, we have not implemented extra_fields on ContentNodes yet, so we would need to do that.

[indirectlylit]

closing as superseded by #6873