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

fixes image fit property not properly working #138

Closed
wants to merge 2 commits into from

Conversation

lscardinali
Copy link

@lscardinali lscardinali commented Mar 1, 2019

Apparently the newly added FadeTransition Widget has trouble expanding its size to fill its parent widget. By putting a Container as a parent and setting the Width and Height to a MediaQuery fixes the problem, allowing the image to fully expand

References:
This StackOverflow post describes a bit what was happening and how to solve it
https://stackoverflow.com/questions/54239068/correctly-fit-image-with-fadeinimage-flutter

This Pull Request should solve the following issue:
https://github.com/renefloor/flutter_cached_network_image/issues/134

@lscardinali
Copy link
Author

Changed a bit the implementation, using the container caused some problems in some other image dispositions. Also Pull Request #139 has problems making the image extend to all edges.
With the new code, the image should expand to the available width and height when those are not provided.

@renefloor
Copy link
Contributor

@lscardinali not sure what you mean with Also Pull Request #139 has problems making the image extend to all edges., but that pull request seems to work nicely with some cases I tested and seems to be a cleaner solution.

@lscardinali
Copy link
Author

I'll run some more tests on pull request 139. The time i tried to add that fix to my app did work on most cases but there were one that was still broken. If i can't find any problems then i'll close this PR.
Thanks and sorry for not being clear on the description.

@renefloor
Copy link
Contributor

No problem. Thanks for helping. I don't know all the possible cases myself.

@lscardinali
Copy link
Author

Sorry, the problem was on my side. I'll close this PR now.

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.

2 participants