-
Notifications
You must be signed in to change notification settings - Fork 1
A3 embeds enhancements #693
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
Conversation
…l layout + wip open-iframe-resizer
… in an auto-resizing iframe
38bbb11 to
4dde39a
Compare
|
Preview deployment: https://a3-embeds-enhancements.preview.avy-fx.org |
…a little less jarring
…y need a single instance of it
…ript tags in the same html can't be self-closing or will be stripped out by DOMPurify + need allow-popups to allow open in new tab links in embeds
…sign but I think a single row makes sense so we use divide-y -- there is no good way to target the last element in a grid column that's widely available in browsers
rchlfryn
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
Otherwise, it might be nice to add all the available options (like |
|
Just a note: The embed was not leading on preview because of a mixed content error. This is low priotity Mixed Content: The page at 'https://nwac.a3-embeds-enhancements.preview.avy-fx.org/education/classes/courses-by-external-providers' was loaded over HTTPS, but requested an insecure frame 'http://a3-embeds-enhancements.preview.avy-fx.org/embeds/providers'. This request has been blocked; the content must be served over HTTPS. |

Description
We still needed a few things for A3 embeds but #675 was already too big.
Those things are:
Related Issues
Resolves #317
Key Changes
Moves /embeds routes into their own route group
(embeds)so we can use a different html skeleton for those routesUses a different persistence setting for PostHog on /embeds route
Adds
textColorquery param to /embeds/providers and /embeds/coursesAdds a CustomEmbedStyles component which sets the custom background and text colors on the html and body tags as well to avoid a flash of transparent/white background when the iframe is resizing. This was particularly necessary for when closing a providers accordion.
Uses IframeResizer for all GenericEmbed components.
Future enhancements / Questions
I think we should add an optional initial height field to the GenericEmbed component. We had height at one point but I want to depend on open-iframe-resizer to set the height of the content. Initial height would allow a user to set a min-height on the containing iframe to help with avoiding layout shift. We'll need to be thorough in our description of the field so it won't be misused but would be very nice if we could get users to use it.