Set a minimum height when using a 100% width embed #1401

jwalgran · 2021-06-16T18:23:22Z

Overview

When setting the embed height as a proportion of the page width, the narrow screen widths of mobile devices lead to an embed that is too sort to be functional.

To work around this we introduce additional styling of the embed that uses media query to override the percentage with a fixed, minimum height.

Connects #1400

Demo

Testing Instructions

I used this index.html page served wtih https://www.npmjs.com/package/http-server to test the embed codes generated by this PR

<!doctype html>

<html lang="en">
    <head>
        <meta charset="UTF-8"/>
        <meta name="viewport" content="width=device-width">
        <title>OAR Embed Demo</title>
    </head>
    <body>
        <h1>
            <div>
                My embedded map
            </div>
        </h1>
        <li>item one</li>
        <li>item b</li>
        <li>item the third</li>



        <!-- embed -->
        <!-- end -->

    </body>
</html>

Test that the embed code generated for both fixed and full width embeds work and the full width embed works at phone width.

Checklist

fixup! commits have been squashed
CI passes after rebase
CHANGELOG.md updated with summary of features or fixes, following Keep a Changelog guidelines

TaiWilkin

The changes to the embed code are working well on various size screens and with various height and width settings. However, the embedded map preview hasn't been updated to match and should be kept consistent.

lederer · 2021-06-16T19:57:16Z

Note this should be added to the <head> of the test html file to help simulate mobile layout:

<meta name="viewport" content="width=device-width">

lederer · 2021-06-16T20:44:37Z

Nice catch @TaiWilkin.

I think we can get away with a more relaxed minimum, so we're not usurping too much of their customers' webpages. 500px seems okay in my casual testing?

Even still, one wrinkle with our solution is that we run the risk of mooting the customer-specified relative height at desktop widths. Eg, if someone sets 100% width and 50% height (fairly short aspect ratio but not unreasonable), then even if we lower our hardcoded minimum to 500px, the embed height will remain at 500px all the way up to viewport widths of 1000px, after which it becomes dynamic/relative.

We could address this through a media query, where we force a static height only for viewports below a certain width (600px would match our current mobile breakpoint), and use the customer's relative height otherwise.

<div>
    <style>
        #oar-embed-GUID { /* append some RANDOM GUID? */
            position: relative;
            padding-top: 50%; /* customer-specified relative height */
        }
        @media (max-width: 600px) { /* mobile breakpoint */
            #oar-embed-GUID {
                padding-top: 500px; /* our mobile minumum */
            }
        }
    </style>
    <div id="oar-embed-GUID"> <!-- id for CSS -->
        <iframe
            src="http://localhost:6543/?contributors=6&embed=1"
            frameborder="0"
            allowfullscreen
            style="position:absolute;top:0;left:0;width:100%;height:100%;"
            title="embedded-map">
        </iframe>
    </div>
</div>

jwalgran · 2021-06-16T22:23:38Z

I implemented Scott's suggested media query-based solution, reduced the min height to 500px, and fixed the preview. Thanks for the help. This is ready for another review.

TaiWilkin

This is working very well. Nice incorporation of changes, and the additional code comments will be helpful. I was able to simulate a number of height and width settings successfully.

lederer

👍

When setting the embed height as a proportion of the page width, the narrow screen widths of mobile devices lead to an embed that is too sort to be functional.

The critical search button at the bottom of the sidebar search tab was being obscured by the footer.

When the contributor uses the 100% width embed option we were enforcing a minimum allowable height to avoid creating an iframe that is too short to be functional. Replacing the `max` function with a media query allows us to conditionally enforce this minimum only at mobile widths. We append GUID to the DOM ID we use to attach styles so that there is no chance of the ID conflicting with another element on the host page.

We update the content of the preview React component to be the same as the generated iframe text and add comments explaining that future changes to the markup should be applied in both places.

jwalgran · 2021-06-17T15:18:50Z

Thanks, again, for helping me revise this.

lederer · 2021-06-17T15:20:46Z

Thanks for jumping in and fixing this!

jwalgran requested review from lederer and TaiWilkin June 16, 2021 18:23

jwalgran assigned lederer and TaiWilkin Jun 16, 2021

TaiWilkin suggested changes Jun 16, 2021

View reviewed changes

jwalgran requested a review from TaiWilkin June 16, 2021 22:22

TaiWilkin approved these changes Jun 17, 2021

View reviewed changes

lederer approved these changes Jun 17, 2021

View reviewed changes

TaiWilkin assigned jwalgran and unassigned TaiWilkin Jun 17, 2021

jwalgran added 5 commits June 17, 2021 07:54

Set a minimum height when using a 100% width embed

8fe7b84

When setting the embed height as a proportion of the page width, the narrow screen widths of mobile devices lead to an embed that is too sort to be functional.

Add a sidebar bottom margin avoid being overlapped by embed footer

577ad80

The critical search button at the bottom of the sidebar search tab was being obscured by the footer.

Use the mobile-aware embed styling in the embed preview

b39209f

We update the content of the preview React component to be the same as the generated iframe text and add comments explaining that future changes to the markup should be applied in both places.

Update CHANGELOG

fec8eed

jwalgran force-pushed the feature/jcw/embed-min-width branch from 64c4068 to fec8eed Compare June 17, 2021 14:59

jwalgran merged commit d46648f into develop Jun 17, 2021

jwalgran deleted the feature/jcw/embed-min-width branch June 17, 2021 15:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Set a minimum height when using a 100% width embed #1401

Set a minimum height when using a 100% width embed #1401

jwalgran commented Jun 16, 2021 •

edited

TaiWilkin left a comment

lederer commented Jun 16, 2021

lederer commented Jun 16, 2021

jwalgran commented Jun 16, 2021

TaiWilkin left a comment

lederer left a comment

jwalgran commented Jun 17, 2021

lederer commented Jun 17, 2021

Set a minimum height when using a 100% width embed #1401

Set a minimum height when using a 100% width embed #1401

Conversation

jwalgran commented Jun 16, 2021 • edited

Overview

Demo

Testing Instructions

Checklist

TaiWilkin left a comment

Choose a reason for hiding this comment

lederer commented Jun 16, 2021

lederer commented Jun 16, 2021

jwalgran commented Jun 16, 2021

TaiWilkin left a comment

Choose a reason for hiding this comment

lederer left a comment

Choose a reason for hiding this comment

jwalgran commented Jun 17, 2021

lederer commented Jun 17, 2021

jwalgran commented Jun 16, 2021 •

edited