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

Mwpw 170384 lost info 320 #3881

Open
wants to merge 5 commits into
base: stage
Choose a base branch
from
Open

Mwpw 170384 lost info 320 #3881

wants to merge 5 commits into from

Conversation

zagi25
Copy link
Contributor

@zagi25 zagi25 commented Mar 31, 2025

  • Prevent video from overflowing inside modal
  • Put chat button behind modal curtain when modal is open

Resolves: MWPW-170384

Test overflow by zooming in the page or on very wide screen.
Chat button is only present on CC page.

Test URLs:

Example CC page from the ticket

@zagi25 zagi25 requested a review from a team March 31, 2025 08:57
@aem-code-sync aem-code-sync bot temporarily deployed to MWPW-170384-lost-info-320 March 31, 2025 12:46 Inactive
Copy link
Contributor

aem-code-sync bot commented Mar 31, 2025

Copy link
Contributor

@mokimo mokimo left a comment

Choose a reason for hiding this comment

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

Small suggestion in regards to making the z-index relationships more obvious, would be nice to take that in before merging.

@@ -1,3 +1,7 @@
:root {
--iframe-video-aspect-ratio: 0.5625;
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm a bit reluctant to assume a 16:9 aspect ratio for all videos inside modals. We have 9:16 videos (which seem to be covered by the tall-video modal metadata style), square videos, etc. What would be the outcome for other aspect ratios? A modal larger than the video? Or would it have any other effect?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added the tall-video example, on normal size screens it behaves the same because tall-video overrides my css.
For square videos I assume they will behave the same as 16:9 behaves on ultra wide screen
Screenshot 2025-04-04 at 10 58 27 (2)
Modal gets wider than the video when it hits max-height.

I did notice that tall video gets an overflow on zoom bigger than 250% - 300% and also has bigger height on ultra wide screens ( and overflow on zoom)
Screenshot 2025-04-04 at 10 58 14 (2)
Screenshot 2025-04-04 at 11 03 58 (2)
I will look into tall-video issues that I found.
If you have a link to the example of the square video, that would be nice so I can check the exact behavior.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't have a prod example, but I generated something quickly with Express, so you can use it on a page 😅

Square.Video.mp4

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For square videos it will be the same way as for wide videos on ultra wide screen, modal becomes bigger then the video and MPC fills it with background.
Screenshot 2025-04-09 at 13 02 34

Copy link
Contributor

github-actions bot commented Apr 3, 2025

Reminder to set the Ready for Stage label - to queue this to get merged to stage & production.

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.

4 participants