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.

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

Bug with navController.popBackStack() in 0.15.0-SNAPSHOT #128

Closed
wizarlock opened this issue Jan 10, 2025 · 25 comments
Closed

Bug with navController.popBackStack() in 0.15.0-SNAPSHOT #128

wizarlock opened this issue Jan 10, 2025 · 25 comments

Comments

@wizarlock
Copy link

wizarlock commented Jan 10, 2025

Hi. I found another interesting problem.
I'm using zoomable-image-coil3 0.15.0-SNAPSHOT, since you recently fixed my problem in this version. But I came across a new problem. When I switch to a new screen and then do navController.popBackStack() to the photo, a bug occurs (the photo cannot load). At the same time, if I make a zoom or offset gesture on the screen, it appears. But the offset is clearly violated...
Everything works fine in version 0.14.0.

SVID_20250110_131130_1.mp4

There is a bug in version 0.15.0-SNAPSHOT

SVID_20250110_131226_1.mp4

If the loading indicator is confusing, then without it

SVID_20250110_134020_1.mp4

Do you have any ideas?

@wizarlock
Copy link
Author

wizarlock commented Jan 10, 2025

It doesn't depend on the elements on photo at all. For some reason navController.popBackStack() just broke in version 0.15.0-SNAPSHOT. I can also say that sometimes the photo is displayed, and sometimes it is not. Here is the video

SVID_20250110_140633_1.mp4
SVID_20250110_140017_1.mp4

@wizarlock wizarlock changed the title Bug in 0.15.0-SNAPSHOT that occurs on an Honor phone Bug with navController.popBackStack() in 0.15.0-SNAPSHOT that occurs on an Honor phone Jan 10, 2025
@wizarlock wizarlock changed the title Bug with navController.popBackStack() in 0.15.0-SNAPSHOT that occurs on an Honor phone Bug with navController.popBackStack() in 0.15.0-SNAPSHOT Jan 10, 2025
@saket
Copy link
Owner

saket commented Jan 11, 2025

Can I use https://github.com/wizarlock/TestTelePhoto for reproducing this?

@wizarlock
Copy link
Author

wizarlock commented Jan 11, 2025

You can try, but there is no transition to the screen after the photo

@wizarlock
Copy link
Author

Or do you want me to reproduce this problem in this repository? I don't quite get it.

@saket
Copy link
Owner

saket commented Jan 11, 2025

I essentially need a project to reproduce the bug. Are you using https://github.com/wizarlock/TestTelePhoto in your videos?

@wizarlock
Copy link
Author

No. Then please wait. I will complete this project soon and let you know.

@saket
Copy link
Owner

saket commented Jan 11, 2025

Thank you, I'll take a look this weekend

@wizarlock
Copy link
Author

wizarlock commented Jan 11, 2025

sorry, i deleted comment, sec. I am confused and don't really understand yet what exactly is causing this behavior. I am investigating this.

@wizarlock
Copy link
Author

wizarlock commented Jan 13, 2025

Hi @saket! It really seems to me that something is wrong. I launched my project on Pixel_3a_API 34_extension_level7_x86_64. Look at this

TestTelePhoto.build.gradle.kts._app.2025-01-13.13-46-49.mp4

I'm using this https://github.com/wizarlock/TestTelePhoto

As you can see, on version 0.15.0 the behavior is strange: either the offset is wrong, or the photo doesn't load at all and you have to swipe the screen to load it. But on 0.14.0 everything is fine.

@wizarlock
Copy link
Author

wizarlock commented Jan 15, 2025

Did you catch the bug?

@saket
Copy link
Owner

saket commented Jan 22, 2025 via email

@wizarlock
Copy link
Author

wizarlock commented Jan 22, 2025

Hi! Sure, have a nice trip! Have a good rest. I'll wait for news about the bug

@wizarlock
Copy link
Author

Any progress? Need help catching it or something?

@saket
Copy link
Owner

saket commented Jan 31, 2025

Unfortunately not. I wasn't able to reproduce this on your branch. Here's what I'm seeing:

Screen_recording_20250130_223507.mp4

Do you have any other changes in your project that might not be staged?

@wizarlock
Copy link
Author

wizarlock commented Jan 31, 2025

Hi. Thanks for answering! Please try coil3 "zoomable-image-coil3" (now in prj coil). And don't run it on your phone. This problem doesn't always occur and not on all phones. But it happens quite often on Pixel_3a_API 34_extension_level7_x86_64. This is an emulator in Android Studio. And for some reason I can't catch it on a simple coil as easily.
This bug is still relevant, if you can't catch it - I'll find a better example. Just let me know

@saket
Copy link
Owner

saket commented Feb 2, 2025

That worked. Let me investigate what might be happening.

@wizarlock
Copy link
Author

I'm glad!

@wizarlock
Copy link
Author

Just to clarify: "zoomable-image-coil" also has the same problem on version 0.15.0-SNAPSHOT. I just can't reproduce it properly for some reason. On 0.14.0 both coil versions work correctly. That is, the matter is in the changes of the snapshot.

@wizarlock
Copy link
Author

Have you figured out what causes this interesting problem?

@saket
Copy link
Owner

saket commented Feb 8, 2025 via email

@wizarlock
Copy link
Author

wizarlock commented Feb 8, 2025

Thanks for the ways on how to fix the error. Okay, I'll use them for now, but I hope there will be a fix

@wizarlock
Copy link
Author

wizarlock commented Feb 9, 2025

Maybe I didn't quite understand you, sorry. Pasting these lines into my code didn't change anything. Could you please explain where I am wrong? Or how can i disable state restoration?

 LaunchedEffect(Unit) {
       zoomableState.resetZoom(animationSpec = SnapSpec())
 }

Sorry for the possibly stupid question, I just don't understand this topic very well, it turns out that the solutions you propose will make it so that when returning to the screen, the information about the previous value of offset and zoom will be lost? That is, I will return to the initial state of the image?

@saket saket closed this as completed in 2f8a08b Feb 12, 2025
@saket
Copy link
Owner

saket commented Feb 12, 2025

Alright that is no longer needed. I've disabled #104 and added a test to ensure this doesn't break again. Could you try out the newest snapshot to see if the issue is fixed?

Temporarily disable retaining of pan across image changes

@wizarlock
Copy link
Author

Thank you, everything works

@saket
Copy link
Owner

saket commented Feb 13, 2025

Thanks for reporting this critical bug!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants