ABW-1983: Fix the issue with brokenImage #663
Conversation
There was a problem hiding this comment.
Hmm the screenshots does show improvements for sure, but the NFTs list is probably not not how we want it to look and NFTs are arguable the most important images... should you be using different content modes and resizeable view modifier perhaps? perhaps @kugel3 has some ideas?
| } | ||
| .frame(height: .imagePlaceholderHeight) |
There was a problem hiding this comment.
Can't we use aspectRatio? or even just set the frame?
There was a problem hiding this comment.
Can't we use aspectRatio?
There is a number of possible expectations when it comes to layout. aspectRatio is not a silver bullet. I think it's safe to reuse the sizing behavior of the image in the context of brokenImage.
or even just set the frame?
I'm not sure if I understand your comment correctly @kugel3 - it was previously set and that's the source of the entire issue. We can't simply set the frame here as this component works in a number of different contexts.
|
When did this issue first appear? As far as I remember, this used to work fine everywhere. |
Not sure when exactly the regression got introduced, but all the recent changes to that file belong to you @kugel3 . 
|
|
So I reused the logic of We have two more things out there as well - the |
maciek-rdx
requested review from
CyonAlexRDX and
kugel3
and removed request for
CyonAlexRDX
|
Fixed in competing PR |
Jira ticket: ABW-1983
Description
Fixes the issue with
brokenImage. It's a problem common to all images that are displayed usingLoadableImageunder the hood, so the change is pretty significant. I think it looks better now for most cases. Thelist of NFTsvalidatorbrokenImageplaceholders still doesn't look that great, but I think it's not that big of a deal.Notes
There are some limitations to applied solution that I highlighted here.
How to test
Retest the layout of loadable images, when the image link is broken across the app.
Screenshot