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 2021-11-29] $500 Old Edited comments that are still open trigger the keyboard when the user is scrolling the conversation - Reported by: @parasharrajat #5896

Closed
isagoico opened this issue Oct 15, 2021 · 27 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Engineering External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review Weekly KSv2

Comments

@isagoico
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. Open any chat.
  2. Scroll back in the history.
  3. Edit the message. Leave it as it is. Don't close the editor
  4. Go back to LHN.
  5. Open any other chat.
  6. back to LHN.
  7. Open back the previous chat.
  8. Scroll back in the list slowly.
  9. Screen will jump to the message being edited as soon as you reach closed to it.

Expected Result:

Old edited comment that are still open should not trigger the keyboard when the user is scrolling.

Actual Result:

Old Edited comments that are still open trigger the keyboard when the user is scrolling the conversation

Workaround:

Close or save the comments that are being edited.

Platform:

Where is this issue occurring?

  • iOS
  • Android

Version Number: 1.1.8-0

Reproducible in staging?: Yes
Reproducible in production?: Yes

Logs: https://stackoverflow.com/c/expensify/questions/4856

Notes/Photos/Videos: Any additional supporting documentation

Screencast.2021-10-09.21_25_07.mp4

Expensify/Expensify Issue URL:

Issue reported by: @parasharrajat
Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1633794986065800

View all open jobs on GitHub

@MelvinBot
Copy link

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

@thienlnam thienlnam added the External Added to denote the issue can be worked on by a contributor label Oct 15, 2021
@MelvinBot
Copy link

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

@thienlnam thienlnam removed their assignment Oct 15, 2021
@parasharrajat
Copy link
Member

Video

output_file.mp4

@SofiedeVreese
Copy link
Contributor

Posted on Upwork: https://www.upwork.com/jobs/~018f79ec6f97efcf87

Adding Exported Label.

@MelvinBot MelvinBot added Weekly KSv2 Help Wanted Apply this label when an issue is open to proposals by contributors and removed Daily KSv2 labels Oct 17, 2021
@MelvinBot
Copy link

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

@NikkiWines
Copy link
Contributor

Hmm this is kind of an interesting edge case... I think how we fix this depends on how we want to handle moving away from a chat with an active edit. Posted in #expensify-open-source here to get some thoughts on how to move forward.

@parasharrajat
Copy link
Member

I think, Keyboard should only open when the user clicks to edit a message aka User action trigger.

@SofiedeVreese
Copy link
Contributor

Hey @NikkiWines is this a dupe from #4756 that has been reopened? Feels like this case would fall under that other GH?

@NikkiWines
Copy link
Contributor

mmm, I'd say they're related but not sure if they're exactly the same issue. In this one, it's more that navigating back to this chat and scrolling over an old open message will retrigger keyboard to pop up.

@parasharrajat
Copy link
Member

They are completely different.
This => explained above.
#4756 => has to do with draft messages.

@SofiedeVreese
Copy link
Contributor

Thanks for the explanation both, we'll keep this issue open then!

@SofiedeVreese
Copy link
Contributor

Price increased to $500

@SofiedeVreese SofiedeVreese changed the title Old Edited comments that are still open trigger the keyboard when the user is scrolling the conversation - Reported by: @parasharrajat $500 Old Edited comments that are still open trigger the keyboard when the user is scrolling the conversation - Reported by: @parasharrajat Oct 27, 2021
@parasharrajat
Copy link
Member

Proposal

AFAIK, I think there is only one way we can keep both autofocus and fix this issue. If we only autofocus, when the user edits messages (trigger edit action). Then old message being edited will not be focused when the user is scrolling. I think this is ideal behaviour as well.

If the user is scrolling back in the history, he should click on the edit box to focus it, instead of autofocus. But if a user press edits on any message, then it should be autofocused.

To do this:

  1. Straight way to do this would be to capture the edit trigger in ReportActionItem via.
    componentDidUpdate(prevProps) {
        if (!prevProps.draftMessage && this.props.draftMessage) {
            // eslint-disable-next-line react/no-did-update-set-state
            this.setState({isMessgeEdited: true});
        }
    }

Then pass the Autofocus prop to ReportActionItemMessageEdit which will control the autofocus on underlined text input.

@NikkiWines
Copy link
Contributor

I think that makes sense. cc: @SofiedeVreese would you hire @parasharrajat for this one?

@SofiedeVreese
Copy link
Contributor

Yes sure can @NikkiWines.

@parasharrajat can you submit your Upwork proposal so I can hire you there? Thanks!

@parasharrajat
Copy link
Member

@SofiedeVreese Job is private.

@SofiedeVreese
Copy link
Contributor

@parasharrajat that should work better now! https://www.upwork.com/jobs/~018f79ec6f97efcf87

@SofiedeVreese
Copy link
Contributor

I've just hired you in Upwork @parasharrajat !

@SofiedeVreese
Copy link
Contributor

PR merged and QA ongoing.

@SofiedeVreese SofiedeVreese changed the title $500 Old Edited comments that are still open trigger the keyboard when the user is scrolling the conversation - Reported by: @parasharrajat [HOLD for payment 2021-11-23] $500 Old Edited comments that are still open trigger the keyboard when the user is scrolling the conversation - Reported by: @parasharrajat Nov 15, 2021
@SofiedeVreese
Copy link
Contributor

PR deployed, adding a payment hold 7 days from deploy.

@botify botify added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Nov 22, 2021
@botify botify changed the title [HOLD for payment 2021-11-23] $500 Old Edited comments that are still open trigger the keyboard when the user is scrolling the conversation - Reported by: @parasharrajat [HOLD for payment 2021-11-29] [HOLD for payment 2021-11-23] $500 Old Edited comments that are still open trigger the keyboard when the user is scrolling the conversation - Reported by: @parasharrajat Nov 22, 2021
@botify
Copy link

botify commented Nov 22, 2021

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.1.15-15 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

If no regressions arise, payment will be issued on 2021-11-29. 🎊

@SofiedeVreese SofiedeVreese changed the title [HOLD for payment 2021-11-29] [HOLD for payment 2021-11-23] $500 Old Edited comments that are still open trigger the keyboard when the user is scrolling the conversation - Reported by: @parasharrajat [HOLD for payment 2021-11-29] [HOLD for payment 2021-11-29] $500 Old Edited comments that are still open trigger the keyboard when the user is scrolling the conversation - Reported by: @parasharrajat Nov 23, 2021
@NikkiWines NikkiWines changed the title [HOLD for payment 2021-11-29] [HOLD for payment 2021-11-29] $500 Old Edited comments that are still open trigger the keyboard when the user is scrolling the conversation - Reported by: @parasharrajat [HOLD for payment 2021-11-29] $500 Old Edited comments that are still open trigger the keyboard when the user is scrolling the conversation - Reported by: @parasharrajat Nov 29, 2021
@SofiedeVreese
Copy link
Contributor

Job paid out & contract cancelled. Closing GH.

@parasharrajat
Copy link
Member

@SofiedeVreese I think you missed the reporting bonus.

@SofiedeVreese
Copy link
Contributor

Yes I did, sorry @parasharrajat ! I'll pay that one out now.

@SofiedeVreese
Copy link
Contributor

Ok that bonus is now also paid out, sorry about that @parasharrajat !

@mallenexpensify
Copy link
Contributor

mallenexpensify commented Dec 3, 2021

It looks like I forgot to add the Company Offsite hold label to this issue. I've added the bonus for the hold to his Upwork payment.
The timeline was... Nov 8th I did a pass through all open issues in this repo and added the Company Offsite Hold label to most. I missed this one somehow and I should have added it.
The hold was lifted/removed on 11/15
The PR was deployed on 11/22

So... my thinking is .. this issue is eligble for the hold bonus. Sorry for missing it and any confusion.

@NikkiWines
Copy link
Contributor

NikkiWines commented Dec 3, 2021

That makes sense to me, I was thinking it was ineligible for the n6-hold bonus but hadn't even considered the company offsite bonus (i.e. didn't consider this issue could've spanned multiple holds) and should've compared the dates as well. My bad and thanks for clearing things up @mallenexpensify

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 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

8 participants