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

imageAspectRatio: 1:1 doesn't change width and height of the frame unless they're set explicitly #463

Closed
1 task done
dalechyn opened this issue Aug 6, 2024 · 5 comments
Labels
Good First Issue Misc: Good First Issue

Comments

@dalechyn
Copy link
Collaborator

dalechyn commented Aug 6, 2024

Describe the bug

Screenshot 2024-08-06 at 22 53 31

Link to Minimal Reproducible Example

No response

Steps To Reproduce

No response

Frog Version

latest

TypeScript Version

No response

Check existing issues

Anything else?

No response

@dalechyn dalechyn added the Good First Issue Misc: Good First Issue label Aug 8, 2024
@DrParadox05
Copy link

Hey, can I work on this?

@dalechyn
Copy link
Collaborator Author

dalechyn commented Sep 2, 2024

Hey, can I work on this?

sure!

@DrParadox05
Copy link

Hey, how can I reproduce this?

@dalechyn
Copy link
Collaborator Author

dalechyn commented Sep 3, 2024

Hey, how can I reproduce this?

Simply change the aspect ratio to 1:1!

What would be expected is if aspect ratio is 1:1 to have library automatically to fall back to a height: 1000 and width: 1000.

Should also think about the frog/ui which also accepts height and width properties and don't adjust if aspect ratio is 1:1. Possibly need to pass aspect ratio as a createSystem parameter.

@dalechyn
Copy link
Collaborator Author

I spent some time thinking how to pull this off properly.

Since imageAspectRatio doesn't do much but sets the fc:frame:image:aspect_ratio tag, it's not included as a request to the route that generates an image.

Unless Image Handler is used, the default behavior compresses the image in search parameters and we've seen many issues due to browser's limitations of maximum URL length.
I'd not be willing to add another parameter there since there's a risk to increase URL length and potentially break frames that exist now.

Also, imageAspectRatio would then have to be passed to createSystem, which made to be unaware of the Frog instance itself, and the change would make it aware of it, or would require passing imageAspectRatio there explicitly again.

Too many effort for such a little fix.
Closing for now, but if anyone has ideas on how to fix it without breaking the API, you're welcome to!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Good First Issue Misc: Good First Issue
Projects
None yet
Development

No branches or pull requests

2 participants