[Woo POS] M2: Fix extra padding appearing after removing the tab

staskus commented 1 month ago

Steps

Open Woo app (I use iPad Air 11 inch iOS 17.5 simulator)
Open Menu -> Point of Sale
Notice extra padding at the bottom
Rotate iPad / Click dots at the top
Notice it fixes

Probably we need to do some extra layout code when tabs are removed so the whole UI would be refreshed

Video

https://github.com/user-attachments/assets/005392d4-06e5-43d7-ac0e-d78690db3be9

dangermattic commented 1 month ago

Thanks for reporting! 👍

staskus commented 2 weeks ago

Started appearing again

https://github.com/user-attachments/assets/2c56cc40-5343-4969-8ba2-bf5ac136e423

iamgabrielma commented 1 week ago

👋 @staskus are you able to replicate this consistently with the iPad Air 11 inch iOS 17.5 simulator? I'm just asking because I'll need to update Xcode to test that specific iOS version.

I was unable to see this issue on the following devices:

Simulator: iPad Pro 12.9-inch - iOS 16.4
Simulator: iPad Air (5th gen) - iOS 17.0
Physical: iPad Pro 12.9-inch - iOS 17.6.1

staskus commented 1 week ago

@iamgabrielma yes, I get it all the time on Xcode 15.4, iOS 17.5 (21F79) iPad Air 11-inch (M2)

It looks like the previous fix that set tabBarController.selectedViewController?.view.layoutIfNeeded() is no longer working.

iamgabrielma commented 1 week ago

Alright, so I could finally replicate it with the same configuration 🎉

Xcode 15.4 (15F31d), iOS 17.5 (21F79) iPad Air 11-inch (I assume M1 since says nothing?)
Xcode 15.4 (15F31d), iOS 17.5 (21F79) iPad Air 11-inch (M2)

I'm not sure if is related, but notice how the initial loading screen moves up before loading the dashboard as well:

https://github.com/user-attachments/assets/223b9318-757a-4515-8ba5-385d78093f4a

It also doesn't seem to happen in subsequent runs of the POS (if we exit POS and re-enter), it only happens the first time we access after running the app.

staskus commented 1 week ago

I'm not sure if is related, but notice how the initial loading screen moves up before loading the dashboard as well:

@iamgabrielma yes, it's the same issue 👍 You can also workaround this issue by clicking on the three dots at the top of iPad when POS loads, then it forces a re-layout.

iamgabrielma commented 1 week ago

So far I've been unable to figure out how to resolve this. What I've tried:

Using setNeedsLayout() rather than layoutIfNeeded()
Hide the tabBar and re-layout within .onAppear and .onDisappear on the point of entry, dashboard, and other subviews
Confirmed that floatingControlAreaSize does not change its value unexpectedly when loading vs loaded
I've attempted to force view recreation in several places using .id(UUID()) and triggering new ID's upon view loading
Wrapped AppDelegate.shared.tabBarController?.view.layoutIfNeeded() on a Dispatchqueue, to assure that happens way after the tabBar has been hidden
Attempted to make the tabBar disappear from the frame via tabBarController.tabBar.frame = .zero
Attempt to hide the tabBar within the HubMenu, when handleTap(menu: HubMenuItem) is performed
Attempted to layout directly from the HubMenu's view controller via : tabBarController.hubMenuTabCoordinator?.hubMenuController?.tabBarController?.view.layoutIfNeeded()

Ultimately, what appears to happen is that SwiftUI considers the space of the hidden tabBar part of the available space when laying out the different views, and does not refresh this calculation until there's a size class update:

staskus commented 1 week ago

@iamgabrielma thanks for the investigation! It sure looks like a tough case. I wonder why the fix worked here [Woo POS] M2: Fix extra padding appearing after removing the tab and then suddenly stopped working 🤔 What changes we made in the last week or so could've caused this issue to re-appear.

iamgabrielma commented 1 week ago

What changes we made in the last week or so could've caused this issue to re-appear.

That's a good question, I've never seen this error until I ran iOS 17.5 specifically (still cannot reproduce it with 17.6.1 on physical device), so I wonder if the runtime used has also something to do with it. Just curious here: Does it happen on other devices/runtimes on your end? Or is specific to the iPad Air + iOS 17.5 config?

As good next step I'm thinking we could do a bisect to figure out some commits where this restarted to happen again.

staskus commented 1 week ago

Just curious here: Does it happen on other devices/runtimes on your end? Or is specific to the iPad Air + iOS 17.5 config?

@iamgabrielma haven't tested on other runtimes myself.

As good next step I'm thinking we could do a bisect to figure out some commits where this restarted to happen again.

My best guess is changes related to modals, since I think they affected overall layout setup.

iamgabrielma commented 1 week ago

I started to run a bisect but I couldn't wrap it up today due to a PR not compiling commits independently, for now the issue seems to point to this PR at the moment: https://github.com/woocommerce/woocommerce-ios/pull/13671

- checking 1512e3d3e50cb0b8ef7405b36120237637357234 good
- checking 36c453532379cd499420ebb05f344b279d85ef5c bad
- checking a8411a819763bc8867090c84cbc95a3b5e9fe76e bad
- checking 16b0fe4266cb3453cde5dfc4f475c685d8231fa7 bad
- checking 72293ca7c456fd13bd492b74ca4ee9c9d4132066 bad
- checking 04090ef892117b1cd1a001b12c36fd9188f12304 bad
- checking 77dc5952c562ca56470e51a89237e610603dd41a bad
- checking c76d37bbb1b8d6aec8682647f4dc53c281267d4b (does not compile - skipped)
- checking 4fe3e0a9b177685d46556845cd3140dba1b444cb (does not compile- skipped)
- checking 71df1bfa07ddd2527980408820f25f42fafac749 (does not compile- skipped)

I'll pick up from here tomorrow and see if I can narrow it down further.

woocommerce / woocommerce-ios

[Woo POS] M2: Fix extra padding appearing after removing the tab #13432

Steps

Video