search

woocommerce / woocommerce-android

WooCommerce Android app
https://www.woocommerce.com/mobile
GNU General Public License v2.0
277 stars 134 forks source link

Infinite Order Detail Loading Screen #12142

Closed malinajirka closed 2 weeks ago

malinajirka commented 1 month ago

Describe the bug If I rotate my device during the order creation, I end up with an Order Detail screen with an infinite loading on my backstack.

To Reproduce Steps to reproduce the behavior:

  1. Go to Order List
  2. Start Order Creation
  3. Add any product
  4. Rotate the device
  5. Rotate it back
  6. Tap on Create
  7. Notice you are redirected to the newly created order
  8. Press back and notice an infinite loading screen

Screenshots If applicable, add screenshots to help explain your problem.

https://github.com/user-attachments/assets/d2ab31bc-fce4-4b18-8f47-a0b546d10b68

Expected behavior Tapping back from the detail of the newly created order should navigate me back directly to the order list.

Mobile Environment Please include:

dangermattic commented 1 month ago

Thanks for reporting! 👍

malinajirka commented 1 month ago

I spent a few hours on this, but unfortunately I wasn't able to reliable fix this. The logic which determines how to display the order detail is quite complex and not encapsulated, which makes it tricky to orient in. I attempted to refactor the code so the OrderDetail is always opened from the OrderList no matter the state and starting destination. This seemed to work fine, but in rare cases introduced different issues - eg. navigation to the PaymentSelectionScreen was sometimes ignored by the NavigationComponent for some reason.

I also noticed that the OrderListFragment is recreated way too often, which is not expected - if it's always in the phone layout, it works as expected and a single instance is re-used. However, when it enters the two-pane layout, it gets recreated after the app exits the OrderCreation => it should keep the original instance, but for some reason it doesn't.

Here is my patch of WIP/POC of the fix. It somewhat works, but introduces different issues. WIP_-_Attempt_at_fixing_issue_12142.patch

Considering the above, I don't believe this issue is worth fixing at the moment - it's quite rare as it happens only on devices which switch between single-pane and two-pane layouts. Moreover, it happens only when the user rotates their screen back and forth. The issue is not a crash and doesn't block the user - it displays an infinite loading which can be dismissed.

samiuelson commented 2 weeks ago

👋🏼 @malinajirka, thanks for summarizing your research, and providing the context.

I also noticed that the OrderListFragment is recreated way too often, which is not expected - if it's always in the phone layout, it works as expected and a single instance is re-used. However, when it enters the two-pane layout, it gets recreated after the app exits the OrderCreation => it should keep the original instance, but for some reason it doesn't.

I realize it's been a while since you looked into this, but I'm curious how you discovered the OrderListFragment being recreated too often.

I spent some time debugging and observed that in case the OrderListFragment is not at the top of the fragments stack (e.g. the OrderCreateEditForm fragment is the currently visible one), OrderListFragment is not recreated on config change (e.g. onCreateView, onDestroyView are not called), however, its Activity hooks (e.g. onSavedInstanceState, onCreate) are called (despite the Fragment itself not being recreated). I think it's a default behavior of lifecycle hooks though.

As far as I understand, the fact that onSavedInstanceState hook is invoked in the OrderListFragment, despite the fact that it's not visible, was the root cause of the bug in Fragment's logic managing the visibility of screen panes.

malinajirka commented 2 weeks ago

I don't recall to be honest, I believe I checked the memory address of the fragment which shouldn't change. However, I might have gotten confused by the onCreate behavior, I can't tell for sure. However, I'm not sure if it would explain explain why the fragment's oncreate would be invoked on a phone which switches between single/two pane layouts but isn't invoked when the phone stays on a single pane layout in both landscape/portrait?

Unfortunately, I don't have capacity to look into this anytime soon. So if you haven't noticed any issues, let's just ignore that part. Thanks for working on the fix for the infinite loading!