Skip to content

AspectRatio is forcing its img, video children to behave objectFit="cover" #4347

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

Closed
3 tasks
tomchentw opened this issue Jul 8, 2021 · 2 comments
Closed
3 tasks
Labels
needs triage Issues and pull requests that need triage attention Type: Bug 🐛 Something isn't working workaround available ✌️ Issue has quick hack or walk-around

Comments

@tomchentw
Copy link
Contributor

tomchentw commented Jul 8, 2021

Description

When I set <Image objectFit="contain" />, I expect it to happen. But if I render it inside an <AspectRatio />, it will always be objectFit="cover".

<AspectRatio ratio={3 / 1}>
  <Image
    src="https://bit.ly/naruto-sage"
    alt="naruto"
    objectFit="contain" // Does NOT work
  />
</AspectRatio>

Link to Reproduction

https://codesandbox.io/s/awesome-ritchie-ec8n9?file=/src/index.js

Steps to reproduce

  1. Go to https://codesandbox.io/s/awesome-ritchie-ec8n9?file=/src/index.js
  2. You should see the "Actual" and the "Expected" section

CS

Chakra UI Version

1.6.4

Browser

No response

Operating System

  • macOS
  • Windows
  • Linux

Additional Information

Source:
https://github.com/chakra-ui/chakra-ui/blob/%40chakra-ui/react%401.6.4/packages/layout/src/aspect-ratio.tsx#L62-L64

Tracing it back, these lines were first introduced in this commit: c91fdf3#diff-ce7500ee881128fd4e892b90abbccad46958754956da612389d3d5e4f75ebec8


In my opinion, the objectFit of its img, video children should be configurable directly on its children, not on the <AspectRatio /> itself.

@tomchentw tomchentw added needs triage Issues and pull requests that need triage attention Type: Bug 🐛 Something isn't working labels Jul 8, 2021
@tomchentw
Copy link
Contributor Author

Since @segunadebayo authored the commit c91fdf3#diff-ce7500ee881128fd4e892b90abbccad46958754956da612389d3d5e4f75ebec8
I'm wondering what's the rational behind this? Are we okay to have a breaking change that removes this behavior? Thank you.

@segunadebayo
Copy link
Member

This might have been an assumption around the possible use-cases of the aspect-ratio component. This was added to ensure that images stretch to fill the aspect ratio container's space.

At the moment, we're not able to change this as it'll be considered a breaking change. Removing it is not technically the solution here, but exposing a prop that determines whether to stretch the child is the solution.

As a workaround, feel free to use the workaround or pass objectFit: contain !important to the Image component.

Thanks for understanding.

@segunadebayo segunadebayo added the workaround available ✌️ Issue has quick hack or walk-around label Aug 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs triage Issues and pull requests that need triage attention Type: Bug 🐛 Something isn't working workaround available ✌️ Issue has quick hack or walk-around
Projects
None yet
Development

No branches or pull requests

2 participants