-
Notifications
You must be signed in to change notification settings - Fork 987
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
Enter scan code tab in Syncing #16852
Conversation
Jenkins BuildsClick to see older builds (110)
|
07d4dd2
to
45f1b6f
Compare
Hello @siddarthkay! Thank you. ISSUES 1,2,3,4 are fixed At the same time I am still reproducing following issues ISSUE 5 text is not centered in the input fieldISSUE 6 Pasted qr code of a right format is not confirmed automatically |
ISSUE 8 Sometimes user gets stucked on Enter scan code tab after confirming valid QR codeActual result: Eventually user is redirected to Syncing screen. But sometimes it takes minutes before it happens (see video below). telegram-cloud-document-2-5208500277149185983.mp4Expected result: user should be redirected to Syncing screen right after Confirm button is tapped (in case if QR passes validation). |
@siddarthkay I am also facing some weird issues with IOS: sometimes Sync fails despite valid QR is used. The other time IOS appears to crash after successful sync. I will investigate those cases tomorrow and try to provide some details. |
9a858e7
to
1a87d2e
Compare
1a87d2e
to
f387290
Compare
@siddarthkay could you please have a look? |
ah yes at the moment I am working on fixes for issues pointed out by @pavloburykh |
@siddarthkay @pavloburykh |
Thanx @churik! Looks like ISSUE 8 is not reproducible in case of valid scan code. @siddarthkay could you please take a look at other issues ISSUE 9 IOS is crashing after sync performed by entering scan code (IOS)IOS 16.6, iPhone X Crash is reproducible every time but I am facing it in 8 of 10 cases. It is always reproducible suring first sync after app install Preconditions: User A - any device, QR generator; User B - IOS fresh install, not logged yet into any account Steps:
Actual result: User is redirected to Syncing screen. App crashes. After app re-opening synced account is displayed in the list and can be logged in, so sync is in fact successful. telegram-cloud-document-2-5231034621931570101.mp4Expected result: app does not crash. |
ISSUE 10 Sometimes user gets stucked on Syncing screen after using valid but expired codeIt takes a lot of time (> 1 min) before user is finally redirected to Oops something's wrong screen Steps:
Actual result: user is redirected to Syncing page and remain there for > 1 before Oops something's wrong screen appears telegram-cloud-document-2-5231034621931570101.mp4Expected result: user is redirected to Oops something's wrong faster |
@siddarthkay I would take a deeper look at ISSUE 9 while ISSUE 10 can be resolved in a followup. |
76b9ceb
to
57f5840
Compare
Hi @pavloburykh : I've addressed Issue 5 in this PR. Initially the scope of this PR was just to add an input field for connection strings to be accepted for syncing. Regarding remaining issues about crash and users being stuck will be worked on in a follow up. Thanks for testing and could you please have a look again ? |
Hi @siddarthkay. Unfortunately ISSUE 5 is still not fixed. IOS seems to be okay, but on Android placeholder and text is now dispositioned to the top. Below video recorded on Samsung Galaxy A52, Android 12. telegram-cloud-document-2-5251438323632714958.mp4Also, what about ISSUE 6? I have mentioned it among unfixed issues here |
This commit adds the enter sync code UI. Fixes : #16062 Design link : https://www.figma.com/file/V6nlpAWIf2e1XU8RJ9yQPe/Syncing-for-Mobile?node-id=675%3A179729&mode=dev Review notes : We do not make use of quo2/input here because currently that component does not allow stretching to fit in sync code which spans upto 3 lines.
57f5840
to
6cee942
Compare
Agreed to merge this PR and take care of the rest issues in followups. |
@Francesca-G we have skipped design review for this PR as according to @siddarthkay we need to merge it ASAP. You can review it after merge and share your comments but take into consideration that at this point we have some issues left as followups. So maybe it will be better to review after we fix them. Thank you. |
makes sense to review it after the fixes :) |
This PR adds the enter sync code UI.
Fixes : #16062
Design link : https://www.figma.com/file/V6nlpAWIf2e1XU8RJ9yQPe/Syncing-for-Mobile?node-id=675%3A179729&mode=dev
Screenshot :
Review notes :
We do not make use of
quo2/input
here because currently that component does not allow stretching to fit in sync code which spans upto 3 lines.status: ready