search

woocommerce / woocommerce-ios

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

Shipping Labels: impossible to digit more than two or three digits in phone number #5051

Closed pmusolino closed 2 years ago

pmusolino commented 2 years ago

I discovered a serious bug (a regression) in Shipping Labels. This happens both on International and local shipping labels. It seems that if the Ship From address has the phone missing, it's impossible to add a phone number with more than 3 digits.

After typing 3 digits, the keyboard will be dismissed, the field is no more selectable and the done button will be enabled but by pressing it nothing will happen.

This issue happens also on Ship To address form, but the limit of digits will be 2 instead of 3.

The only way to remedy this problem is to copy and paste a number with ten digits.

Video:

https://user-images.githubusercontent.com/495617/134660825-0a2a8623-2b60-4d7b-9f13-9084da78b261.mp4

itsmeichigo commented 2 years ago

It seems that this bug only happens when building the app with Xcode 13.

itsmeichigo commented 2 years ago

I can also reproduce the frozen screen by just tapping Done several times. I haven't yet found the cause for the issue though.

pmusolino commented 2 years ago

@itsmeichigo just with Xcode 13? 😮 This is interesting...

rachelmcr commented 2 years ago

Thanks @itsmeichigo! I hadn't tried with an Xcode 13 build and I do see the issue that way. I see the frozen screen when I change any of the address form fields (not just the phone number field).

In my investigation, it looks like the issue is linked to reloading the table data:

https://github.com/woocommerce/woocommerce-ios/blob/0cbf4c74fc0e2a7234cae7f7e148e064b0e9c6bc/WooCommerce/Classes/ViewRelated/Orders/Order%20Details/Shipping%20Labels/Create%20Shipping%20Label%20Form/Shipping%20Address%20Validation/ShippingLabelAddressFormViewController.swift#L154

I haven't gotten farther than that, but removing that line avoids the freeze. (I might be mistaken about that, but if that is the cause of the freeze, we'll need to find a different way to update the form to display or hide validation errors.)

Since we aren't building releases with Xcode 13 yet, I think this doesn't have to be high priority/hotfixed right away, so I'll prioritize the feature work before investigating this one further.

itsmeichigo commented 2 years ago

@rachelmcr thanks for finding where the issue lies! I think this might be caused by my fix for the instant validation in #4973.

However I can't seem to reproduce the frozen screen when build with Xcode 12 so I can't confirm that. Can you clarify how you got the issue to happen?

rachelmcr commented 2 years ago

However I can't seem to reproduce the frozen screen when build with Xcode 12 so I can't confirm that. Can you clarify how you got the issue to happen?

I'm also unable to reproduce this with an Xcode 12 build; it only happens when building with Xcode 13. That's why I'm a little unsure about the actual cause.

itsmeichigo commented 2 years ago

I've done a few more tests and it seems that the text fields are frozen after reloading the table view a few times. I'm not sure if there's a workaround for this issue - if not, maybe we should consider another way to work with the validation error rows. I think there are 2 options:

pmusolino commented 2 years ago

Converting the entire view to SwiftUI might be too much work, and I think it would also be hard not to miss anything on the way. Nothing is impossible, but I think it would be better to try to fix the bug on the current view controller. Could be something related to the first responder of the cell? It seems that it's the only cell that became unresponsive after an error.