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

#135/aspect ratio #136

Merged
merged 4 commits into from
Nov 15, 2024
Merged

#135/aspect ratio #136

merged 4 commits into from
Nov 15, 2024

Conversation

camelPhonso
Copy link
Contributor

Description

Closes #135

This pr is dependent on the changes in the outstanding PR on the backend

Files changed

  • utils/popup-utils/parseAspectRatio.ts - because the backend will only allow a drop down selector for string values (which must include characters besides numbers), this util will take any string that includes a recognisable aspect ratio in it (i.e.: 4:3) and parse that out and return the calculated ratio
  • parseWindowSize.ts - now expects a string for the aspectRatio and calls parseAspectRatio.ts to return the appropriate ratio calculated
  • backgroundPopCreate.ts && displayPopup.ts- both take the new aspect_ratio property from the popups and pass them to parseWindowSize.ts if the type of content is "video"
  • types/eventTypes.ts - extend the types system to reflect the new property for MediaContent

UI changes

It all looks the same except the window size for popups will match what's been chosen in the CMS. The ratio will default to 16:9 if nothing has been passed or if the content type is an image or text.

Changes to Documentation

n/a

Tests

  • parseWindowSize.test.js - one of the tests was checking the return value for a call where the aspect ratio is explicitly passed. This was adjusted as the argument type is no longer a number but a string
  • parseAspectRatio.test.js - new test suite for the new util

Copy link
Contributor

@nichgalzin nichgalzin left a comment

Choose a reason for hiding this comment

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

Nice, looks great.

@nichgalzin nichgalzin merged commit be995bb into dev Nov 15, 2024
1 check passed
@nichgalzin nichgalzin deleted the #135/aspect-ratio branch November 15, 2024 12:04
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.

Allow different aspect ratios for video popups ♦ 🔷 ⬛
2 participants