This is the first time I have ever worked with Expo, React Native, and even used native-esque debug in Xcode. So I apologize if anything I saw here is grossly misconstrued. I am hoping to continue learning the next few months and I thought this might be a good issue to tackle.
Problem statement
I observed a few days ago that we had a redundant logic condition in the image item components are what I believe is loading logic:
The first function "prepare" calculates some value (in this case a boolean) based on whether in the current state of the app the view has finished animating and the image has not loaded yet. This defines the "current value" on first run, called show. Per the API docs the previous value defaults to null and is thus falsy.
So for the first stage we can assume the image is NOT loaded yet and the animation is likely not done yet
show = false | prevshow = null (false)
Since both conditions are false showloader remains false
For second stage I believe animation of the Lightbox "settles" however at this point the image may still not be loaded for a variety of reasons (slow connection, etc). Therefore the prepare function will now show true.
show = true | prevshow = false (previous show value)
Per the current logic, this sets the loader to the first if condition, which keeps the setter value in useState to false.
For the third stage the image has loaded. Our prepare function now flips to false since hasLoaded is now true
show = false | prevshow = true
Neither condition in the current logic evaluates to true for the current logic. So the loader remains false.
Proposed solution
I think the intent of this code is follow:
For the second stage I believe what actually should happen is when the animation has "settled" rendering, but we have not finished loading we want to set the boolean from prepare to be true. At the same time we should ensure that the prevShow is false because we really only want to load if the image has never loaded before.
therefore we can consolidate the logic condition to simply
Stage 3 then occurs after and since show becomes false the setter sets the loader back to false
Testing wise
This is where I honestly struggled a bit. I did manage to get expo running on a Xcode simlulator. I scrolled my feed and clicked many images to expand to full screen. I was able to see the image appear and dismiss (by closing the box in the upper right corner)
I hope this was enough detail. If I have misunderstood the intent of this code please let me know. Thank you so much for everything the Bsky team has done so far!
Closes #6499
Preface
This is the first time I have ever worked with Expo, React Native, and even used native-esque debug in Xcode. So I apologize if anything I saw here is grossly misconstrued. I am hoping to continue learning the next few months and I thought this might be a good issue to tackle.
Problem statement
I observed a few days ago that we had a redundant logic condition in the image item components are what I believe is loading logic:
Here is what I think this code intends to do reference to docs for this OSS
The first function "prepare" calculates some value (in this case a boolean) based on whether in the current state of the app the view has finished animating and the image has not loaded yet. This defines the "current value" on first run, called show. Per the API docs the previous value defaults to null and is thus falsy.
So for the first stage we can assume the image is NOT loaded yet and the animation is likely not done yet
show = false | prevshow = null (false)
Since both conditions are false showloader remains false
For second stage I believe animation of the Lightbox "settles" however at this point the image may still not be loaded for a variety of reasons (slow connection, etc). Therefore the prepare function will now show true.
show = true | prevshow = false (previous show value)
Per the current logic, this sets the loader to the first if condition, which keeps the setter value in useState to false.
For the third stage the image has loaded. Our prepare function now flips to false since hasLoaded is now true
show = false | prevshow = true
Neither condition in the current logic evaluates to true for the current logic. So the loader remains false.
Proposed solution
I think the intent of this code is follow:
For the second stage I believe what actually should happen is when the animation has "settled" rendering, but we have not finished loading we want to set the boolean from prepare to be true. At the same time we should ensure that the prevShow is false because we really only want to load if the image has never loaded before.
therefore we can consolidate the logic condition to simply
Stage 3 then occurs after and since show becomes false the setter sets the loader back to false
Testing wise
This is where I honestly struggled a bit. I did manage to get expo running on a Xcode simlulator. I scrolled my feed and clicked many images to expand to full screen. I was able to see the image appear and dismiss (by closing the box in the upper right corner)
I hope this was enough detail. If I have misunderstood the intent of this code please let me know. Thank you so much for everything the Bsky team has done so far!