Skip to content
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

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

Closed
4 of 6 tasks
kbecciv opened this issue Jun 30, 2023 · 95 comments
Closed
4 of 6 tasks
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor

Comments

@kbecciv
Copy link

kbecciv commented Jun 30, 2023

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Action Performed:

  1. Open Login Page
  2. Input email => Continue
  3. input wrong magic: example: 111111
  4. Click last input
  5. Input again 1
  6. input any number

Expected Result:

User able to proceed after re-enter last digit of magic code number

Actual Result:

User unable to proceed after re-enter last digit of magic code number

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

  • Android / native
  • Android / Chrome
  • iOS / native
  • iOS / Safari
  • MacOS / Chrome / Safari
  • MacOS / Desktop

Version Number: 1.3.34-1
Reproducible in staging?: y
Reproducible in production?: y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation

RPReplay_Final1688038420.MP4
RPReplay_Final1688136839.MP4

Expensify/Expensify Issue URL:
Issue reported by: @namhihi237
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1688039417283349

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01306ade9d70001f15
  • Upwork Job ID: 1674835066467475456
  • 2023-07-28
  • Automatic offers:
    • samh-nl | Contributor | 25862341
    • namhihi237 | Reporter | 25862342
Issue OwnerCurrent Issue Owner: @mallenexpensify
@kbecciv kbecciv added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Jun 30, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jun 30, 2023

Triggered auto assignment to @CortneyOfstad (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

@melvin-bot
Copy link

melvin-bot bot commented Jun 30, 2023

Bug0 Triage Checklist (Main S/O)

  • This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
  • This bug is not a duplicate report (check E/App issues and #expensify-bugs)
    • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
  • This bug is reproducible using the reproduction steps in the OP. S/O
    • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
    • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
  • This issue is filled out as thoroughly and clearly as possible
    • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
  • I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

@samh-nl
Copy link
Contributor

samh-nl commented Jun 30, 2023

Proposal

Please re-state the problem that we are trying to solve in this issue.

The user is unable to change the magic code if an error has occurred without refocusing manually.

What is the root cause of that problem?

Upon submitting, the magic code input is blurred:

if (inputValidateCodeRef.current) {
inputValidateCodeRef.current.blur();
}

What changes do you think we should make in order to solve the problem?

Remove the automatic blurring of the input.

What alternative solutions did you explore? (Optional)

An alternative solution is to keep the auto blurring of the input, but refocus only if an error subsequently occurs.

EDIT: Added code snippet.

@namhihi237
Copy link
Contributor

namhihi237 commented Jun 30, 2023

Proposal

Please re-state the problem that we are trying to solve in this issue.

User able to proceed after re-enter last digit of magic code number

What is the root cause of that problem?

The cause of this problem lies here:
This issue occurs when after entering last input same as current number in that cell, it won't call API so value value of input will not be reset resulting numbersArr will always be equal to last value before when the 6th cell is entered, and does not change no matter what numbers are entered after that.
For example, when the last number is 1 => we focus on the last cell => re-enter the number 1 => value = "1", then the next entered random numbers value = "1xxxx". this code will always return [1].

const numbersArr = value
.trim()
.split('')
.slice(0, props.maxLength - editIndex);
const updatedFocusedIndex = Math.min(editIndex + (numbersArr.length - 1) + 1, props.maxLength - 1);

What changes do you think we should make in order to solve the problem?

We will update: when in the last input box, we will always take the last value of the user entered `value

let numbersArr = []
numbersArr = value
    .trim()
    .split('')
    .slice(0, props.maxLength - editIndex);

// handle the last number if after entering the last number but still enter:
let newValue = '';
if (value.length > props.maxLength - editIndex) {
    const lastInput = value[value.length - 1];
    numbersArr[numbersArr.length - 1] = lastInput;
    newValue = value.slice(0, props.maxLength - editIndex) + lastInput;
}

setInput(newValue || value);

Also we need to remove the limit maxLength={props.maxLength} because in case when user input wrong magic code then user click first input and re-enter same wrong magic code above then the user numbers input will not work because the length is already equal to maxLength even though inpiut still has focus and does not give an error. We can omit it because above we have covered the magic code where the set will not be larger than prop.maxLength.
Result:

Screen.Recording.2023-06-30.at.22.53.11.mov

 ### What alternative solutions did you explore? (Optional)
 We can unfocus when inputting the last number or after fulfilling whether it calls API or not

@CortneyOfstad CortneyOfstad added the External Added to denote the issue can be worked on by a contributor label Jun 30, 2023
@melvin-bot melvin-bot bot changed the title Log in - User unable to proceed after re-enter last digit of magic code number [$1000] Log in - User unable to proceed after re-enter last digit of magic code number Jun 30, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jun 30, 2023

Job added to Upwork: https://www.upwork.com/jobs/~01306ade9d70001f15

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Jun 30, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jun 30, 2023

Current assignee @CortneyOfstad is eligible for the External assigner, not assigning anyone new.

@melvin-bot
Copy link

melvin-bot bot commented Jun 30, 2023

Triggered auto assignment to Contributor-plus team member for initial proposal review - @eVoloshchak (External)

@CortneyOfstad
Copy link
Contributor

I am heading OoO for the week (back July 10) so re-assigning this in the meantime 👍

@CortneyOfstad CortneyOfstad removed their assignment Jun 30, 2023
@CortneyOfstad CortneyOfstad added Bug Something is broken. Auto assigns a BugZero manager. and removed Bug Something is broken. Auto assigns a BugZero manager. labels Jun 30, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jun 30, 2023

Triggered auto assignment to @mallenexpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

@melvin-bot
Copy link

melvin-bot bot commented Jun 30, 2023

Bug0 Triage Checklist (Main S/O)

  • This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
  • This bug is not a duplicate report (check E/App issues and #expensify-bugs)
    • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
  • This bug is reproducible using the reproduction steps in the OP. S/O
    • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
    • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
  • This issue is filled out as thoroughly and clearly as possible
    • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
  • I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

@ugur-akin
Copy link

ugur-akin commented Jul 1, 2023

Proposal

Please re-state the problem that we are trying to solve in this issue.

When clicked/touched on the last digit of a magic code input that is previously non-empty, upon re-inputting the same digit as previously entered (i.e. the last digit was previously 6, user presses 6 again after clicking the last digit), the code will not auto submit and nor will any further digit inputs will update the last digit.

What is the root cause of that problem?

The handling of the underlying text input in magic code input component is implemented incorrectly to handle the case when the user re-types the same last digit as the previously entered one.

const numbersArr = value
.trim()
.split('')
.slice(0, props.maxLength - editIndex);

What changes do you think we should make in order to solve the problem?

Recommended Approach:

I think there's a good opportunity to improve MagicCodeInput component and the magic code UX for Expensify (see alterative solutions explored section).

Fixing the existing implementation:

Split up the underlying text input to: [...digitsUpToLastDigit, lastDigit] .

const  valueAsNumbers  =  value.trim().split('');

const  numDigitsToTakeIfValueIsPastMaxLength  =  props.maxLength  -  editIndex  -  1;
const  numDigitsToTakeIfValueIsShorterThanMaxLength  =  value.length -  1;
const  numDigitsToTake  =  Math.min(numDigitsToTakeIfValueIsPastMaxLength, numDigitsToTakeIfValueIsShorterThanMaxLength);
const  valueUpToLastDigit  =  value.slice(0, numDigitsToTake);  

const  lastDigitInValue  =  valueAsNumbers[valueAsNumbers.length -  1];

const  effectiveValueAsNumbers  = [...valueUpToLastDigit, lastDigitInValue];

const  updatedFocusedIndex  =  Math.min(editIndex  + (effectiveValueAsNumbers.length -  1) +  1, props.maxLength  -  1);

let  numbers  =  decomposeString(props.value, props.maxLength);
numbers  = [...numbers.slice(0, editIndex), ...effectiveValueAsNumbers, ...numbers.slice(effectiveValueAsNumbers.length +  editIndex, props.maxLength)];

What alternative solutions did you explore?

Opinionated Statement: The complexity of the magic code input can be reduced drastically if we opt for better UX choices.
Claim: Often re-typing the entire code is easier for the user than correcting digits. Fixing a digit requires identifying the incorrect digit, focusing it (either by clicking/touching the right digit or through arrow keys navigation) and entering the right digit.

Clear on incorrect submission

Maintain focus on the magic input and clear it upon an unsuccessful code entry.
Claim: Users are pretty effective with the keyboard (especially on mobile). Don't make them re-focus the input, let them re-type without hassle. A bonus of this is a second opportunity to leverage pasting a value or automatic text fills such as iOS' SMS Passcode feature. This method is also used widely in the industry and is Tinder's choice of UX for one-time password inputs.

Don't let users enter past the first empty index

Claim: Allowing users to enter digits past the first empty digit is practically useless - arguably nobody fills starting from mid-way through and if it happens, it's probably a mistake. If the user loses focus on the text input, let them click anywhere on the magic code and automatically register the input for the first empty digit. Along with clear on incorrect submission, this will entirely remove the need for complicated input-index/focus-index state management. If we don't like to clear input, this would still be a worth improvement on its own.

Edit: Removed anchor links to comment sections - they weren't working. Clarifications and typo/styling fixes.

@melvin-bot
Copy link

melvin-bot bot commented Jul 1, 2023

Looks like something related to react-navigation may have been mentioned in this issue discussion.

As a reminder, please make sure that all proposals are not workarounds and that any and all attempt to fix the issue holistically have been made before proceeding with a solution. Proposals to change our DeprecatedCustomActions.js files should not be accepted.

Feel free to drop a note in #expensify-open-source with any questions.

@melvin-bot
Copy link

melvin-bot bot commented Jul 1, 2023

📣 @ugur-akin! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

  1. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
  2. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
  3. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
    Screen Shot 2022-11-16 at 4 42 54 PM
    Format:
Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

@ugur-akin
Copy link

Contributor details
Your Expensify account email: ugurakinsoftware@gmail.com
Upwork Profile Link: https://www.upwork.com/freelancers/~0161bd395d1f6814a7

@melvin-bot
Copy link

melvin-bot bot commented Jul 1, 2023

✅ Contributor details stored successfully. Thank you for contributing to Expensify!

@lazar-stamenkovic
Copy link
Contributor

lazar-stamenkovic commented Jul 2, 2023

Please re-state the problem that we are trying to solve in this issue.

User unable to proceed after re-enter last digit of magic code

What is the root cause of that problem?

This issue is caused by following reason

  • The MagicCode input doesn't clear text input value when the user inputs last digit of a magic code.
  • Whenever typing the text, OnChangeText is called even if code text isn't changed.

What changes do you think we should make in order to solve the problem?

  • Text Input value has to be cleared if current focused input is last input.
  • OnChangeText is called only if code text is changed.

Screenshot 2023-07-02 at 10 53 09

@melvin-bot melvin-bot bot added the Overdue label Jul 3, 2023
@amyevans
Copy link
Contributor

Works for me, thanks for keeping a pulse on it

@melvin-bot melvin-bot bot added the Overdue label Oct 23, 2023
@amyevans
Copy link
Contributor

Still on hold for #28711

@melvin-bot melvin-bot bot removed the Overdue label Oct 23, 2023
@melvin-bot melvin-bot bot added the Overdue label Nov 1, 2023
@mallenexpensify
Copy link
Contributor

Actively being worked on

@melvin-bot melvin-bot bot removed the Overdue label Nov 3, 2023
@melvin-bot melvin-bot bot added the Overdue label Nov 13, 2023
@amyevans
Copy link
Contributor

Still on hold, there's movement on the PR

@melvin-bot melvin-bot bot removed the Overdue label Nov 14, 2023
@melvin-bot melvin-bot bot added the Overdue label Nov 23, 2023
@amyevans
Copy link
Contributor

Linked PR is still open

@melvin-bot melvin-bot bot removed the Overdue label Nov 27, 2023
@melvin-bot melvin-bot bot added the Overdue label Dec 6, 2023
@amyevans
Copy link
Contributor

amyevans commented Dec 6, 2023

#28711 is merged, are we ready to roll on this one @samh-nl?

@melvin-bot melvin-bot bot removed the Overdue label Dec 6, 2023
@amyevans amyevans changed the title [HOLD 28711][$1000] Log in - User unable to proceed after re-enter last digit of magic code number [$1000] Log in - User unable to proceed after re-enter last digit of magic code number Dec 6, 2023
@melvin-bot melvin-bot bot added the Overdue label Dec 14, 2023
@amyevans
Copy link
Contributor

Discussing in Slack

@melvin-bot melvin-bot bot removed the Overdue label Dec 14, 2023
@mallenexpensify
Copy link
Contributor

@samh-nl do you want to work on this?

@mkhutornyi
Copy link
Contributor

This is now expected behavior and can be closed. Also dupe of #31798

@samh-nl
Copy link
Contributor

samh-nl commented Dec 15, 2023

Sorry for the reduced responsiveness. In principle I'd like to further pursue this following the holding PR is now merged.

The issues referenced by @mkhutornyi do make it seem redundant, however. We currently only auto-submit the first time, allowing the user to adjust multiple digits down the line without it resubmitting automatically. Our issue here would undo this 'new' behavior.

@amyevans
Copy link
Contributor

Agreed, and thanks for the links @mkhutornyi.

Let's close this then - @mallenexpensify can you handle any payments that may be due for work completed to date?

Thanks!

@amyevans amyevans added Daily KSv2 and removed Weekly KSv2 labels Dec 15, 2023
@melvin-bot melvin-bot bot added the Overdue label Dec 18, 2023
@mallenexpensify
Copy link
Contributor

Thanks all!
Compensation shouldn't be due because we didn't move forward with a fix. If you disagree, please post!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor
Projects
None yet
Development

No branches or pull requests