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

fix: file image doesn't render in full size #1935

Merged
merged 5 commits into from
Oct 9, 2023
Merged

Conversation

Paul-Bob
Copy link
Contributor

Description

Fixes #1897

File image was applying aspect-video forcing the image to render on 16/9 aspect ratio format. This PR removes that format.

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works

@Paul-Bob Paul-Bob self-assigned this Aug 31, 2023
@github-actions github-actions bot added the Fix label Aug 31, 2023
@codeclimate
Copy link

codeclimate bot commented Aug 31, 2023

Code Climate has analyzed commit 849c5dd and detected 0 issues on this pull request.

View more on Code Climate.

@adrianthedev adrianthedev changed the title fix: file image don't rendering full size fix: file image doesn't render in full size Sep 9, 2023
Copy link
Collaborator

@adrianthedev adrianthedev left a comment

Choose a reason for hiding this comment

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

What if someone wants to apply that aspect ratio?

How about this?
we add all Tailwind aspect- classes in the whitelist file and folks can apply it when they want it with the html option.
Is that something we could support?

@Paul-Bob
Copy link
Contributor Author

What if someone wants to apply that aspect ratio?

How about this? we add all Tailwind aspect- classes in the whitelist file and folks can apply it when they want it with the html option. Is that something we could support?

How would you target that specific div with html option? I think using html option it's possible to apply it oo the wrapper but not that particular div.

@Paul-Bob
Copy link
Contributor Author

Paul-Bob commented Sep 11, 2023

About the white list file, there are too many combination, we can't guess all what users want to use (sure we can add the aspect-[all] but do we want to add each custom style for each host app on request?), maybe we should provide a hook where users can declare them own whitelist that will merge with ours.

PS: @adrianthedev maybe this issue (Allow variants on file fields) is related, I see people wanting to use variants, that it's even more customizable than aspect-something

@Paul-Bob
Copy link
Contributor Author

@adrianthedev the aspect-video it's applied only when multiple files are rendered as grid. Also renamed some components for better readability

@github-actions
Copy link
Contributor

github-actions bot commented Oct 5, 2023

This PR has been marked as stale because there was no activity for the past 15 days.

@Paul-Bob Paul-Bob merged commit 0ea43bf into main Oct 9, 2023
13 checks passed
@Paul-Bob Paul-Bob deleted the fix/file_image_size branch October 9, 2023 11:43
@github-actions
Copy link
Contributor

github-actions bot commented Oct 9, 2023

This PR has been merged into main. The functionality will be available in the next release.

Please check the release guide for more information.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Vertical image is truncated in file field show view
2 participants