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

feat: Add limited support for objectPosition property on images #464

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

damianstasik
Copy link

Currently any image that has objectFit property will be centered without any way to control this behavior.

This PR adds a limited support for objectPosition for images, by tweaking what is being passed to preserveAspectRatio property.

By limited I mean you need to pass both X and Y, and for both of them you can pass only three keyword values:

  • for X: left, center, right
  • for Y: top, center, bottom

It is still something more than always center, so I hope it is useful enough to get merged.

I am marking this as draft, because I am not sure what is the preferred way of testing it. Should I convert the test cases to use snapshots?

@vercel
Copy link

vercel bot commented Apr 27, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
satori-playground ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 7, 2023 10:00pm

@damianstasik damianstasik changed the title Add limited support for objectPosition property on images feat: Add limited support for objectPosition property on images Apr 27, 2023
Copy link
Member

@shuding shuding left a comment

Choose a reason for hiding this comment

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

Yep please use snapshots so it's easier to identify changes. We might modify the underlying SVG code some day. Thanks!

@Aerophite
Copy link

Can we get some movement on this? This is definitely functionality that I'd like to use!

@damianstasik
Copy link
Author

I'll try to take a look at it today or tomorrow.

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.

3 participants