[$500] Chat - Unable to remove emoji after inserting emoji using undo shortcut

[izarutskaya]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Version Number: v1.4.2-0
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
Expensify/Expensify Issue URL:
Issue reported by: Applause-Internal Team
Slack conversation: @

Action Performed:

1. Navigate to staging.new.expensify.com.
2. Go to any chat.
3. Click on the emoji picker.
4. Select an emoji.
5. Press CMD+Z to undo the emoji selection.

Expected Result:

The emoji will be removed.

Actual Result:

The emoji cannot be removed using undo shortcut.

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Bug6287178_1700659510506.20231122_191947.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01c8ca033c48a35540
• Upwork Job ID: 1727341395598020608
• Last Price Increase: 2023-12-06

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~01c8ca033c48a35540

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[suneox]

Proposal

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

Unable to remove emoji after inserting emoji using undo shortcut

What is the root cause of that problem?

By default input history is managed by the browser, it only keeps history when a user interacts by typing on an input field. in this case, the value state passes to input the same with history value so we still can use undo, redo feature. But when we update the value is different from the input history value then the current history on the input field will be reset and that is a native behavior.
For this issue when we insert emoji at this line we have replaceSelectionWithText to update the value state diffirent with current input history so we can not use undo, redo feature.

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

We can manually addEventListener when key down to handle history via the state of the composer instead of using default behavior of browser.
We will created the useInputHistory hook to handle all the cases of undo, redo, and manage history state

i have implemented this hook at this file

Then using this hook at Composer

   ......
    useInputHistory({
        value,
        textInput,
        onChangeText: props.onChangeText,
    });
    return (
    )
    ....

POC

31712.mp4

What alternative solutions did you explore? (Optional)

We can use execCommand function exiting on document to insert emoji at onEmojiSelected function, for this function it still keep history for us but execCommand is deprecated

  onEmojiSelected={(emoji) => {
      ReportActionComposeFocusManager.focus();
      document.execCommand('insertText', false, `${emoji} `);
  }}

We have 3 cases that need to be updated typing, select from picker and select from suggestion so we will create a function insertEmoji inside ComposerWithSuggestions.js with 2 parameters text and selection and then insertText base on selection param

[s77rt]

@suneox Thanks for the proposal. I don't think your RCA is correct / complete. If we are updating the text without updating the history then I'd expect the undo action to skip a step and keep working. Also what about the case where the emoji is typed (and not selected from the picker)?

[suneox]

Also what about the case where the emoji is typed (and not selected from the picker)?

I have missed information for this case, To handle this case we will create a function insertEmoji inside ComposerWithSuggestions.js to setSelection for text input then exec insertText command

After that we will update using at insertSelectedEmoji function, also update at onEmojiSelected function

[s77rt]

@suneox Can you please elaborate your RCA first? How exactly is the history lost?

[suneox]

@suneox Can you please elaborate your RCA first? How exactly is the history lost?

By default input history is managed by the browser, it only keeps history when a user interacts by typing on an input field. in this case, the value state passes to input the same with history value so we still can use undo, redo feature. But when we update the value is different from the input history value then the current history on the input field will be reset and that is a native behavior.

[shubham1206agra]

@suneox Please see https://developer.mozilla.org/en-US/docs/Web/API/Document/execCommand. execCommand is deprecated and is not recommended.

[suneox]

@shubham1206agra

Please see https://developer.mozilla.org/en-US/docs/Web/API/Document/execCommand. execCommand is deprecated and is not recommended.

Yes I know but exeCommand is still used in other places, I'm not sure using Selection API will work for both Android and iOS so just keep it consistent with another places

[s77rt]

@suneox Thanks for the clarification. Indeed this is a browser bug/limitation as I was able to reproduce it using just html input. Can you please update your solution to cover both cases? (typing the emoji code and selecting it from the picker)

[suneox]

@suneox Thanks for the clarification. Indeed this is a browser bug/limitation as I was able to reproduce it using just html input. Can you please update your solution to cover both cases? (typing the emoji code and selecting it from the picker)

Yes we have 3 cases need to update include typing, select from picker and select from suggestion, I'll update detail asap

[suneox]

Hi @s77rt I have updated my proposal and I have moved the alternative solution to the primary solution, due to execCommand is deprecated. You can checkout my branch for testing

[s77rt]

@suneox Thanks for the update. I think the history approach is an overkill for a bug that is browser limitation related. execCommand is deprecated indeed but it may be a better option here. Can you please update your solution for the execCommand approach just in case we wanted to go that way.

[suneox]

@suneox Thanks for the update. I think the history approach is an overkill for a bug that is browser limitation related. execCommand is deprecated indeed but it may be a better option here. Can you please update your solution for the execCommand approach just in case we wanted to go that way.

sure I'll update ASAP but i'd like to confirm a proposal has using some deprecated feature doesn't reject by code review?

[s77rt]

The proposal will be reviewed by an internal engineer and can be escalated to a Slack thread for more engineers review. It could be rejected, although we already use it in our codebase this is not a strong argument to use it on other places.

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[suneox]

It could be rejected, although we already use it in our codebase this is not a strong argument to use it on other places.

@s77rt Due to execCommand solution can be rejected so should we go a head using history hook? and Could you please try the hook on my proposal?

[s77rt]

@suneox I think we should try explore more solutions. The history approach looks an overkill here and I'd prefer to avoid it.

[suneox]

@s77rt I'd like to inform the alternative solution for the feature undo redo from Selection API instead of deprecated exeCommand it's doesn't work after insertNode or replaceChild with default behavior of browser. and exeCommand also can not cover case undo when typing. So could you please consider we will applying a history management solution? and you can try my implementation on this branch

[s77rt]

@suneox I would really prefer to avoid the history approach. Also please note that the solution should be explained in plain English without code diff, PRs, etc. (you can use pseudo code though)

[melvin-bot]

@sakluger @s77rt this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[s77rt]

Not close Melvin. This is arguably a browser limitation/design. Still looking for proposals

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[suneox]

@suneox I would really prefer to avoid the history approach.

Hi @s77rt @sakluger, I think in the future this approach is better and currently I have implemented it for this issue we can handle undo, redo, reset by a manual or shortcut.

If we don't want to apply the history management approach, I think we can only choose a solution using exeCommand.
From my research zulip chat also has the same issue with us, you can go through their issue and they resolve this issue with text-field-edit library. Inside the library also uses exeCommand to insert and replace text. When I get the assignment I'll implement it for our app.

[s77rt]

Asked on Slack https://expensify.slack.com/archives/C01GTK53T8Q/p1701901193272099

[s77rt]

Not overdue. Still discussing on slack

[mountiny]

Coming from the Slack thread, this is a minor issue with a simple workaround and hard/ hacky fix so I am going to close this out