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

Compose box is not cleared after sending a message if the user starts typing just after pressing send - iOS / Android #4129

Closed
isagoico opened this issue Jul 17, 2021 · 75 comments
Labels
Engineering Improvement Item broken or needs improvement. Internal Requires API changes or must be handled by Expensify staff Monthly KSv2 Not a priority

Comments

@isagoico
Copy link

isagoico commented Jul 17, 2021

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. Log in to e.cash in mobile and navigate to a conversation
  2. Type a message and press send
  3. Immediately after pressing send start typing another message

Expected Result:

Compose box should be cleared after sending the message

Actual Result:

Previous message is still in compose box if the user starts typing just after pressing send

Workaround:

Waiting a bit after pressing send clears the compose box. But this could be a pain for fast typers.

Platform:

Where is this issue occurring?

Web
iOS ✔️
Android ✔️
Desktop App
Mobile Web

Version Number: 1.0.79-0

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

Notes/Photos/Videos:

Screen.Recording.2021-07-15.at.12.47.54.PM.mov

Expensify/Expensify Issue URL:

View all open jobs on Upwork


From @aliabbasmalik8 https://expensify.slack.com/archives/C01GTK53T8Q/p1626378713340800 and @joaniew https://expensify.slack.com/archives/C01GTK53T8Q/p1626387085365100

Send message takes some time so last message append with next message.
we can clear the compose box after he click on send icon. (@aliabbasmalik8)

i noticed on mobile (iOS) there’s a lag between when i hit ‘send’ and when it actually sends. but i’m the type of typer who will start typing a new message expecting the chat box to clear once the message has been sent. what happens in echat is that the message sends, but if i don’t wait for it to send/clear the msg box and start typing immediately, it keeps the original message, which means i have to either go back and delete it or wait until it sends (who knows how long that takes sometimes. Video attached (wait til the 2nd testing chat) (@joaniew)


IMPORTANT PLEASE READ

Instructions on how to make a proposal for this issue:

  • Read the entire comment thread to be sure you are not proposing a solution that was already discussed
  • Provide a clear explanation of the problem and why it happens
  • Provide a clear explanation of the solution and how to implement it
  • Provide proof that the solution will solve the issue on iOS/Android (ideally a video of native platform - not web)
  • Provide enough detail so that we can implement your solution and verify that it works.

Proposals without this information will be marked as off-topic. Thanks!

@MelvinBot
Copy link

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

@robertjchen robertjchen added Weekly KSv2 Improvement Item broken or needs improvement. and removed Daily KSv2 labels Jul 22, 2021
@robertjchen
Copy link
Contributor

I was able to reproduce this behavior in the iOS emulator. Interestingly enough, it doesn't seem to be an issue on the Web-based client.

After unsuccessfully trying some quick JS changes, I think the potential solution might have to involve low-level, native control of the input field behavior. Opening it up for others to try tackling it, perhaps a more suitable JS solution can be found! 👍

@robertjchen robertjchen added the External Added to denote the issue can be worked on by a contributor label Jul 29, 2021
@MelvinBot
Copy link

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

@robertjchen robertjchen removed their assignment Jul 29, 2021
@MelvinBot MelvinBot added Daily KSv2 and removed Weekly KSv2 labels Jul 29, 2021
@bfitzexpensify

This comment has been minimized.

@MelvinBot MelvinBot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Jul 29, 2021
@MelvinBot
Copy link

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

@muratti32
Copy link

the problem seems to be related to state management

@sonipranjal
Copy link

I can easily fix this; it seems to be the problem because the input field is not controlled. (state management problem) My Upwork profile: https://www.upwork.com/freelancers/~014e072a4daf3ed991
Thanks

@marcaaron
Copy link
Contributor

the problem seems to be related to state management

@muratti32 How so?

I can easily fix this; it seems to be the problem because the input field is not controlled. (state management problem)

@sonipranjal We had a controlled input used in the past. The performance was very bad.

@marcaaron
Copy link
Contributor

If it helps, here's where we clear the comment by calling this method with an empty string

this.updateComment('');
this.setTextInputShouldClear(true);

updateComment(newComment) {
this.textInput.setNativeProps({text: newComment});
this.setState({
isCommentEmpty: newComment.length === 0,
});
this.comment = newComment;
this.debouncedSaveReportComment(newComment);
this.debouncedBroadcastUserIsTyping();
}

I'm not certain, but if I had to guess there's some conflict between the logic that is doing

setTextInputShouldClear() (which will trigger this.textInput.clear()) and setNativeProps({text: ''})

@mananjadhav
Copy link
Collaborator

mananjadhav commented Jul 29, 2021

Proposal

We can try running the submitting after clearing the textField.

setTextInputShouldClear(shouldClear, fn) {
    this.setState({textInputShouldClear: shouldClear}, fn ? fn() : undefined);
  }

submitForm(e) {
    if (e) {
      e.preventDefault();
    }

    const trimmedComment = this.comment.trim();
    // Don't submit empty comments
    if (!trimmedComment) {
      return;
    }
    
    this.updateComment('');
    this.setTextInputShouldClear(true, () => {
      this.props.onSubmit(trimmedComment);
    });

  }

@marcaaron
Copy link
Contributor

We can try running the submitting after clearing the textField.

@mananjadhav Interesting, is there some explanation for why this would help?

@arpitdeveloper
Copy link

we update the submitForm in "src/pages/home/report/ReportActionCompose.js"
this.setTextInputShouldClear(true); is use the first line in submitForm

@MelvinBot
Copy link

@marcaaron, @bfitzexpensify Whoops! This issue is 2 days overdue. Let's get this updated quick!

@harshilJs
Copy link

harshilJs commented Aug 3, 2021

I would try to clear input first and then trigger the send message functionality. clearing input is a state update so it should not affect the sending message time. and yes updating the state is the only solution I see here, there can be problem with re-rendering and we can optimise the component to avoid re-render by making some components Pure or by using useMemo if that is not implemented yet.

@bzaman
Copy link

bzaman commented Aug 4, 2021

once it submit the message from state, we just need to clear/empty state again.

@marcaaron marcaaron added Daily KSv2 Internal Requires API changes or must be handled by Expensify staff and removed Help Wanted Apply this label when an issue is open to proposals by contributors Exported External Added to denote the issue can be worked on by a contributor Daily KSv2 labels Sep 3, 2021
@marcaaron
Copy link
Contributor

Thanks for everyone's input. Based on the complexity of the solutions suggested and lack of a clear solution we're going to take some time to investigate this internally before proceeding. Please let us know in the Slack channel if any new ideas pop up.

@Expensify Expensify locked and limited conversation to collaborators Sep 3, 2021
@marcaaron marcaaron removed their assignment Sep 3, 2021
@mallenexpensify mallenexpensify removed their assignment Sep 3, 2021
@mallenexpensify
Copy link
Contributor

Job closed in Upwork

@MelvinBot
Copy link

This issue has not been updated in over 15 days. eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

@MelvinBot
Copy link

@isagoico, this Monthly task hasn't been acted upon in 6 weeks; closing.

If you disagree, feel encouraged to reopen it -- but pick your least important issue to close instead.

@mallenexpensify
Copy link
Contributor

mallenexpensify commented Dec 10, 2021

This is still happening on v1.1.18-3, iOS. I'm reopening cuz it's a bug and we should fix it, sooner or later.
@marcaaron is there a downside of continuing to double this job until we get accept a solution? Or.. is this bug associated with something else internally we need to take into consideration?

@MelvinBot
Copy link

@isagoico, this Monthly task hasn't been acted upon in 6 weeks; closing.

If you disagree, feel encouraged to reopen it -- but pick your least important issue to close instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Engineering Improvement Item broken or needs improvement. Internal Requires API changes or must be handled by Expensify staff Monthly KSv2 Not a priority
Projects
None yet
Development

No branches or pull requests