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

[Payment due Jul 5 - $1000] App does not refocus to compose box after deleting the message using enter key #20619

Closed
1 of 6 tasks
kavimuru opened this issue Jun 12, 2023 · 40 comments
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

@kavimuru
Copy link

kavimuru commented Jun 12, 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
  2. Open any report
  3. Send any message
  4. Click on upper arrow to edit the message
  5. Remove all the content, click on save changes
  6. In delete popup, use mouse pointer and click on delete to see app focuses back on compose box
  7. Repeat steps 3-5, when delete popup is open, click enter to delete the message and observe that app does not focus back on compose box

Expected Result:

App should focus back on compose box when we delete any message by enter click as it normally focuses back to compose back while deleting message using click on delete button

Actual Result:

App does not focus back on compose box when we delete any message by enter click.

Workaround:

Can the user still use Expensify without this being fixed? Have you informed them of the workaround?

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.26-3
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

no.focus.back.on.delete.by.enter.mp4
Recording.951.mp4

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

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01488b62e8528b9e9b
  • Upwork Job ID: 1670866028498096128
  • Last Price Increase: 2023-06-19
@kavimuru kavimuru added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Jun 12, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jun 12, 2023

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

@melvin-bot
Copy link

melvin-bot bot commented Jun 12, 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

@dukenv0307
Copy link
Contributor

Proposal

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

Inconsistency Bug- App does not refocus to compose box after deleting the message using enter key (Works fine on using mouse to click delete)

What is the root cause of that problem?

When we use enter key to delete comment, onBackdropPress is triggered. In this function we check if event is enter key and type is keydown we will not call onClose function. But type of event is keyup when we use enter key to delete comment.

image

onBackdropPress={(e) => {
if (e && e.type === 'keydown' && e.key === 'Enter') {
return;
}
this.props.onClose();
}}

and this.props.onClose that is onCancel function in delete modal is called and then edit compose is re-focused instead of main compose.

InteractionManager.runAfterInteractions(() => this.textInput.focus()),

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

We should change the check in here to if (e && (e.type === 'keydown' || e.type === 'keyup') && e.key === 'Enter') { to prevent correctly calling onClose function when we use enter key to confirm an action.

onBackdropPress={(e) => {
if (e && e.type === 'keydown' && e.key === 'Enter') {
return;
}
this.props.onClose();
}}

What alternative solutions did you explore? (Optional)

NA

Result

Screencast.from.12-06-2023.16.40.20.webm

@melvin-bot melvin-bot bot added the Overdue label Jun 15, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jun 16, 2023

@michaelhaxhiu Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

@michaelhaxhiu michaelhaxhiu added the External Added to denote the issue can be worked on by a contributor label Jun 19, 2023
@melvin-bot melvin-bot bot changed the title App does not refocus to compose box after deleting the message using enter key [$1000] App does not refocus to compose box after deleting the message using enter key Jun 19, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jun 19, 2023

Job added to Upwork: https://www.upwork.com/jobs/~01488b62e8528b9e9b

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

melvin-bot bot commented Jun 19, 2023

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

@melvin-bot
Copy link

melvin-bot bot commented Jun 19, 2023

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

@michaelhaxhiu
Copy link
Contributor

yep, I think it's not consistent.

(I almost felt like... this is more a keyboard thing. But I thought more and thing it's more focus > keyboard. So let's fix the inconsistent behavior on web browsers to align with other platforms.)

@melvin-bot melvin-bot bot removed the Overdue label Jun 19, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jun 19, 2023

📣 @Artemis330! 📣
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>

@web3gru
Copy link

web3gru commented Jun 19, 2023

Contributor details
Your Expensify account email: felixleung330@gmail.com
Upwork Profile Link: https://www.upwork.com/freelancers/~01fedc475640d6bc79

@melvin-bot
Copy link

melvin-bot bot commented Jun 19, 2023

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

@web3gru
Copy link

web3gru commented Jun 19, 2023

Hello, I am Felix, I would be happy to help you fix this issue with your Expensify app. Based on the details you provided, I understand that the app is not focusing back on the compose box when a message is deleted using the "enter" key.

To fix this, I recommend implementing an event listener that listens for the "delete" action and then triggers the focus back to the compose box. The implementation will vary depending on the platform, but in general, it involves adding an event listener to the delete button and calling the appropriate method to focus the app back on the compose box.

For example, on iOS, you can use the UITextFieldDelegate protocol to detect when the user has pressed the return key and then trigger the focus back on the compose box. On Android, you can use the setOnKeyListener method to listen for key events and then focus the view accordingly.

Once the code changes are complete, I recommend testing the new functionality thoroughly on all supported platforms to ensure that the issue has been resolved. Lastly, I'll submit a pull request with the code changes and detailed documentation so that your team can review and merge the changes into the main branch.

Please let me know if you have any questions or concerns about my proposed solution. I'm looking forward to working with you to resolve this issue!

@Santhosh-Sellavel
Copy link
Collaborator

@Artemis330 If you haven’t already, check out our contributing guidelines to understand our process and make a proposal as per guidelines thanks!

@Santhosh-Sellavel
Copy link
Collaborator

@dukenv0307 Looks good, Can you check and ensure it does not break anything else, thanks!

@dukenv0307
Copy link
Contributor

@Santhosh-Sellavel seem react-native-web has change the event to keyup event. And I also tested modals in the app that use BaseModal component with this change in my proposal, it doesn't break anything.

@Santhosh-Sellavel
Copy link
Collaborator

Was the change made recently in the react-native-web. Do you have any PR references?

@dukenv0307
Copy link
Contributor

@Santhosh-Sellavel This change is in this commit on Sep 9, 2020.

@Santhosh-Sellavel
Copy link
Collaborator

If that's the case I'm not sure how we missed capturing this bug, I kind of feel this might be broken due to any recent changes.

@Santhosh-Sellavel
Copy link
Collaborator

@dukenv0307's Proposal looks good me to.
C+ Reviewed
🎀 👀 🎀

@melvin-bot
Copy link

melvin-bot bot commented Jun 23, 2023

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

@melvin-bot melvin-bot bot removed the Overdue label Jun 29, 2023
@robertjchen
Copy link
Contributor

Thanks for all the discussion here! Upon review, I agree with @dukenv0307 's proposal as well, let's move forward with the solution 👍

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

melvin-bot bot commented Jun 29, 2023

📣 @Santhosh-Sellavel You have been assigned to this job!
Please apply to this job in Upwork here and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Once you apply to this job, your Upwork ID will be stored and you will be automatically hired for future jobs!
Keep in mind: Code of Conduct | Contributing 📖

@melvin-bot
Copy link

melvin-bot bot commented Jun 29, 2023

📣 @dukenv0307 🎉 An offer has been automatically sent to your Upwork account 🎉

Contributor - [$1000] App does not refocus to compose box after deleting the message using enter key Please accept the offer 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 📖

@melvin-bot
Copy link

melvin-bot bot commented Jun 29, 2023

📣 @dhanashree! 📣
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>

@melvin-bot
Copy link

melvin-bot bot commented Jun 29, 2023

The BZ member will need to manually hire dhanashree for this role Reporter. Please store your Upwork details and apply to our Upwork job so this process is automatic in the future!

@dukenv0307
Copy link
Contributor

dukenv0307 commented Jun 30, 2023

@Santhosh-Sellavel @michaelhaxhiu This issue couldn't be reproduced because while we review the proposal, another PR fixed it. Although, my proposal came first (13/6/2023) the proposal (17/6/2023) of the merged PR

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

melvin-bot bot commented Jul 3, 2023

@robertjchen @michaelhaxhiu @Santhosh-Sellavel @dukenv0307 this issue is now 3 weeks old. There is one more week left before this issue breaks WAQ and will need to go internal. What needs to happen to get a PR in review this week? Please create a thread in #expensify-open-source to discuss. Thanks!

@melvin-bot
Copy link

melvin-bot bot commented Jul 3, 2023

@robertjchen, @michaelhaxhiu, @Santhosh-Sellavel, @dukenv0307 Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

@Santhosh-Sellavel
Copy link
Collaborator

Santhosh-Sellavel commented Jul 3, 2023

Not overdue! Discussing this internally

@melvin-bot melvin-bot bot removed the Overdue label Jul 3, 2023
@michaelhaxhiu
Copy link
Contributor

michaelhaxhiu commented Jul 4, 2023

Ok interesting, seems we missed a dupe issue and spent time/efforts here (which yes... it's a bummer, but it happens sometimes!).

Here's my suggestion:
Let's stop here, don't sink more time on this given it's a dupe!

  • Contributor - $1000 to @dukenv0307 - for scoping the solution (+ potentially you gave time to the redundant PR)
  • Bug reporting bonus - $250 @dhanashree-sawant - for reporting the bug first (before QA caught it)
  • C+ review - $1000 to @Santhosh-Sellavel - for doing C+ review of the proposal

@Santhosh-Sellavel
Copy link
Collaborator

Thanks @michaelhaxhiu!

@michaelhaxhiu michaelhaxhiu changed the title [$1000] App does not refocus to compose box after deleting the message using enter key [Payment due Jul 5 - $1000] App does not refocus to compose box after deleting the message using enter key Jul 4, 2023
@michaelhaxhiu
Copy link
Contributor

Job is here - https://www.upwork.com/jobs/~01488b62e8528b9e9b

Let's start the payment process.

@dhanashree-sawant
Copy link

Thanks @michaelhaxhiu, I have accepted the offer.

@melvin-bot melvin-bot bot added the Overdue label Jul 7, 2023
@anmurali
Copy link

anmurali commented Jul 7, 2023

Paid Santhosh on New Dot.

@robertjchen
Copy link
Contributor

Looks like @dhanashree-sawant is all set, so I checked them off the list #20619 (comment) Closing this out 🎉

@melvin-bot melvin-bot bot removed the Overdue label Jul 10, 2023
@dhanashree-sawant
Copy link

Hi @robertjchen, @michaelhaxhiu I have recieved the offer but still needs to be approved, can you check once available?

@michaelhaxhiu
Copy link
Contributor

all are paid

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

8 participants