[HOLD for payment on June 15th] Edit comment - Saving an unedited comment shows the edited status for a brief moment

[isagoico]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

Upwork Posting: https://www.upwork.com/jobs/~014b1b019c459a02d5

---

Expected Result:

If the message was not changed the edited status should not appear.

Actual Result:

Edited status appears for a brief moment after saving a message (without changes)

Action Performed:

1. Navigate to a conversation in e.cash
2. Click on edit comment on a message
3. Press save without making any changes

Workaround:

No need, visual issue.

Platform:

Where is this issue occurring?

Web ✔️
iOS ✔️
Android (another behaviour, please check notes)
Desktop App
Mobile Web ✔️

Version Number: 1.0.56-0

Logs:

Notes/Photos/Videos:
On android app, the edited status persists.

Recording.186.mp4

Expensify/Expensify Issue URL:

View all open jobs on Upwork

---

From @aliabbasmalik8 https://expensify.slack.com/archives/C01GTK53T8Q/p1622188890359100

On Edit Comment, When we clicked enter button without edit any comment then (edited) tag appear for short duration

[MelvinBot]

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

[marcaaron]

Looks like the correct behavior would be for this text to persist.

[MelvinBot]

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

[marcaaron]

Or maybe not because it wasn't "actually" edited? Not too sure. Either way, it definitely shouldn't flash like that.

[parasharrajat]

Backend says isEdited:false due to the reason message does not really change but we optimistically save isEdited:true . so the ideal solution would be to do the string comparison and then trigger the edit only if not the same (===).

[parasharrajat]

Proposal

1. We optimistically set the isEdited Status of the action here https://github.com/Expensify/Expensify.cash/blob/790a7545701e950b474c71b42ceb87e8ea352f29/src/libs/actions/Report.js#L1208
2. I checked that we don't change the message at all and send the Report_EditComment request to backend backend sends us back isEdited:false via pusher which then overwrite out optimistic status.
3. This behaviour can be fixed but checking the message string before sending the request to backend.
   We just do the html string comparision here
   https://github.com/Expensify/Expensify.cash/blob/790a7545701e950b474c71b42ceb87e8ea352f29/src/libs/actions/Report.js#L1205
   like

 if(newReportAction.message[0].html === htmlForNewComment){
  return; // we break the further processing and stop is edited status from being set.
 }

It seems that the backend ignores the ending whitespaces as well. So it may be doing the text comparisons. I am not sure that I should compare the HTML or text. @Jag96 Could you please clarify this?

[pROFESOR11]

Proposal

1. In ReportActionItemMessageEdit, we can check if the new message string is different than the initial value.
2. Optionally we can show disabled Save changes button if the current message string is exactly the same with the initial value.
3. I think for UX, users should still exit edit mode onPress Enter, even if the values are the same.

I think the most efficient way to achieve these is that if the values are the same, making publishDraft the same functionality with deleteDraft.

publishDraft() {
    if (this.state.draft !== this.props.draftMessage?.trim()) {
        editReportComment(this.props.reportID, this.props.action, this.state.draft);
    }
    this.deleteDraft();
}

...
 
 render() {
        return (
            ...
                ...
                    <Button
                        success
                        style={[styles.mr2]}
                        onPress={this.publishDraft}
                        text="Save Changes"

                        // optionally show disabled save button
                        // isDisabled={this.state.draft === this.props.draftMessage?.trim()}
                    />
                ...
            ...
        )
}

expensify_issue3219.mov

[Jag96]

Both proposals look like they'll work, going to go with @parasharrajat's since he added his first. Regarding HTML vs text comparison, you can compare the HTML in this case since that's what the back end is doing. I also don't think we need any UI updates since there isn't any way for the user to know that adding a space to the message doesn't count as an edit.

@parasharrajat I've invited you to the job on Upwork, once it's accepted feel free to create a PR.

[pROFESOR11]

@Jag96 Okay, @parasharrajat 's solution doesn't provide a way to disable Save Changes button on the frontend, so if you prefer to have it in place, please feel free to use my approach as well.

[Jag96]

Reopening to update title for payment

[Jag96]

Paid!