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 $1000] Date picker doesn’t show after focus on another field and press Fix the error #14529

Closed
2 tasks
kavimuru opened this issue Jan 24, 2023 · 29 comments
Closed
2 tasks
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors

Comments

@kavimuru
Copy link

kavimuru commented Jan 24, 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 Settings -> Workspaces => Connect Bank Account Manually => Enter invalid information for only Date of birth at step 3.
  2. Now press Save & Continue, an error will show up, focus on SSN field (or any field), scroll down and press Fix the error, scroll up and press the Date picker.
  3. Notice that Date picker is not responding.

Expected Result:

Date picker should show up at the bottom and respond to click

Actual Result:

Nothing happened after pressing Date picker field.

Workaround:

unknown

Platforms:

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

  • Android / native
  • iOS / native

Version Number: 1.2.58-2
Reproducible in staging?: y
Reproducible in production?: could not check
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:

RPReplay_Final1674318754.MP4
repros.ios.1.2301.mp4

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

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01c59fc18f15ea7403
  • Upwork Job ID: 1618921611535593472
  • Last Price Increase: 2023-02-03
@kavimuru kavimuru added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Jan 24, 2023
@melvin-bot melvin-bot bot locked and limited conversation to collaborators Jan 24, 2023
@greg-schroeder
Copy link
Contributor

greg-schroeder commented Jan 25, 2023

Bug0 Triage Checklist

Note: see this SO for more information.

  • The "bug" is actually a bug
  • The bug is not a duplicate report of an existing GH.
    • If it is, close the GH and add any novel details to the original GH instead
  • The bug is reproducible, following the reproduction steps.
    • If the GH doesn’t have steps to reliably reproduce the bug and you figure it out, then please add them.
    • If you’re unable to reproduce the bug, add the Needs reproduction label.
    • Comment on the issue outlining the steps you took to try to reproduce the bug, your results and tag the issue reporter and the Applause QA member who created the issue. Ask them to clarify reproduction steps and/or to check the reproduction steps again. Close issue.
  • The GH template is filled out as fully as possible
    • The GH body and title are clear (ie. could an external contributor understand it and work on it?)
  • The GH was created by an Expensify employee or a QA tester
  • If the bug is an OldDot issue, you should decide whether it is widespread enough to justify fixing or it is better to wait for reunification. If it’s better to wait, close the GH & provide this justification
  • If there's a link to Slack, check the discussion to see if we decided not to fix it
  • Decide if the GH should be resolved by an External contributor or Internal engineer, add the appropriate label

@greg-schroeder
Copy link
Contributor

@melvin-bot melvin-bot bot added the Overdue label Jan 27, 2023
@greg-schroeder
Copy link
Contributor

Okay, got a bit more detail about the reproduction and also adjusted the OP a little bit

@melvin-bot melvin-bot bot removed the Overdue label Jan 27, 2023
@greg-schroeder greg-schroeder added the External Added to denote the issue can be worked on by a contributor label Jan 27, 2023
@melvin-bot melvin-bot bot unlocked this conversation Jan 27, 2023
@melvin-bot melvin-bot bot changed the title Date picker doesn’t show after focus on another field and press Fix the error [$1000] Date picker doesn’t show after focus on another field and press Fix the error Jan 27, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jan 27, 2023

Job added to Upwork: https://www.upwork.com/jobs/~01c59fc18f15ea7403

@melvin-bot
Copy link

melvin-bot bot commented Jan 27, 2023

Current assignee @greg-schroeder is eligible for the External assigner, not assigning anyone new.

@melvin-bot
Copy link

melvin-bot bot commented Jan 27, 2023

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

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

melvin-bot bot commented Jan 27, 2023

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

@s77rt
Copy link
Contributor

s77rt commented Jan 27, 2023

Should be on hold in favor of #14322

@marcaaron
Copy link
Contributor

I'm pretty confused actually. In both of the example videos I don't see this happening:

Nothing happened after pressing Date picker field.

The date picker is unresponsive when the user tried to interact with it?

Can someone post a better video that clearly shows this behavior?

@marcaaron
Copy link
Contributor

@s77rt I'm reluctant to put this on HOLD as it's not clear when exactly those design improvements would be made to this particular flow.

@s77rt

This comment was marked as off-topic.

@marcaaron
Copy link
Contributor

So the issue is that the menu doesn't open but the field gets focus? I'm still not sure because the description of the issue says:

Nothing happened after pressing Date picker field.

I don't see the date picker field getting "pressed"

@s77rt
Copy link
Contributor

s77rt commented Jan 27, 2023

@marcaaron Ah my bad, I got it wrong, it's a bug.

If you click on fix the errors while focusing on another input and this takes you to the date picker, the date picker won't actually get focused (date picker won't show up) and even if you try to click on it, it won't show up either.
You will have to focus on another element first then you will be able to open date picker.

Kooha-2023-01-27-23-11-03.mp4

@fedirjh

This comment was marked as resolved.

@melvin-bot melvin-bot bot added the Overdue label Jan 30, 2023
@marcaaron
Copy link
Contributor

@fedirjh I would suggest organizing your proposal like this:

  • Some clear explanation of the root cause.
  • Solution in text (English) and how it addresses the root cause.
  • Please do not provide code diffs and use pseudocode if necessary.

Thanks!

@melvin-bot melvin-bot bot removed the Overdue label Jan 30, 2023
@fedirjh
Copy link
Contributor

fedirjh commented Jan 30, 2023

@marcaaron Sorry if I miss explained my proposal , I will retry that

Some clear explanation of the root cause.

To Show Date picker at the bottom , we should to add a focus handler that will trigger this.showPicker (this is missing in the actual code) . However when TextInput prop editable is set to false, then the focus event is disabled

how it addresses the root cause

we can make Datepicker focusable by enabling the editable prop , then when input receives focus we set editable to false . this workaround will let us trigger focus event and show the date picker selector at the bottom

@s77rt

This comment was marked as off-topic.

@fedirjh
Copy link
Contributor

fedirjh commented Jan 30, 2023

@s77rt I think we should have the RCA to decide to hold it or not , I mean the issue may be related to Form or something else , so it's not wise to hold before we know the root cause.

@marcaaron
Copy link
Contributor

To Show Date picker at the bottom , we should to add a focus handler that will trigger this.showPicker (this is missing in the actual code) . However when TextInput prop editable is set to false, then the focus event is disabled

Sorry, this still sounds like a solution. Can you try to explain what the problem is without saying how to fix it at all?

@fedirjh
Copy link
Contributor

fedirjh commented Jan 30, 2023

Sorry, this still sounds like a solution. Can you try to explain what the problem is without saying how to fix it at all?

The date picker isn't focusable . The date picker selector is shown on Press event . So actually focus event will do nothing . There is no focus handler to show date picker on focus.

The 2nd part even if we add focus handler to show datepicker it will never work.

Edit : the Form uses the focus event to focus errored inputs . Datepicker isn't focusable that's the issue here.

I hope this explain it.

@marcaaron
Copy link
Contributor

The date picker isn't focusable

Can you be more specific here? The date picker on iOS and Android both have TextInput. Are you saying we can't focus that? There is an onFocus event here.

The 2nd part even if we add focus handler to show datepicker it will never work.

Why will it never work?

Edit : the Form uses the focus event to focus errored inputs . Datepicker isn't focusable that's the issue here.

Sorry, I don't understand what you mean at all. Can you try saying it another way?

@fedirjh
Copy link
Contributor

fedirjh commented Jan 31, 2023

Can you be more specific here? The date picker on iOS and Android both have TextInput. Are you saying we can't focus that? There is an onFocus event here.

When the editable prop of a TextInput is set to false, it means the user cannot modify the text inside the input field. As a result, the TextInput cannot receive focus as it is not meant to be edited by the user.

Supposing that the focus event works and we can focus the TextInput, focusing the TextInput will not show the selector at the bottom. Currently, this behavior is not yet implemented.

And that is the expected result here, when we focus the date picker in native, the selector will show up at the bottom.

@mdneyazahmad

This comment was marked as resolved.

@parasharrajat
Copy link
Member

I will check the proposals sometime.

@marcaaron
Copy link
Contributor

Maybe I am missing something obvious. But I have a few thoughts:

  • the focus() method can be whatever we want it to be on the DatePicker component. it doesn't need to have anything to do with the TextInput at all. It's just a method that we could implement to show the DatePicker.
  • I'm not sure anyone has addressed why tapping on the field after tapping on "Fix the errors" has "no effect" it seems like there was a suggestion that this is broken. I have not seen anyone mention this in their proposals to fix the problem.

So, can someone please clarify what exactly we are fixing in this issue?

@parasharrajat I would suggest to not review any of the current proposals until there is a consensus about what we are fixing and a clear explanation of the root cause. I'm going to go through and hide them so it's easier for us to focus on that.

@fedirjh

This comment was marked as off-topic.

@fedirjh

This comment was marked as off-topic.

@melvin-bot melvin-bot bot changed the title [$1000] Date picker doesn’t show after focus on another field and press Fix the error [$2000] Date picker doesn’t show after focus on another field and press Fix the error Feb 3, 2023
@melvin-bot melvin-bot bot added the Overdue label Feb 3, 2023
@marcaaron
Copy link
Contributor

Gonna put this issue on HOLD until we can clarify what the bug actually is. I don't agree with the doubling that just happened above since so far we have just been trying to establish what exactly we are fixing.

@melvin-bot melvin-bot bot removed the Overdue label Feb 3, 2023
@marcaaron marcaaron changed the title [$2000] Date picker doesn’t show after focus on another field and press Fix the error [HOLD $1000] Date picker doesn’t show after focus on another field and press Fix the error Feb 3, 2023
@marcaaron
Copy link
Contributor

Actually I'm going to kick this one back to Slack. If there are 2 bugs lets report them separately and restart the conversation here.

@marcaaron marcaaron closed this as not planned Won't fix, can't repro, duplicate, stale Feb 3, 2023
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 External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors
Projects
None yet
Development

No branches or pull requests

7 participants