woocommerce / woocommerce-ios

WooCommerce iOS app
https://www.woocommerce.com/mobile
GNU General Public License v2.0
258 stars 108 forks source link

Shipping Labels: fix navigation title showed in TabBar in Package List #3995

Closed pmusolino closed 2 years ago

pmusolino commented 3 years ago

As found by @rachelmcr here https://github.com/woocommerce/woocommerce-ios/pull/3967 there is a bug in the Package List screen implemented in SwiftUI. It seems that the navigation title is showed also in the tab bar as UITabBarButtonLabel. It is probably caused by the fact that the SwiftUI view is originally presented by a View Controller.

Schermata 2021-04-19 alle 17 08 12

pmusolino commented 3 years ago

After a more in-depth investigation, the only way I've found to solve it effectively is to embed the second view in the UIHostingController as well and make the navigation happen between the view controllers. This is clearly not a "lightweight" solution, because it would also mean exposing the model data to the view controllers, and finding a way to get the views in SwiftUI to communicate with the view controllers.

I continue to investigate.

Ecarrion commented 3 years ago

Do we know why it happens? Or is that still a mystery?

pmusolino commented 3 years ago

No idea why it happens. It's like if there is a conflict with our TabBar. The text remains inside the TabBar also on screens that are not related to SwiftUI, and I wasn't able to find someone else with the same issue. @Ecarrion

Ecarrion commented 3 years ago

@pmusolino I spent a little bit on this and got nowhere 🤷

However, I noticed that we could mitigate this if we hide the TabBar when pushing the shipping label creating flow 🤷 🤷

I think we should try to properly fix it, but if not, We can go with the workaround. What do you think?

Branch: https://github.com/woocommerce/woocommerce-ios/compare/issue/3995-possible-fix?expand=1 Demo:

https://user-images.githubusercontent.com/562080/116469021-37eeda00-a837-11eb-8262-a51321c98dbc.mov

pmusolino commented 3 years ago

Thank you @Ecarrion for spending time investigating this issue. 🙇‍♂️

However, I noticed that we could mitigate this if we hide the TabBar when pushing the shipping label creating flow

Unfortunately, this is not a solution, because when you come back to the Order Detail, and the tab bar re-appear, you can see the bug. :(

pmusolino commented 3 years ago

Found someone with a similar issue: https://developer.apple.com/forums/thread/677559

Ecarrion commented 3 years ago

@pmusolino What a weird bug!

I may have a workaround for my proposed workaround. If we set the navigation controller title to an empty string, the label that is magically created in the tab bar item still exists but now has empty content. That + hiding the tab bar could hide the issue to the user.

Another option could be to present the shipping label flow in a modal presentation context. That would also hide the issue as we won't have a tab-bar anymore.

In any case, I think we should use one of those Apple Calls to inform them, and maybe we could get a proper solution.

What do you think?

https://user-images.githubusercontent.com/562080/116560740-4a185900-a8c7-11eb-9629-d0267ad38186.mov

Ecarrion commented 3 years ago

Forgot: This is the branch with the updated code.

pmusolino commented 3 years ago

I did a test also with the methods for setting the title of the SwiftUI view available only on iOS 14, but with no luck.

@Ecarrion thanks for the workaround!

In any case, I think we should use one of those Apple Calls to inform them, and maybe we could get a proper solution.

I think that we can fill directly a radar, but if you think that it's more useful to discuss with them why it's happening (maybe they have a solution we didn't take into consideration), I'll open a discussion with them. By the way, we are near to finish this Shipping Labels Milestone, but at the same time, it will not be visible to users until we complete Milestone 3. So I think we can leave this issue open, and in case we haven't been able to find a definitive solution, we can use your workaround. Sound good to you?

Ecarrion commented 3 years ago

Sure, as long as the bug is not visible to the user I'm 👍

pmusolino commented 3 years ago

Thank you so much for helping me investigating this issue. I'm going to fill a radar 👍

pmusolino commented 3 years ago

Filled a radar with this reference: FB9092390 (Extra UITabBarButton added to UITabBar when setting the navbar title of a SwiftUI view)

nutsmuggler commented 3 years ago

Hey, I was directed here from my post in the developer forum. I solved the UX problem in a different way; I basically traverse the tab bar view hierarchy and remove extra views. I really don't like this fix, but it seems like the most lightweight way to work around the issue. If you're interested, here is the function:

https://developer.apple.com/forums/thread/677559?page=1#673640022

Is there a way to see Apple's feedback on your radar? I wasn't able to find it on OpenRadar and am still a bit confused by Feedback Assistant. Cheers!

pmusolino commented 3 years ago

Thank you @nutsmuggler for your suggestion, but I don't think that this workaround works for us, because it can be a little bit weak. Unfortunately, there is no way to see a feedback sent to Apple :( I'm sorry.

pmusolino commented 2 years ago

Apple replied after a few months: Please verify this issue with the Xcode 13 Beta 5 and update your bug report with your results by logging into https://feedbackassistant.apple.com/ or by using the Feedback Assistant app.

Xcode 13 Beta 5 (13A5212g) https://developer.apple.com/download/

We need to try if this issue still persists.