search

zulip / zulip-flutter

Upcoming Zulip mobile apps for Android and iOS, using Flutter
Apache License 2.0
179 stars 169 forks source link

content: Design a "request failed" placeholder for images in messages #265

Open chrisbobbe opened 1 year ago

chrisbobbe commented 1 year ago

By default when an image's network request fails, the image just stays blank.

We should override that default with our own custom placeholder. The API gives us the error object, so if we want to, I expect we can offer a different placeholder for different failure modes, like:

Note that there's a related but different issue, where we don't handle well the case where an image's srcUrl string doesn't parse. For that, see discussion at: https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/flutter.3A.20Handling.20preview-image.20.60src.60.20that.20doesn't.20parse/near/1623918

Details about debug builds In debug builds, there's an uglier default placeholder: | In message list | In lightbox | | --- | --- | | image | image | (And in case it's helpful, here's a video of tapping the preview to open the lightbox, then exiting the lightbox: https://github.com/zulip/zulip-flutter/assets/22248748/1a50401b-eec6-4a2a-8002-583a6ed15e34 )
gnprice commented 1 year ago

Cool, good thought. @terpimost, do you have a suggestion for what design to use?

My default would be to go for something similar to what browsers show, with a "broken image" icon.

terpimost commented 1 year ago

How about this icon in the middle of the image?

image

color is #818181 with 50% transparency

image-error

I suggest to have background be #000000 3% transparency background in light theme and #ffffff 3% transparent background in dark theme.

image image
gnprice commented 3 months ago

The default placeholder for an image's failed network request is ugly

Looking back at the screenshots, I notice that they're in a debug build. That placeholder has the feel of a debug-build placeholder to me. I wonder what the status quo is in release builds — whether it's that or something more reasonable already.

gnprice commented 1 week ago

My question above about release builds has been answered: the existing behavior is quite reasonable, which is that the images just stay blank.

It'd still be nice to have an explicit "broken image" display. But I think the existing behavior is fine for launch, so pushing this out to post-launch.

The way the question got answered is that we had a burst of broken images on chat.zulip.org yesterday and today, as a result of experimental changes to avatars for system bots: https://chat.zulip.org/#narrow/channel/48-mobile/topic/notification.20bot.20icon/near/1966229 The avatars for Notification Bot and other system bots are currently broken; and this related message has five image previews of which the last three give 404.

In my observation using release builds of the app, all of those broken images remain blank, effectively transparent, so that the applicable ambient background color shows through. That includes avatars for message senders; avatars on the profile page; and image previews in messages, as this issue is aimed at.