Magic code blur #28711

wojtus7 · 2023-10-03T11:03:19Z

Details

This PR is a direct continuation of the PR that got reverted here.

It should his two release blockers:
#28494
#28590

Additionally resolve issue:
#26090

Description:
In order to make blur possible I needed to refactor the magic component. Instead of 6 separate text inputs there is only one wrapped inside TapGestureHandler that determine which number should be inputed. This resulted is some code that is simpler, taps are handled by gesture handler and right-click/context menu/context is handled by native text input. but at the same time created small issue that paste button shows only on the beginning of the input.

Fixed Issues

$ #18244
PROPOSAL: #18244 (comment)

Tests

Log out
Provide email address
Try to break the magic code input (Click at random places, use arrows, backspace, change tabs and go back on web etc)

Verify that no errors appear in the JS console

Offline tests

QA Steps

Log out
Provide email address
Try to break the magic code input (Click at random places, use arrows, backspace, change tabs and go back on web etc)

Verify that no errors appear in the JS console

PR Author Checklist

Screenshots/Videos

Web

bez.nazwy.mov

Mobile Web - Chrome

Screenrecorder-2023-08-01-17-22-47-275.mp4

Mobile Web - Safari

Nagranie.z.ekranu.2023-08-1.o.14.42.25.mov

Desktop

bez.nazwy.mov

iOS

Nagranie.z.ekranu.2023-08-1.o.14.44.35.mov

Android

Nagranie.z.ekranu.2023-08-1.o.14.41.23.mov

melvin-bot · 2023-10-03T11:03:24Z

@Santhosh-Sellavel Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button]

wojtus7 · 2023-10-03T11:18:00Z

@dangrous @Santhosh-Sellavel

Santhosh-Sellavel · 2023-10-04T18:43:42Z

@wojtus7 What's the new fix for the two blockers, can you explain for me please?

wojtus7 · 2023-10-05T12:16:44Z

@Santhosh-Sellavel

Issue #28494:

So the problem here is that the code you get from SMS is not pasted but very rapidly inputed on the keyboard. In this implementation of magic input I get the value and clear the input after user type anything. If user paste anything I get 6 digits and use it and then clear input. The way iOS rapidly input new digits from SMS is breaking because it inputs the values so fast that react doesn't have time to clear the input because useState is async and takes a little time. So for code: 123456 the input in magic code was: 112123 because every change of the input was 1 -> 12 -> 123 -> 1234 and every state of the input was interpreted as pasted code.

The answer was to create new ref const lastValue = useRef(); that will hold last input value and remove the shared beginning. If the code would be 123456 then for 1 -> 12 it would recognise the clear of the input did not happen and remove it from the string and just use 2.

At the same time we can't just not clear the input because the paste button on mobile devices will show in random places on the input not just on the first space :(

Answer for #28590 was just add focus() after modal closure

Santhosh-Sellavel · 2023-10-10T17:26:16Z

@wojtus7 Sorry for the delay, please resolve conflicts!

Santhosh-Sellavel · 2023-10-16T16:37:15Z

bump @wojtus7

dangrous · 2023-10-17T20:36:19Z

hi, just letting y'all know I'm back from OOO and happy to review this whenever!

dangrous · 2023-10-23T15:33:47Z

@wojtus7 I'd love to get this merged this week - do you have time to resolve conflicts and get this ready to review again? Thank you!

wojtus7 · 2023-10-23T17:01:09Z

Hi @dangrous Today I'm first day back after being OOO for 2 weeks. I started to work on it right away and have all conflicts resolved but my GPG stopped working after not using Mac for some reason and trying to figure it out. Working on it right now, will push the changes whenever it's resolved and will do my best to make this PR merge this week!

wojtus7 · 2023-10-23T17:30:39Z

@Santhosh-Sellavel all conflicts resolved!

dangrous · 2023-10-24T14:38:26Z

Thank you @wojtus7 ! @Santhosh-Sellavel let us knwo if you have time to review today or tomorrow - thank you!

Santhosh-Sellavel · 2023-10-25T18:44:10Z

TestFails on Android.

The keyboard disappears immediately when magic code input is presented

Screen.Recording.2023-10-26.at.12.08.38.AM.mov

Santhosh-Sellavel · 2023-10-25T18:56:22Z

This #28590 is not resolved

Screen.Recording.2023-10-26.at.12.24.52.AM.mov

Santhosh-Sellavel · 2023-10-25T19:25:54Z

@wojtus7
#26090 This is not resolved either

Screen_Recording_20231026_004910_New.Expensify.Dev.mp4

🔴 🔴 While testing found even a weirder case was able to enter more than 6 numbers but it changed only the last digit and the code was not submitted at all.

Screen_Recording_20231026_005218_New.Expensify.Dev.mp4

cc: @dangrous

wojtus7 · 2023-10-25T22:07:34Z

Hmm. I guess some things break after the main merge or I am going crazy 😞
I'm looking at thise things right now

@Santhosh-Sellavel about:
"🔴 🔴 While testing found even a weirder case was able to enter more than 6 numbers but it changed only the last digit and the code was not submitted at all."

I think this is caused by and the auto submit feature being turn off after one time:
#29731

wojtus7 · 2023-10-27T18:02:14Z

TestFails on Android.

The keyboard disappears immediately when magic code input is presented

Screen.Recording.2023-10-26.at.12.08.38.AM.mov

I think this is more complex than that. I tested it a lot and it happen in a lot of places and not only in the way you described but also can happen when go back a screen then the email input fails, and sometimes the error doesn't happen at all.
Trying to fix it locally in magic code but maybe this is some navigation issue?

Nagranie.z.ekranu.2023-10-27.o.19.59.15.mov

Santhosh-Sellavel · 2023-11-15T18:00:20Z

@wojtus7 @dangrous @kosmydel

Mostly looks good. While testing today I ran into this issue while testing on Android Alone.

Tested this issue #28590. Focus is set correctly but the keyboard is not shown on the Android

Screen.Recording.2023-11-15.at.11.23.43.PM.mov

This is NAB as it is specific to a platform and can be handled as a follow-up.

Basic cases pass for me, so I'm good with approving this. But I couldn't test this issue on iOS Simulator can you test this please @dangrous iPhone, thanks!

dangrous · 2023-11-15T19:27:09Z

Oh hm I'm not sure I can test that even with a physical iPhone since I need it to text me and I don't think it will... Let me investigate.

dangrous · 2023-11-15T19:36:09Z

In the meantime, if we CAN find a fix for the keyboard thing you mentioned that would be ideal, but yeah I don't think it's a blocker

BaseValidateCodeForm.js

kosmydel · 2023-11-16T11:11:16Z

In the meantime, if we CAN find a fix for the keyboard thing you mentioned that would be ideal, but yeah I don't think it's a blocker

I've fixed this issue here, pulled main & resolved conflicts.

Video:

fix-android.mov

I've tested the magic code, but I couldn't test the OTP auto-fill functionality (I can't send text messages to the testing devices that I have).

cc @Santhosh-Sellavel for another review

dangrous · 2023-11-17T17:00:20Z

Okay I managed to set up getting texts on my physical iOS device.

BUT - both on staging, main, and this branch - the phone does not try to autofill from a text anymore - I don't get the prompt. Can one of you possibly try on staging and see if it's my phone or something else? It does look like the messaging changed since that bug was filed, so that could be it.

If you can't, I can ask around internally, too. Thanks!

Santhosh-Sellavel · 2023-11-17T17:42:54Z

@dangrous I can't test this case alone, please ask internally.

dangrous · 2023-11-17T18:22:43Z

Okay, I figured it out! Seems to work well.

kosmydel · 2023-11-21T09:56:24Z

Hey, when can I expect another round of review? @Santhosh-Sellavel @dangrous

dangrous · 2023-11-21T15:50:23Z

@Santhosh-Sellavel are you able to test/review again? The iOS issue seems to be solved with this PR.

Santhosh-Sellavel · 2023-11-21T18:23:52Z

AFK today, will do tomorrow.

Santhosh-Sellavel · 2023-11-23T13:23:12Z

On it now again!

Santhosh-Sellavel

LGTM, tests well!

dangrous

Two minor clarifications, otherwise this is looking good!

dangrous · 2023-11-27T16:14:37Z

src/components/MagicCodeInput.js

+            setInput(TEXT_INPUT_EMPTY_STATE);
+            setFocusedIndex(newFocusedIndex);
+            setEditIndex(newFocusedIndex);


We use these three lines in a number of places - might it be good to separate them out into a helper function?

dangrous · 2023-11-27T16:15:33Z

src/components/MagicCodeInput.js

@@ -181,9 +233,12 @@ function MagicCodeInput(props) {
            return;
        }

+        const addedValue =


Can you add a comment to explain this? It's very confusing to look at, haha. Also some parentheses around the boolean section might help.

dangrous

love it!

dangrous · 2023-11-28T16:33:49Z

Okay, going to merge, let's be sure to watch this for bugs but I think we're going to be good this time.

situchan · 2023-11-29T14:29:22Z

src/pages/settings/Profile/Contacts/ContactMethodDetailsPage.js

@@ -264,6 +264,11 @@ class ContactMethodDetailsPage extends Component {
                        title={this.props.translate('contacts.removeContactMethod')}
                        onConfirm={this.confirmDeleteAndHideModal}
                        onCancel={() => this.toggleDeleteModal(false)}
+                        onModalHide={() => {
+                            InteractionManager.runAfterInteractions(() => {
+                                this.validateCodeFormRef.current.focusLastSelected();


This caused crash. Now deploy blocker - #32181
@wojtus7 are you able to raise quick PR?

I can handle the fix if need be but if you're around @wojtus7 that would be great!

Hey, @wojtus7 is not here. I can help with this one in an hour or two.

I can help too )

Working on the PR

Thank you for jumping in @kosmydel! We'll be ready to review whenever

PR

I'm recording videos, however, I have a rate limit for adding new contact methods.

The PR is ready :)
cc @situchan @dangrous

OSBotify · 2023-11-30T12:14:48Z

🚀 Deployed to production by https://github.com/mountiny in version: 1.4.5-7 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

alitoshmatov · 2023-12-29T19:47:43Z

src/pages/settings/Profile/Contacts/ValidateCodeForm/BaseValidateCodeForm.js

    }));

    useFocusEffect(
        useCallback(() => {
            if (!inputValidateCodeRef.current) {
                return;
            }
+            if (focusTimeoutRef.current) {
+                clearTimeout(focusTimeoutRef.current);
+            }
            focusTimeoutRef.current = setTimeout(inputValidateCodeRef.current.focus, CONST.ANIMATED_TRANSITION);


focusLastSelected should have been used also here, like this:
focusTimeoutRef.current = setTimeout(inputValidateCodeRef.current.focusLastSelected, CONST.ANIMATED_TRANSITION);

This caused issue - #33170

wojtus7 added 2 commits October 2, 2023 19:37

Fix stuff

d1a7800

Merge remote-tracking branch 'origin/main' into magic-code-blur

f2a47aa

wojtus7 requested a review from a team as a code owner October 3, 2023 11:03

melvin-bot bot removed the request for review from a team October 3, 2023 11:03

melvin-bot bot requested a review from Santhosh-Sellavel October 3, 2023 11:03

Prettier

d50aae2

wojtus7 mentioned this pull request Oct 3, 2023

[HOLD for payment 2023-12-07] [$2000] magic code input remains highlighted even after not being focused #18244

Closed

6 tasks

dukenv0307 mentioned this pull request Oct 6, 2023

[$500] Re-entering same wrong magic code and verify keeps last digit in green #26813

Closed

6 tasks

samh-nl mentioned this pull request Oct 12, 2023

[$1000] Log in - User unable to proceed after re-enter last digit of magic code number #21958

Closed

6 tasks

wojtus7 force-pushed the magic-code-blur branch from ef8dea3 to d50aae2 Compare October 23, 2023 17:23

Merge remote-tracking branch 'origin/main' into magic-code-blur

e96e1dd

strepanier03 mentioned this pull request Oct 24, 2023

[HOLD] [$1000] Log in - Fast typing of login magic code omits digits #26090

Closed

6 tasks

wojtus7 added 2 commits October 27, 2023 15:30

Merge remote-tracking branch 'origin/main' into magic-code-blur

690d22a

Refocus input in basevalidatecodeform and remove wasSubmitted flag

2618870

kosmydel added 3 commits November 16, 2023 11:10

fix android keyboard

92f5be1

Merge branch 'main' into @kosmydel/magic-code-blur

09358c8

Refactor focusTimeoutRef.current in

98c04ab

BaseValidateCodeForm.js

Santhosh-Sellavel approved these changes Nov 23, 2023

View reviewed changes

melvin-bot bot requested a review from dangrous November 23, 2023 17:20

dangrous requested changes Nov 27, 2023

View reviewed changes

kosmydel added 2 commits November 28, 2023 16:08

add helper function

5c2a666

add comments

de8fee6

dangrous approved these changes Nov 28, 2023

View reviewed changes

dangrous merged commit 0c78c5b into Expensify:main Nov 28, 2023
15 checks passed

github-actions bot mentioned this pull request Nov 29, 2023

Deploy Checklist: New Expensify 2023-11-29 #32175

Closed

55 tasks

ZhenjaHorbach mentioned this pull request Nov 29, 2023

[HOLD for payment 2023-12-08] [HOLD for payment 2023-12-07] Contact method - App crashes when removing secondary contact method #32181

Closed

6 tasks

situchan reviewed Nov 29, 2023

View reviewed changes

lanitochka17 mentioned this pull request Nov 30, 2023

[$500] iOS - Login - "Autofill" is offered for magic code #32238

Closed

6 tasks

aimane-chnaif mentioned this pull request Nov 30, 2023

[$500] iOS - Login - Paste menu does not point to the correct field on magic code page #32241

Closed

6 tasks

alitoshmatov reviewed Dec 29, 2023

View reviewed changes

alitoshmatov mentioned this pull request Dec 29, 2023

[HOLD for payment 2024-01-02] [$500] Settings - Focus shifts to first digit on shortcut & back #33170

Closed

6 tasks

Magic code blur #28711

Magic code blur #28711

Conversation

wojtus7 commented Oct 3, 2023 • edited by github-actions bot Loading

Details

Fixed Issues

Tests

Offline tests

QA Steps

PR Author Checklist

Screenshots/Videos

melvin-bot bot commented Oct 3, 2023

wojtus7 commented Oct 3, 2023

Santhosh-Sellavel commented Oct 4, 2023 • edited Loading

wojtus7 commented Oct 5, 2023

Santhosh-Sellavel commented Oct 10, 2023

Santhosh-Sellavel commented Oct 16, 2023

dangrous commented Oct 17, 2023

dangrous commented Oct 23, 2023

wojtus7 commented Oct 23, 2023

wojtus7 commented Oct 23, 2023

dangrous commented Oct 24, 2023

Santhosh-Sellavel commented Oct 25, 2023 • edited Loading

Santhosh-Sellavel commented Oct 25, 2023

Santhosh-Sellavel commented Oct 25, 2023

🔴 🔴 While testing found even a weirder case was able to enter more than 6 numbers but it changed only the last digit and the code was not submitted at all.

wojtus7 commented Oct 25, 2023

wojtus7 commented Oct 27, 2023

Santhosh-Sellavel commented Nov 15, 2023

dangrous commented Nov 15, 2023

dangrous commented Nov 15, 2023

kosmydel commented Nov 16, 2023

dangrous commented Nov 17, 2023

Santhosh-Sellavel commented Nov 17, 2023

dangrous commented Nov 17, 2023

kosmydel commented Nov 21, 2023

dangrous commented Nov 21, 2023

Santhosh-Sellavel commented Nov 21, 2023

Santhosh-Sellavel commented Nov 23, 2023

Santhosh-Sellavel left a comment

Choose a reason for hiding this comment

dangrous left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dangrous left a comment

Choose a reason for hiding this comment

dangrous commented Nov 28, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

OSBotify commented Nov 30, 2023

alitoshmatov Dec 29, 2023 • edited Loading

Choose a reason for hiding this comment

wojtus7 commented Oct 3, 2023 •

edited by github-actions bot

Loading

Santhosh-Sellavel commented Oct 4, 2023 •

edited

Loading

Santhosh-Sellavel commented Oct 25, 2023 •

edited

Loading

alitoshmatov Dec 29, 2023 •

edited

Loading