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

Use jotform's iframe embed code #28087

Merged
merged 5 commits into from Apr 19, 2019
Merged

Conversation

clareconstantine
Copy link

@clareconstantine clareconstantine commented Apr 18, 2019

PLC-223

Based on JotForm's advice in this thread: https://www.jotform.com/answers/1786265-Embedded-form-load-failure-spiked-on-Saturday

Screen Shot 2019-04-17 at 5 59 58 PM

Hopefully this reduces some of the mysterious issues with surveys not loading. I tested this manually and was able to submit forms, including a workshop daily survey, facilitator survey, and post-workshop survey.

cc: @agealy

@breville
Copy link
Member

Does the monitoring added in #27764 still work?

// The setHeight message seems to be sent every time we load the form, so use this
// to fire the hook we are monitoring to indicate the iframe content has loaded.
// eslint-disable-next-line
JotFormFrameLoaded();
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@brad is it ok for the monitoring if this is called more than once per load?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that would give us misleading stats about how often this happens. I'd fix this in jotformLoader.js though, where we listen for this call, and be resilient against multiple calls from either embed method in the future.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, I suppose that promise can't resolve twice... I wonder if it causes an error.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems to resolve twice (or at least it logs the call twice)
Screen Shot 2019-04-18 at 6 22 16 PM

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

update - it only goes into the then once, so it only actually sends the load event to New Relic once, but it calls JotFormFrameLoaded (which does the console.log) twice.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, that answers the question. 👍

@clareconstantine
Copy link
Author

Got verbal confirmation from @islemaster that this is ready to ship!

@clareconstantine clareconstantine merged commit f45f91e into staging Apr 19, 2019
@clareconstantine clareconstantine deleted the update-jotform-embeds branch April 25, 2019 22:12
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.

None yet

3 participants