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.

Sign up for GitHub

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

Jump to bottom

Make use of thumbnails #5875

Closed
alya opened this issue Jun 25, 2024 · 2 comments · Fixed by #5880
Closed

Make use of thumbnails #5875

alya opened this issue Jun 25, 2024 · 2 comments · Fixed by #5880
Assignees
@gnprice
Labels
a-lightbox a-message list performance Issues related to improving performance (speed, latency, battery usage, etc) server release goal Things we should try to coordinate with a major Zulip Server release.

Comments

@alya
Copy link
Collaborator

alya commented Jun 25, 2024 •
edited by gnprice
Loading

In zulip/zulip#16210, we're implementing thumbnailing for uploaded images.

We should use thumbnails in the mobile app. In particular, the message view should show thumbnails, while the lightbox should download the full-scale images.

Blockers:

Flutter issue:

#api design threads:
@alya alya added a-message list a-lightbox performance Issues related to improving performance (speed, latency, battery usage, etc) server release goal Things we should try to coordinate with a major Zulip Server release. labels Jun 25, 2024
@alya alya mentioned this issue Jul 10, 2024
Closed
@gnprice gnprice self-assigned this Jul 16, 2024
@gnprice
Copy link
Member

gnprice commented Jul 16, 2024

The current state of this is (release-management chat thread, and see also the #api design threads linked above):

  • The server side might be ready to test on chat.zulip.org later today; if not, then hopefully by tomorrow at latest.
  • We'll verify nothing goes horribly wrong already with existing clients; if it does, figure out on #api design how to deal.
  • We'll verify that once there is a thumbnail, the existing client behaves well and there's nothing to do.
  • We'll implement styling for the thumbnail placeholder.

gnprice added a commit to gnprice/zulip-mobile that referenced this issue Jul 17, 2024
@gnprice
content: Show spinner for thumbnail placeholders
5bbbc2b 
This is a port of zulip/zulip#30477.  That PR isn't yet merged, but
it's been deployed on chat.zulip.org and I believe the styling is
unlikely to change.  (If it does get tweaked, we can always update.)

The placeholder `img` elements already come with a `src` pointing
to this same `loader-black.svg` file, but on the Zulip server.
So the effects of this change are:

 * The placeholder image is local instead of remote, which may
   improve the experience a bit on a slow connection.

 * In dark theme, we use an appropriately contrasting white spinner,
   instead of a black spinner that blends in with the background.

Fixes: zulip#5875
@gnprice gnprice mentioned this issue Jul 17, 2024
Merged
@gnprice
Copy link
Member

gnprice commented Jul 17, 2024

  • We'll verify nothing goes horribly wrong already with existing clients; if it does, figure out on #api design how to deal.
  • We'll verify that once there is a thumbnail, the existing client behaves well and there's nothing to do.

Confirmed!

  • We'll implement styling for the thumbnail placeholder.

Sent this as #5880.

Details in chat thread: https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/thumbnails/near/1892137

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@gnprice gnprice

Labels
a-lightbox a-message list performance Issues related to improving performance (speed, latency, battery usage, etc) server release goal Things we should try to coordinate with a major Zulip Server release.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Style thumbnail placeholders nicely gnprice/zulip-mobile
2 participants
@gnprice @alya