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

Use the aspect-ratio property. #109

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

peiche
Copy link
Contributor

@peiche peiche commented Nov 8, 2021

The aspect-ratio property has support across all major browsers. Since CodyFrame v3 has dropped support for IE, I propose it adopts proper aspect ratio implementation, as opposed to the padding hack. Cheers!

@sebastiano-guerriero
Copy link
Contributor

Hi Paul, we're still looking into this because 1. support is not that great yet (e.g., aspect-ratio is not supported in iOS < v15, which is about 10% according to caniuse), and 2. it would be great if the new classes are somehow 'compatible' with the old ones - not sure if possible. Anyway, this is def on our radar 👍

@peiche
Copy link
Contributor Author

peiche commented Dec 1, 2021

Hi @sebastiano-guerriero, thanks for the feedback. I just added the fallback again for browsers that don't support the property natively. (Props to CSS Tricks for providing the @supports logic.)

@sebastiano-guerriero
Copy link
Contributor

Hi @peiche! I've run many tests based on your suggestion. Here are the results:

  • the aspect-ratio CSS property is quite smart, and it's not possible to provide a real fallback right now. The method described in the CSS Tricks article, for example, wouldn't work if you apply the utility class to an image or any other element that can't have pseudo elements (while the CSS aspect-ratio property can be used on images etc.).
  • the current implementation requires to use the .aspect-ratio-{ratio} class on the parent of an image/video/iframe. Hence the new, proposed implementation would cause a breaking change.
  • the only way to create utility classes based on the aspect-ratio property is to use them in projects where support for older browsers is not needed. E.g., you can create custom ratio classes in your 'custom-style/_util.scss' CodyFrame file:
.ratio-16\:9 { aspect-ratio: 16/9; }
.ratio-3\:2  { aspect-ratio: 3/2; }
.ratio-4\:3  { aspect-ratio: 4/3; }
.ratio-5\:4  { aspect-ratio: 5/4; }
.ratio-1\:1  { aspect-ratio: 1/1; }
.ratio-4\:5  { aspect-ratio: 4/5; }
.ratio-3\:4  { aspect-ratio: 3/4; }
.ratio-2\:3  { aspect-ratio: 2/3; }
.ratio-9\:16 { aspect-ratio: 9/16; }

In the future, probably the best approach would be renaming the current classes to something like .media-wrapper-{ratio} and use the aspect-ratio property for the `.aspect-ratio-{ratio}' classes - with no fallback.

Any feedback is welcome.

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.

2 participants