[Bug]: Content overflows outside content area on mobile devices for theme Feelin' Good
Quick summary
Feelin' Good theme is not mobile responsive. The content added overflows outside content area for mobile devices. Some of the content can't be seen. User reported issue with videos on their site in -zen. I could replicate this on my site - https://testbizplanplayground.wpcomstaging.com/home-2/
Steps to reproduce
- Activate Feelin' Good theme - https://wordpress.com/theme/feelingood
- Create a page and add image block with image
- Add videopress block with a video
- Add few column blocks with content
- Add paragraph block with some text
- Publish the page
- Check the page on mobile device
What you expected to happen
The content should appear within the content area
What actually happened
The content (text, image, video) stretches outside the content area making a portion of it not visible to viewers
Browser
Google Chrome/Chromium
Context
Customer report in 7894295-zen regarding video not fitting in the frame.
Platform (Simple, Atomic, or both?)
Simple, Atomic
Other notes
No response
Reproducibility
Consistent
Severity
None
Available workarounds?
None
Workaround details
On inspecting I noticed, this is caused because of following CSS
body .is-layout-flex { display: flex; }
Changing to display:block fixes the issue but I feel this workaround is incorrect. Let me know if this would work as temporary fix:
@media only screen and (max-width: 600px) { body .is-layout-flex { display: block; } }
Support References
This comment is automatically generated. Please do not edit it.
- [ ] -zen
- [ ] 7894295-zen
I have reproduced this and will look at a fix now.
This is turning out to be a tricky issue. The fix is simple in theory but can be made in different places: core, Jetpack, and individual theme files. I've found that most themes don't have this issue, but comparing those with Feelin' Good, I can't see what's different. Ideally, I would make Feelin' Good consistent with them rather than add some special CSS to its theme.
Another option is to update Jetpack in a way that iframe use width:100%; max-width: nnnpx instead of width:nnnpx as the first one will not overflow like the containers. I've drafted a PR, https://github.com/Automattic/jetpack/pull/37025, but I'm still not sure it's the best solution and testing turned out to be problematic anyway.
I also found that the Freddie theme has a similar issue to the one reported here. There are probably others, too, which makes me think that we should fix it in the block. I will try and deploy something tomorrow.
I've drafted a PR, https://github.com/Automattic/jetpack/pull/37025, but I'm still not sure it's the best solution and testing turned out to be problematic anyway.
This looks workable to me with the aspect-ratio change. Let's just make sure we're not breaking other themes in the process:
Other themes like twentytwentythree / four are doing dynamic resizing of the iframe to avoid this overflow. Is that part of videopress or the theme itself?
Other themes like twentytwentythree / four are doing dynamic resizing of the iframe to avoid this overflow. Is that part of videopress or the theme itself?
This is the resize function that sets the iframe's width and height:
This code is in the script https://v0.wordpress.com/js/next/videopress-iframe.js?m=1674852142 and runs on load. It executes for all themes I checked. The value it uses for the width comes from the parent element, so it depends on the CSS rules used in the parent DOM hierarchy, and in Feelin' Good, those rules are different from those in other themes, where it doesn't overflow.
Let's just make sure we're not breaking other themes in the process:
This is always the tricky part with so many themes out there. But based on how T23 and T24 work today, setting width to be 100% shouldn't have any adverse effects. I will test a fix tomorrow with a mix of themes.
I'll also update the aspect-ratio.
@lsl
@mpkelly You think this should be flagged as project or apply simple fix and provide more elegant solution later on?
I'm taking this over. I have found a solution.
| BEFORE | AFTER |
|---|---|
The issue is because the script gets the iframe parent to get the width without removing the iframe from the DOM tree so it cannot return the correct value.
The fix would be something like this:
+ // Hide the iframe to calculate the real parent width
+ t.firstElementChild.style.display = 'none';
var r = getComputedStyle(t), n = parseFloat(r.width);
+ // Show the iframe after reading the parent's width
+ t.firstElementChild.style.display = null;
Where can I find the source file for videopress-iframe.js to make that change? I haven't found it in PCYsg-28C-p2