Show almost-instant preview when opening lightbox #6000

gaearon · 2024-10-29T15:53:53Z

Stacked on #5999

We have two kinds of images: the "previews" shown in feed and fullsize images.

Previously, the lightbox would always show the fullsize image. After this change, the lightbox will initially show the "preview" and then switch to the fullsize image once it's downloaded. This works via Expo Image placeholder prop.

This means that, in most cases, opening lightbox should now feel almost instant. (No animation yet though.) It's not fully instant because there's still a delay to load it from the cache etc — we'll have more control over it after upgrading Expo.

This PR achieves that in several steps:

We need to plumb down the thumbnail URL — this one is easy.
This couldn't work before because we waited for onLoad event and forcibly displayed the spinner until then. The problem is that onLoad doesn't fire for placeholder images. So we couldn't use the placeholder prop before. To solve that, I'm changing the layout to place the spinner beneath the image. Then it can be rendered unconditionally.

I've had to resolve a minor layout hiccup on Android which was compositing opacity during dismiss incorrectly.

While the image is loading, its dimensions aren't available. There's no quick way to get placeholder dimensions either so instead we just rely on the gestures (like zoom and pinch) being disabled until dimensions load.

The logic to ignore missing dimensions was already present. However, I've noticed that with internet being offline, the dimensions were incorrectly reported as 0x0 which caused double-tap gesture to send NaNs to zoomToRect, causing a crash on iOS. I've made a driveby fix to treat 0x0 as missing dimensions and to avoid calculations based on those.

Before

You always have to wait for the full image to load first. If there is no WiFi, you don't see anything at all on opening lightbox.

nowifi_before.mov

After

Images will appear instantly (and progressively enhance as full size loads) thanks to the preview being cached via feed.

If you have no WiFi, you still see the lower quality image (almost) instantly:

no_wifi.mov

However, you won't be able to interact with it (aside from dismissing) until the internet comes back online.

Test Plan

Try with WiFi off and on.

Substitute thumbUri with some fake URI to verify the image does get swapped.

Verify that the spinner still works (e.g. by setting thumbUri to a completely unrelated URL that isn't cached and turning off WiFi). Verify you can dismiss the image before even the placeholder has loaded.

Verify that while dismissing the image, the spinner beneath it is not visible.

gaearon · 2024-10-29T15:57:41Z

src/view/com/lightbox/ImageViewing/components/ImageItem/ImageItem.android.tsx

          accessibilityLabel={imageSrc.alt}
          accessibilityHint=""
-          onLoad={() => setIsLoaded(true)}
+          accessibilityIgnoresInvertColors


was told to add this by the linter

gaearon · 2024-10-29T15:58:25Z

src/view/com/lightbox/Lightbox.tsx

@@ -31,7 +31,9 @@ export function Lightbox() {
    const opts = activeLightbox
    return (
      <ImageView
-        images={[{uri: opts.profile.avatar || ''}]}
+        images={[
+          {uri: opts.profile.avatar || '', thumbUri: opts.profile.avatar || ''},


could pass the low res one but it's maybe too low res? can change tho

gaearon · 2024-10-29T15:58:33Z

src/view/com/profile/ProfileSubpageHeader.tsx

@@ -72,7 +72,7 @@ export function ProfileSubpageHeader({
    ) {
      openLightbox({
        type: 'images',
-        images: [{uri: avatar}],
+        images: [{uri: avatar, thumbUri: avatar}],


github-actions · 2024-10-29T16:00:40Z

Old size	New size	Diff
7.92 MB	7.92 MB	0 B (0.00%)

haileyok · 2024-10-29T16:06:25Z

src/view/com/lightbox/ImageViewing/components/ImageItem/ImageItem.ios.tsx

+        <Animated.View style={[styles.imageScrollContainer, animatedStyle]}>
+          <ActivityIndicator size="small" color="#FFF" style={styles.loading} />
+          <Image


Nice. This is actually the "right" way to do this as of right now (using an Animated.View around the Image, as there's a bug when animating the image directly. Didn't realize we were doing this the whole time...would be curious to profile before/after and see if it was indeed reloading when doing some of the resize operations (probably was eating CPU)

haileyok · 2024-10-29T18:03:02Z

src/view/com/lightbox/ImageViewing/components/ImageItem/ImageItem.ios.tsx

-          enableLiveTextInteraction={showControls && !scaled}
-        />
+        <Animated.View style={[styles.imageScrollContainer, animatedStyle]}>
+          <ActivityIndicator size="small" color="#FFF" style={styles.loading} />


Do we want to use Loader instead to keep consistency with what we do elsewhere? No strong opinion, whatever you think looks nicer in this context.

would rather not touch actual UI for now to stay safe, but can do later

* origin/main: (213 commits) add Thai Language translation support (#5879) fix warning on labeler profile: emoji detected but emoji not enabled (#6011) Added blur to search's onPressMenu (#6017) React compiler beta and reenable rule (#5898) Sort imports (#6009) Clarify build instructions (#6008) Upgrade all tiptap deps to latest (#5232) width full on text container (#6007) Bump 1.94.0 (#6006) Add subtle web hover to interactive rows (#5989) Settings revamp (#5745) Show almost-instant preview when opening lightbox (#6000) Refactor lightbox model to plain object (#5999) temp revert to old modal (#6005) Extend composer checks to all posts in a thread (#5955) Remove indirection when rendering composer state (#5954) Refactor composer state for threads (#5945) Disable Post button when empty (#5953) fix splash screen (#5997) Fix video quality for short videos (#5996) ...

* Plumb thumbUri down to the lightbox * Remove onLoad tracking from lightbox * Hook up placeholder URI to the image * Fix NaN causing crash on double tap while offline * Protect against NaNs in the future

gaearon commented Oct 29, 2024

View reviewed changes

haileyok reviewed Oct 29, 2024

View reviewed changes

gaearon mentioned this pull request Oct 29, 2024

Measure tapped image coordinates before opening lightbox #6001

Merged

haileyok reviewed Oct 29, 2024

View reviewed changes

haileyok approved these changes Oct 29, 2024

View reviewed changes

Base automatically changed from images-lightbox-1 to main October 29, 2024 20:58

gaearon added 5 commits October 29, 2024 20:59

Plumb thumbUri down to the lightbox

d112928

Remove onLoad tracking from lightbox

b1958ba

Hook up placeholder URI to the image

fd69760

Fix NaN causing crash on double tap while offline

ced0cbc

Protect against NaNs in the future

bba89d3

gaearon force-pushed the images-lightbox-2 branch from ab1e1e7 to bba89d3 Compare October 29, 2024 20:59

arcalinea temporarily deployed to images-lightbox-2 - social-app PR #6000 October 29, 2024 20:59 — with Render Destroyed

gaearon merged commit ab492cd into main Oct 29, 2024
6 checks passed

gaearon deleted the images-lightbox-2 branch October 29, 2024 21:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Show almost-instant preview when opening lightbox #6000

Show almost-instant preview when opening lightbox #6000

gaearon commented Oct 29, 2024 •

edited

Loading

gaearon Oct 29, 2024

gaearon Oct 29, 2024

gaearon Oct 29, 2024

github-actions bot commented Oct 29, 2024 •

edited

Loading

haileyok Oct 29, 2024

haileyok Oct 29, 2024

gaearon Oct 29, 2024

Show almost-instant preview when opening lightbox #6000

Show almost-instant preview when opening lightbox #6000

Conversation

gaearon commented Oct 29, 2024 • edited Loading

Before

After

Test Plan

gaearon Oct 29, 2024

Choose a reason for hiding this comment

gaearon Oct 29, 2024

Choose a reason for hiding this comment

gaearon Oct 29, 2024

Choose a reason for hiding this comment

github-actions bot commented Oct 29, 2024 • edited Loading

haileyok Oct 29, 2024

Choose a reason for hiding this comment

haileyok Oct 29, 2024

Choose a reason for hiding this comment

gaearon Oct 29, 2024

Choose a reason for hiding this comment

gaearon commented Oct 29, 2024 •

edited

Loading

github-actions bot commented Oct 29, 2024 •

edited

Loading