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

dalechyn commented 1 month ago

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

[X] I checked there isn't already an issue for the bug I encountered.

Anything else?

No response

DrParadox05 commented 2 weeks ago

Hey, can I work on this?

dalechyn commented 1 week ago

Hey, can I work on this?

sure!

DrParadox05 commented 1 week ago

Hey, how can I reproduce this?

dalechyn commented 1 week ago

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 commented 5 days ago

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!

wevm / frog