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 2022-04-06] Enter/return doesn't work when the button has options - reported by @Tushu17 #7964

Closed
mvtglobally opened this issue Mar 2, 2022 · 30 comments
Assignees
Labels
Engineering External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review Weekly KSv2

Comments

@mvtglobally
Copy link

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. Go to chat of a user who have a paypal account.
  2. Select Actions -> Send money -> enter amount > Hit Next
  3. Pick any user
  4. Press Enter/Return or "Go" on mobile keyboard

Expected Result:

Pressing Enter/Return should send money like it does in the request money.

Actual Result:

Pressing Enter/Return Closes the Keyboard or does nothing

Workaround:

unknown

Platform:

Where is this issue occurring?

  • Web
  • iOS
  • Android
  • Desktop App
  • Mobile Web

Version Number: 1.1.41-0
Reproducible in staging?: Y
Reproducible in production?: Y
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation

screen-rec.mp4

Expensify/Expensify Issue URL:
Issue reported by: @Tushu17
Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1644833714384489

View all open jobs on GitHub

@MelvinBot
Copy link

Triggered auto assignment to @deetergp (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

@sobitneupane
Copy link
Contributor

Proposal
Addition of pressOnEnter in

<Button
success
onPress={props.onButtonPress}
text={props.buttonText}
isLoading={props.isLoading}
shouldRemoveRightBorderRadius
style={[styles.flex1]}
/>

<Button
    success
    onPress={props.onButtonPress}
    text={props.buttonText}
    isLoading={props.isLoading}
    shouldRemoveRightBorderRadius
    style={[styles.flex1]}
+   pressOnEnter
/>

solves the issue.

@Tushu17
Copy link
Contributor

Tushu17 commented Mar 2, 2022

Proposal

Proposing a solution as I reported this issue

We aren't passing pressOnEnter to the Button that we use in ButtonWithDropdown as we can see below -

const ButtonWithDropdown = props => (
<View style={[styles.flexRow, styles.justifyContentBetween, styles.alignItemsCenter]}>
<Button
success
onPress={props.onButtonPress}
text={props.buttonText}
isLoading={props.isLoading}
shouldRemoveRightBorderRadius
style={[styles.flex1]}
/>

So by passing pressOnEnter to the Button which is on line 32 we can fix it.

@deetergp deetergp added the External Added to denote the issue can be worked on by a contributor label Mar 2, 2022
@MelvinBot
Copy link

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

@jliexpensify
Copy link
Contributor

@deetergp - should go to Upworks right? If so, I'll assign myself again and make a job.

@jliexpensify jliexpensify self-assigned this Mar 3, 2022
@jliexpensify
Copy link
Contributor

@MelvinBot
Copy link

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

@MelvinBot MelvinBot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Mar 3, 2022
@MelvinBot
Copy link

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

@rushatgabhane
Copy link
Member

🎀👀🎀 C+ reviewed

@NikkiWines Let's go with @Tushu17's proposal since they reported the issue?

@NikkiWines
Copy link
Contributor

Since the two proposals are identical, I think we should give the job to @sobitneupane, since they made a proposal first. I don't believe we have a process where issue reporters get first dibs on an issue cc: @mallenexpensify

@mallenexpensify
Copy link
Contributor

For reference, here's the details in CONTRIBUTING.md. https://github.com/Expensify/App/blob/main/CONTRIBUTING.md#propose-a-solution-for-the-job

If y'all or anyone else wants to propose an update to this process, please do so in #expensify-open-source, it's been brought up a couple times before in that channel, I'd recommend searching around #expensify-open-source first and gathering some links/details/GH handles before starting a new discussion.

@jliexpensify
Copy link
Contributor

@NikkiWines - the MD states:

This is an opportunity to propose a job, and (optionally) a solution.

Which makes me think that person who identified the bug should be given a chance to propose a solution

@Tushu17
Copy link
Contributor

Tushu17 commented Mar 3, 2022

@sobitneupane, had given his proposal just after it got logged on GH and also there was no external label so there wasn't any way I would have given my proposal before him. I had my proposal ready but I was waiting for the external label. The MD states that we should wait for the external label. I found that some people are proposing solutions just after the issue gets logged on GH. It just demotivating. I try to find solution for different issues and someone just posts his proposal without waiting the external label it makes all the efforts go in vein. I think we should make the proper rules that everyone has to follow so no one's efforts go in vein. I totally agree we should start a discussion on these rules and I am ready to share my views on it.
cc: @mallenexpensify @jliexpensify

@sobitneupane
Copy link
Contributor

@mallenexpensify @jliexpensify @NikkiWines
As per this discussion,

We don't mention "first dibs" (the person reporting is allowed to post a solution first) because our goal is to get bugs reported and fixed quickly, we want to avoid a delay while we wait to see if the reporting contributor. It also adds complexity to our process.

I thought anyone is allowed to propose a solution. So, I proposed the solution.
From now on, I will wait for reporter to propose a solution. But I am not sure how long should I wait for reporter to propose solution before proposing my solution?

@Tushu17
Copy link
Contributor

Tushu17 commented Mar 3, 2022

@sobitneupane, I think you're getting it wrong, It's not about you posting proposal on other's Issue but as it has been made clear many times to us that we have to wait till the external label and I was doing the same on this issue.

@NikkiWines
Copy link
Contributor

@jliexpensify - I raised this discussion in slack in an internal room here and confirmed that we don't have any policies that give precedence to the bug reporter just because they raised the issue. Bug reporters still get a chance to raise a potential solution and, in most cases, there are multiple ways to solve a problem so time isn't the main factor in a proposal being chosen.

@sobitneupane, @Tushu17 is right in that the proper process for contributors is to wait until the External label is applied before asking for proposals.

@Tushu17, regardless of the reason, you technically also violated the above guideline by posting a proposal before the External label was applied, so I don't think that's a good reason to give your proposal priority over @sobitneupane's.

Basically, my stance is that given we have two proposals that are identical and were both posted before the External label was applied, we should go with the first proposal posted.

@rushatgabhane
Copy link
Member

rushatgabhane commented Mar 3, 2022

@NikkiWines That's a good point. I agree 100%

@NikkiWines
Copy link
Contributor

NikkiWines commented Mar 10, 2022

Sorry about the delay here! Posting in slack prompted a much larger discussion than I was expecting 😅

@jliexpensify
Copy link
Contributor

Hired Rushat, Tushar and Sobit!

@MelvinBot MelvinBot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Mar 11, 2022
@jliexpensify
Copy link
Contributor

Adding @kadiealexander to help keep an eye on my GH's as I'll be OOO next week

@MelvinBot
Copy link

📣 @sobitneupane You have been assigned to this job by @jliexpensify!
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 📖

@NikkiWines NikkiWines added the Reviewing Has a PR in review label Mar 18, 2022
@kadiealexander kadiealexander changed the title Enter/return doesn't work when the button has options - reported by @Tushu17 [HOLD for payment 2022-04-06] Enter/return doesn't work when the button has options - reported by @Tushu17 Apr 1, 2022
@kadiealexander
Copy link
Contributor

@jliexpensify leaving you assigned, please make payment on this one on the 6th/7th, as I'll be away. Thanks! 🎉

@jliexpensify
Copy link
Contributor

There's an iOS regression - if this isn't a big deal, I can arrange payment cc @rushatgabhane @NikkiWines

@rushatgabhane
Copy link
Member

@jliexpensify could you please link the iOS regression. I'm not able to find it 😅

@jliexpensify
Copy link
Contributor

@rushatgabhane - apologies: I used the wrong terminology here! I did see that iOS failed to deploy - #8206 - which is what I meant.

If that's not an issue, I'll pay!

@jliexpensify
Copy link
Contributor

Asking in Slack here: https://expensify.slack.com/archives/C01SKUP7QR0/p1649636370757839

Hoping to get an answer in the next 24 hours so I can issue payment.

@jliexpensify
Copy link
Contributor

Sorry for the delay - confirmed that it's all good to proceed with payment, and will do shortly.

@jliexpensify
Copy link
Contributor

Paid @Tushu17 and @rushatgabhane . Just awaiting @sobitneupane to accept and will pay + close job.

@sobitneupane
Copy link
Contributor

Just awaiting @sobitneupane to accept and will pay + close job.

Accepted the offer.

@jliexpensify
Copy link
Contributor

Paid, and closed job in Upworks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Engineering External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review Weekly KSv2
Projects
None yet
Development

No branches or pull requests