Site Editor Compatibility: Fix layout of latest instagram posts cross browser

[aaronrobertshaw]

Fixes #19466

Changes proposed in this Pull Request:

• Updates the latest instagram posts' styles to prevent the images stretching out to 100% height with the FSE

Does this pull request change what data or activity we track or use?

No change.

Testing instructions:

1. With the Gutenberg plugin active (>=10.4.0-rc1), activate a block-ready theme (like tt1-blocks)
2. In the site editor, insert an Instagram Latest Posts block into the Footer template part
3. Connect an account, and note that the images are loaded much too tall vertically
4. Checkout this PR, rebuild, then reload the site editor
5. The images should now be the expected square layout
6. Confirm the latest instagram posts block still displays correctly in the normal post editor and on the frontend
7. Test across different browsers. Especially Safari as it behaves slightly differently in terms of calculating dimensions of flex/grid layouts.

Screenshots

Before	After

[matticbot]

Caution: This PR has changes that must be merged to WordPress.com
Hello aaronrobertshaw! These changes need to be synced to WordPress.com - If you 're an a11n, please commandeer and confirm D60136-code works as expected before merging this PR. Once this PR is merged, please commit the changes to WP.com. Thank you!
This revision will be updated with each commit to this PR

[github-actions]

Thank you for your PR!

When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:

• ✅ Include a description of your PR changes.
  
• ✅ All commits were linted before commit.
  
• ✅ Add a "[Status]" label (In Progress, Needs Team Review, ...).
  
• ✅ Add testing instructions.
  
• ✅ Specify whether this PR includes any changes to data or privacy.
  
• ✅ Add changelog entries to affected projects
  

This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖

---

Once your PR is ready for review, check one last time that all required checks (other than "Required review") appearing at the bottom of this PR are passing or skipped.
Then, add the "[Status] Needs Team review" label and ask someone from your team review the code.
Once you’ve done so, switch to the "[Status] Needs Review" label; someone from Jetpack Crew will then review this PR and merge it to be included in the next Jetpack release.

---

Jetpack plugin:

• Next scheduled release: May 4, 2021.
• Scheduled code freeze: April 26, 2021.

[andrewserong]

Thanks for working on this one @aaronrobertshaw! It's looking good when the feed is made up of square images. On my Instagram, though, I've got a range of image sizes from square to portrait and landscape. On the front end (and in the post editor before this change), the images get the portrait layout (perhaps because the first image is portrait?) and the height: 100% paired with object-fit: cover causes the remaining images to fill out the container. With this change, in the editor, the images retain their height resulting in below (tested in Chrome on macOS):

Front end view	Editor view

I'm not quite sure what the solution is... would it be possible for the img tag to retain height: 100%, but for the height on .wp-block-jetpack-instagram-gallery__placeholder.is-loaded to be set to something else?

[aaronrobertshaw]

I'm not quite sure what the solution is... would it be possible for the img tag to retain height: 100%, but for the height on .wp-block-jetpack-instagram-gallery__placeholder.is-loaded to be set to something else?

I'll take another run at it. Browsers seem to differ on how they calculate their layouts for flex and grid. In Safari for example the 100% in the editor is causing the grid rows to get the full image height.

[aaronrobertshaw]

@andrewserong I believe the handling of varying aspect ratios has been fixed now if you'd like to give it another try. It's working for me across browsers with a mix of landscape, square and portrait images.

Rather than forcing 100% height, the new approach relies on letting the flex layout grow the image to the appropriate height.

[andrewserong]

Nice, that's fixed that up @aaronrobertshaw. In Chrome, Firefox, and Safari, the images are rendering at the correct full height now. Unfortunately, I noticed another issue on Safari that isn't present before this change in the post editor... it looks like the image spacing slider isn't working in Safari, but is working for me in Firefox and Chrome. I also notice that in Safari click out and into the block is inconsistent as to whether or not it registers the block as being selected, but I'm not sure whether that's an issue in FSE, or with this CSS change.

[aaronrobertshaw]

Unfortunately, I noticed another issue on Safari that isn't present before this change in the post editor

Thanks for the thorough testing, one step forwards...two steps back 🙂

I'll work on resolving the new issues tomorrow.

[andrewserong]

Thanks for persevering with it! 🙂

[aaronrobertshaw]

it looks like the image spacing slider isn't working in Safari

After a quick test, it appears for me that changing the image spacing doesn't force Safari to redraw the grid.

If I adjust the value, save the post and reload. The correct spacing is displayed. Toggling the any of the grid styles in Safari's dev tools force it to update as well.

[aaronrobertshaw]

It turns out that the inline padding on grid children is required to cause Safari to redraw the grid. This padding was previously removed via a padding: 0 !important CSS rule so it didn't "double up" the grid gap once that was rendered.

The theory is with a change in the display type and the stylesheets being copied into the site editor's iframe, the order in which the inline styles are applied might be different. This results in the padding: 0 !important always taking affect now as you might expect. Without the padding changing, the redraw of the grid doesn't happen when the grid gap is updated.

The current working solution is to remove that !important rule and counter it with a negative margin in the editor only. Then simply omit the inline padding when the block is dynamically rendered on the frontend. From my testing so far this is working cross browser for me.

If anyone has any better, less hacky, ideas on solving this one, I'd be happy to hear them!

Otherwise, some extra testing would be helpful 🙂

[ramonjd]

Nice sleuthing on this one!

I've tested the following in Chrome, Firefox, Edge and Safari in both site and block editors:

✅ Instagram images appear as expected (cover, and not stretched)
✅ Controls work as expected, including column and image spacing

On the frontend the correct inline style is applied for the grid gap. Here I chose 50px in the editor:

[ramonjd]

Tested across desktop browsers

Image-stretch as well as spacing adjustment is resolved in Safari. Looks good in other browsers as well.

🎉

[andrewserong]

@aaronrobertshaw this is testing well at desktop sizes for me across Safari, Chrome, FF and Edge. However, I think it introduces a regression for the Stack on mobile layout. If that's deselected, then the mobile view correctly has spacing:

If I select stack on mobile, and then view in a narrow viewport, I get this before and after (tested in the post editor on a Varia theme):

Before this change	After this change

So, I wonder if that was another thing that depended on the padding?

[andrewserong]

Nice, that's done the trick, the responsive stacked view on mobile is working for me now. Just did a quick smoke test in desktop Safari and Chrome too, and looking good. Thanks for following up with all the edge cases @aaronrobertshaw! 👍

[jeherve]

This tests well for me. I only have one minor remark.

[github-actions]

Great news! One last step: head over to your WordPress.com diff, D60136-code, and commit it.
Once you've done so, come back to this PR and add a comment with your changeset ID.

Thank you!

[aaronrobertshaw]

Deployed: rWP224580