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

Iframe resize message handling #2311

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ascholerChemeketa
Copy link
Contributor

This updates lti.frameResize to work for all iframes, not just My Open Math

@bnmnetp - It includes your Cay Horstmann sample with you as the author of the commit.

There are artifacts created in the Horstmann after it resizes - scrollbars appear that then go away after you click on the iframe. I did lots of testing, this appears to be an issue on their side with rendering into the new area. Anything that I could figure out to do on our end to try to fix it was insanely hacky (first resize with 30 extra pixels of width, then a few ms later rerender at correct width). In the simple iframe test I added to the same section there are no lingering scrollbars. So for now, no hackery.

@drlippman - this builds on your MoM code. I didn't really change it and it all still works with samples in Sample Article. I was wondering why you change the style.width and style.height of the iframe instead of the iframe's width and height attributes. If there is some magic there I am missing, I would love to know about it.

@drlippman
Copy link
Contributor

No magic. My understanding is that it's just a matter of personal preference, with some people preferring to keep all styling in CSS. I probably got that code from somewhere else originally. Feel free to change it if it's more consistent with other practices in pretext.

@ascholerChemeketa
Copy link
Contributor Author

@drlippman Thanks!

@bnmnetp
Copy link
Contributor

bnmnetp commented Dec 11, 2024

@ascholerChemeketa - so it looks like this will end up in BOOK/_static/pretext/js/lti_iframe_resizer.js. -- which means that I should include it on assignment pages to ensure that problems resize in that arena as well.

@ascholerChemeketa
Copy link
Contributor Author

It could be moved somewhere else, but yes, that is the current location.

Eventually, it would be nice to get all of the PreTeXt related JS bundled up better. Perhaps into two piles - the stuff needed by RS assignment pages (knwols, this, etc...) and the stuff that isn't (toc, permalinks, etc...).

I suppose you could bake in a copy of the logic to the server-generated pages instead of doing that.

@bnmnetp
Copy link
Contributor

bnmnetp commented Dec 11, 2024

No, I already load things from .../BOOK/_static/ so its not a stretch to add it.

I agree that a bundle of PreTeXt stuff needed by the RS assignment pages would be great.

I don't think baking it in to the pages is a great idea.

@rbeezer
Copy link
Collaborator

rbeezer commented Dec 11, 2024

Do we know if this interacts with fram resizing for WeBWorK? There is an example in the distribution, need to run makefile for problems processed by server then make HTML. I think this is a good test example:

https://pretextbook.org/examples/webwork/sample-chapter/html/section-the-quadratic-formula.html

Checkpoint 1.2.2
Do part (a) correctly and watch part (b) reveal itself and resize.

@bnmnetp
Copy link
Contributor

bnmnetp commented Dec 11, 2024

I don't know, but we should strongly encourage WeBWorK to adopt this protocol if they don't, as the lti_reseze message is already supported by other LMS.

@Alex-Jordan
Copy link
Contributor

If someone is able to explain to me what this is about, perhaps I can comment on this and WeBWorK.

Status quo. A WeBWorK server hosts a build of this project: https://github.com/davidjbradshaw/iframe-resizer. There are certain features that we like and/or are necessary. One of those is that you can tag certain elements to be monitored for where their bottom is, and make sure to include those in the iframe height calculation. Even when they are things that would normally be OK to extend below out of the page. This comes up with the feedback buttons, which in 2.19 can produce a tall popover that can dip below the iframe edge without having tagged that popever's div as one of these things to monitor.

With PTX, the web page with the embedded exercise loads iframeResizer.min.js from where the WW server is serving it. The page within the iframe also loads iframeResizer.contentWindow.min.js from the same place. These two things go together.

Feel free to take discussion to the forum if it's not right to continue about this here.

@bnmnetp
Copy link
Contributor

bnmnetp commented Dec 11, 2024

OK, it sounds like this isn't something we need to worry about for WeBWorK, although both are doing similar things. Using the postMessage interface to communicate from child to parent and vice versa.

I haven't seen the code that @ascholerChemeketa is working on, but this seems a lot more complex and more likely fully featured that what the SPLICE project has provided.

@ascholerChemeketa
Copy link
Contributor Author

Tested against the webwork sample - it works correctly.

Yes, @bnmnetp is right. The iframeResizer library is a much more capable version of what the SPLICE protocol provides (which is in turn borrowed from the lti protocol that people like Canvas all agree on.)

So we have two different protocols running to resize elements in an iframe. They should not interact unless a page included as an iframe was trying to use the iframeResizer library AND emit lti.frameResize messages. In a perfect world, we would not need both. But for now I think we wo.

@ascholerChemeketa ascholerChemeketa marked this pull request as ready for review December 11, 2024 23:23
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

Successfully merging this pull request may close these issues.

5 participants