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

[HOLD for payment 2023-03-09] [$1000] Android app does not focus on date of birth field when 'fix the errors' link is clicked #15290

Closed
2 tasks
kavimuru opened this issue Feb 19, 2023 · 32 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor

Comments

@kavimuru
Copy link

kavimuru commented Feb 19, 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 the app in android
  2. Open settings
  3. Open Profile
  4. Open Personal details
  5. Open date of birth
  6. Write or select any wrong date eg: 02/02/2024
  7. Click on save
  8. Click on fix the errors

Expected Result:

App should focus on date of birth field and open the calender when 'fix the errors' is clicked like it does on mWeb android chrome

Actual Result:

App does nothing on 'fix the errors' click on date of birth page (same issue on connect bank account step 3 and step 2 i.e. issue exists on all the pages with calender selection on android app)

Workaround:

unknown

Platforms:

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

  • Android / native
  • iOS / native

Version Number: 1.2.74-0
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:

android.app.calender.issue.mp4
az_recorder_20230219_164132.1.mp4

Expensify/Expensify Issue URL:
Issue reported by: @dhanashree-sawant
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1676817426854809

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01996698b9e9290a5b
  • Upwork Job ID: 1627953353378811904
  • Last Price Increase: 2023-02-21
@kavimuru kavimuru added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Feb 19, 2023
@melvin-bot melvin-bot bot locked and limited conversation to collaborators Feb 19, 2023
@MelvinBot
Copy link

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

@MelvinBot
Copy link

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

@Expensify Expensify unlocked this conversation Feb 20, 2023
@mountiny
Copy link
Contributor

Callstack can take this one

@narefyev91
Copy link
Contributor

Hello, I'm Nicolay from Callstack and I'm interested in taking and analysing this issue to work on a fix for it.

@Beamanator Beamanator added the External Added to denote the issue can be worked on by a contributor label Feb 21, 2023
@melvin-bot melvin-bot bot changed the title Android app does not focus on date of birth field when 'fix the errors' link is clicked [$1000] Android app does not focus on date of birth field when 'fix the errors' link is clicked Feb 21, 2023
@MelvinBot
Copy link

Job added to Upwork: https://www.upwork.com/jobs/~01996698b9e9290a5b

@MelvinBot
Copy link

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

@MelvinBot
Copy link

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

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

Triggered auto assignment to @alex-mechler (External), see https://stackoverflow.com/c/expensify/questions/7972 for more details.

@melvin-bot melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Feb 21, 2023
@MelvinBot
Copy link

📣 @narefyev91 You have been assigned to this job by @Beamanator!
Please apply to this job in Upwork and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

@suryaneelankar
Copy link

On Clicking fix the errors we need to show the modal calendar again using useState with which it will get the expected result

@Santhosh-Sellavel
Copy link
Collaborator

Let me know when PR is ready @narefyev91, thanks!

@narefyev91
Copy link
Contributor

Proposal

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

Native apps don't open the datepicker when clicking on Fix error button

What is the root cause of that problem?

The main issue here - combination of trying to focus input (clicking on fix error button) and editable={false} for input field

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

Case is special because global Form error handling (src/components/Form.js) doesn't even know that any field maybe not editable (focusable) and to be honest it should not know about it - error logic should work for all fields.
Date of birthday field is not editable - and this code will not work

  if (focusInput.focus && typeof focusInput.focus === 'function') {
      focusInput.focus();
  }

BTW we can implement custom function to fire on focus event.
Here we see that we need to have custom ref instead of normal ref for input. If it will be functional components - we will use useImperativeHandle - which specified to apply custom handlers for any default ref functions.
We can't use this hook here - in that case we need to modify TextInput ref to make it working based on our needs and not breaking any existing functionality
Solution will be to add showPicker method for focus event

  ref={(el) => {
      if (!_.isFunction(this.props.innerRef)) {
          return;
      }
      if (el && el.focus && typeof el.focus === 'function') {
          let inputRef = {...el};
          inputRef = {...inputRef, focus: this.showPicker};
          this.props.innerRef(inputRef);
          return;
      }

      this.props.innerRef(el);
  }}

What alternative solutions did you explore?

Alternative solution was playing around with making Input editable - but it will show some weird behaviour - like jumping keyboard/datepicker

cc @Santhosh-Sellavel @mountiny

@mountiny
Copy link
Contributor

@alex-mechler is the engineer assigned here so I will leave this proposal review to them 🙌

@alex-mechler
Copy link
Contributor

That proposal looks good to me @narefyev91. @Santhosh-Sellavel do you have anything to add about it?

@Santhosh-Sellavel
Copy link
Collaborator

All good for now, I will follow up on the PR thanks!

@sakluger
Copy link
Contributor

sakluger commented Mar 3, 2023

@Santhosh-Sellavel I sent you an offer through Upwork.

Hi @narefyev91 - could you please apply to the Upwork job (https://www.upwork.com/jobs/~01996698b9e9290a5b) or comment with your Upwork profile links so that I can send you an offer via Upwork?

@TomaszFrackowiak
Copy link

@sakluger - Hi, @narefyev91 is from Callstack, so you don't need to send an offer via Upwork

@MelvinBot
Copy link

📣 @TomaszFrackowiak! 📣

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>

@Santhosh-Sellavel
Copy link
Collaborator

Accepted now @sakluger Sorry for the delay!

@Santhosh-Sellavel
Copy link
Collaborator

@sakluger @alex-mechler
Does this qualify for an efficiency bonus or not?

@alex-mechler
Copy link
Contributor

Proposal was accepted at 2/23 9:46am PT. PR was merged 2/27 at 8:59am PT. The 25th and 26th were Saturday and Sunday, so this falls in the 3 business day window. I think this qualifies for the bonus @sakluger

@alex-mechler
Copy link
Contributor

On hold for payment tomorrow melvin

@melvin-bot melvin-bot bot added Daily KSv2 and removed Overdue Daily KSv2 labels Mar 8, 2023
@sakluger
Copy link
Contributor

Paid out everyone and closed the Upwork post!

@narefyev91 could you please propose if we need regression testing steps, then check off the checkbox in the BugZero checklist above? Thanks!

@narefyev91
Copy link
Contributor

Paid out everyone and closed the Upwork post!

@narefyev91 could you please propose if we need regression testing steps, then check off the checkbox in the BugZero checklist above? Thanks!

@sakluger in my view no regression tests needed here

@melvin-bot melvin-bot bot added the Overdue label Mar 13, 2023
@sakluger
Copy link
Contributor

Sounds good @narefyev91! On future issues, feel free to check off the checkbox in the BugZero checklist and leave your proposal in a comment (even if that proposal is "No regression steps".

@kbecciv
Copy link

kbecciv commented May 30, 2023

@Beamanator Fix the errors message is not functional on build 1.3.20.1
Let me know if we need a separate bug

Recording.4763.mp4

@kbecciv kbecciv reopened this May 30, 2023
@melvin-bot melvin-bot bot added the Overdue label May 30, 2023
@Santhosh-Sellavel
Copy link
Collaborator

There is nothing to do here,

  1. Date field is the only field here and highlight with the error
  2. We have the calendar always open, so the user can just pick the validate now.

@melvin-bot melvin-bot bot removed the Overdue label May 30, 2023
@Beamanator
Copy link
Contributor

(I think wrong Alex was pinged here? Leaving for you @alex-mechler )

@alex-mechler
Copy link
Contributor

Is this only happening on that specific input @kbecciv? If so, that input is non-focusable, since the calendar is always open, and thus not an issue for the reasons that @Santhosh-Sellavel mentioned.

If this is happening across every form in the app, then its an issue and we should open a new issue for it

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

No branches or pull requests