[HOLD for payment 2024-10-24] [$250] Forward delete moves cursor location after deleting a character

[m-natarajan]

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

---

Version Number: 9.0.31-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: @tgolen Slack conversation: https://expensify.slack.com/archives/C05LX9D6E07/p1725900088874679

Action Performed:

1. Initiate a DM with any user
2. Compose a message
3. Place the cursor in the middle of the composed message
4. Press the Delete button on the keyboard

   Expected Result:

   Pressing the delete key should delete characters after the cursor, then the cursor should remain in its current opsition.

   Actual Result:

   ~Pressing the delete key is deleting characters before the cursor~ After deleting the character after the cursor (expected), the cursor advances one character to the left (unexpected).

   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: mWeb Chrome
• [ ] iOS: Native
• [ ] iOS: mWeb Safari
• [x] MacOS: Chrome / Safari
• [x] MacOS: Desktop

Screenshots/Videos

https://github.com/user-attachments/assets/6c40d39f-7823-4358-ba55-4e246b4e23f6

Add any screenshot/video evidence

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021834326245043110651
• Upwork Job ID: 1834326245043110651
• Last Price Increase: 2024-09-12
• Automatic offers:
• ishpaul777 | Reviewer | 103988646
• QichenZhu | Contributor | 103988649

Issue Owner

Current Issue Owner: @mallenexpensify / @jliexpensify

[melvin-bot[bot]]

Triggered auto assignment to @jliexpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[melvin-bot[bot]]

Triggered auto assignment to @youssef-lr (AutoAssignerNewDotQuality)

[BreakTos]

Can this issue be assigned to me ? Looks like something i can contribute to

[melvin-bot[bot]]

📣 @BreakTos! 📣 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. Make sure you've read and understood the contributing guidelines.
2. 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.
3. 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.
4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details. Format:

   Contributor details
   Your Expensify account email: <REPLACE EMAIL HERE>
   Upwork Profile Link: <REPLACE LINK HERE>

[BreakTos]

Contributor details Your Expensify account email: parthseth2004@gmail.com Upwork Profile Link: https://www.upwork.com/freelancers/~0158b590a3a6ca253f

[melvin-bot[bot]]

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

[Shahidullah-Muffakir]

In the video shared, the Delete key is erasing characters to the right of the cursor, which I also tested and found to be true. I believe this is the expected behavior. As the Backspace key is meant to delete characters to the left of the cursor, while the Delete key should erase characters to the right. Given this, it seems like the Delete key is working as it should.

[tylerkaraszewski]

The video is recorded on Windows, not MacOS.

[muttmuure]

I'm not sure we've established that this is unexpected behavior here

[rafecolton]

The bug description is incorrect. It is not deleting the character before the cursor, which would be unexpected for forward delete. It is deleting the character after (expected), but then advancing the cursor one position to the left (unexpected). The expected behavior is that the cursor remains where it is after the character to the right is deleted. Reported here for macOS too.

[rafecolton]

Happy to take over this issue @youssef-lr, I will be joining the NewDot Quality and ECM chores very soon.

[melvin-bot[bot]]

@tgolen, @youssef-lr, @jliexpensify Whoops! This issue is 2 days overdue. Let's get this updated quick!

[rafecolton]

Per conversation here, I'm taking this over and will solicit proposal for it. I'll also update the OP to correctly state the bug. Not sure if this should be a NewDot Quality issue or not, so discussing that in that thread.

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[QichenZhu]

Proposal

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

The caret is in the wrong position after deleting a character with the Delete key in Composer.

What is the root cause of that problem?

The line below from the live markdown library calculates the new caret position:

https://github.com/Expensify/react-native-live-markdown/blob/0f8f21cb4e7379d8a20111f6023d274536ad5c1e/src/MarkdownTextInput.web.tsx#L297

const newCursorPosition = Math.max(Math.max(contentSelection.current.end, 0) + (parsedText.length - previousText.length), 0);

In the case of this issue, parsedText.length - previousText.length equals -1, which moves the caret one character backward before the selection end.

This behavior is correct when deleting backward or deleting forward with at least one character selected but incorrect when deleting forward with no characters selected.

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

Calculate the new caret position as below:

const newCursorPosition = (inputType === 'deleteContentForward' && contentSelection.current.start === contentSelection.current.end)
    ? Math.max(contentSelection.current.start, 0) // Don't move the caret when deleting forward with no characters selected.
    : Math.max(Math.max(contentSelection.current.end, 0) + (parsedText.length - previousText.length), 0);

What alternative solutions did you explore? (Optional)

N/A

[ishpaul777]

Proposal from @QichenZhu Looks good and works well

https://github.com/user-attachments/assets/9c188648-5399-4935-8624-37b13d59c869

🎀 👀 🎀 C+ reviewed

[melvin-bot[bot]]

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

[rafecolton]

I do the CME chore so not sure why Jules was assigned. Maybe the response to the C+ reviewed message looks at the team instead of the chore. I'll check it out.

[rafecolton]

Ah I was OOO on Friday, that's why 👍

[ishpaul777]

@rafecolton Wdyt about the recomendation https://github.com/Expensify/App/issues/48797#issuecomment-2350376623

[rafecolton]

Was just looking at it. It looks great, and it worked when I tested it locally. Assigning them now.

[melvin-bot[bot]]

📣 @ishpaul777 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link Upwork job

[melvin-bot[bot]]

📣 @QichenZhu 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link Upwork job 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 📖

[QichenZhu]

@ishpaul777 PR is up for review by react-native-live-markdown maintainers.

cc: @rafecolton

[Skalakid]

Hello, we've merged the PR that fixes forward deleting in Live Markdown Input (version 0.1.154). I will bump the version of the library in E/App to 0.1.155 in my PR and add the test steps for this issue to the description. I need to do it since we have to include some bug fixes for my PR that were added after @QichenZhu's changes. Is that okay for you?

[rafecolton]

@Skalakid updated your PR body to add $ <issue link> since that solves this issue too 👍

Thanks for the communication!

[rafecolton]

Not overdue Melvin

[rafecolton]

Confirmed @Skalakid's PR includes this fix, so now we'll wait for that to be merged and deployed.

[mallenexpensify]

When I hold down the fn key and tap delete to remove text in front of the cursor, it doesn't work. Will that bug be fixed here too?

[QichenZhu]

When I hold down the fn key and tap delete to remove text in front of the cursor, it doesn't work. Will that bug be fixed here too?

Tested and it works after the fix.

https://github.com/user-attachments/assets/5893c193-f121-4ef7-826e-8e4bbdc909bd

[rafecolton]

Do you have a video @mallenexpensify? I bet if you take a closer look, you'll see it does work and just doesn't look like it worked because it moves the cursor position afterwards. But it does indeed delete the character after the cursor in my experience.

[mallenexpensify]

Your bug might also be a part of mine. The functionality of fn+delete def isn't working correctly, which you might be able to see in the vid.
https://github.com/user-attachments/assets/50315d7c-f4e8-4ad5-836c-11b99d19b021

Thanks @QichenZhu for confirming!

[rafecolton]

@mallenexpensify in your video, the functionality is working correctly. If you look closely, you are in fact deleting the character to the right of the cursor. This is exactly the same as the bug that is being fixed here.

[melvin-bot[bot]]

@rafecolton, @jliexpensify, @QichenZhu, @ishpaul777 Whoops! This issue is 2 days overdue. Let's get this updated quick!

[ishpaul777]

Update for melving ^, E/react-native-live-markdown https://github.com/Expensify/react-native-live-markdown/pull/496 was merged, we are just waiting on the bump version to latest E/react-native-live-markdown version https://github.com/Expensify/App/issues/40181

[muttmuure]

I'm not 100% sure this is a quality bug, but it is nearly fixed anyway, so I'm going to remove it from the project to focus the scope there.

[rafecolton]

Sounds good, thanks Matt

[melvin-bot[bot]]

@rafecolton, @jliexpensify, @QichenZhu, @ishpaul777 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

[rafecolton]

Not overdue. PR was recently merged, waiting for this change to be deployed.

[jliexpensify]

Hi @rafecolton - just a heads up I'm OOO from 3rd to 14th, so I will prep a Payment Summary. If needed, feel free to reassign to another BZ for payment.

• C: @QichenZhu owed $250 via Upwork
• C+: @ishpaul777 owed $250 via Upwork

Upwork job

[mallenexpensify]

@jliexpensify you can assign to me for payment, if needed.

When you post payment summaries, can you use the below format? For auditing purposes we need to know where someone is being paid (Upwork vs NewDot) and also if they're due/owed funds are 'paid'. Thx

[jliexpensify]

Will do in the future, thanks @mallenexpensify!

[ishpaul777]

unfortunately, E/react-native-live-markdown version bump was reverted because of unrelated changes to our issue, we have new PR https://github.com/Expensify/App/pull/50047 for second try 🤞

[melvin-bot[bot]]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot[bot]]

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

• https://github.com/Expensify/App/pull/50047

If no regressions arise, payment will be issued on 2024-10-24. :confetti_ball:

For reference, here are some details about the assignees on this issue:

• @QichenZhu requires payment automatic offer (Contributor)
• @ishpaul777 requires payment automatic offer (Reviewer)

[melvin-bot[bot]]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [ ] [@ishpaul777] The PR that introduced the bug has been identified. Link to the PR:
• [ ] [@ishpaul777] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [ ] [@ishpaul777] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [ ] [@ishpaul777] Determine if we should create a regression test for this bug.
• [ ] [@ishpaul777] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [ ] [@mallenexpensify / @jliexpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[mallenexpensify]

whoohooo... it's fixed!! Thx

[jliexpensify]

Hi @ishpaul777 - do we need a checklist here?

[ishpaul777]

i could not find PR that introduce this bug, But i think we need regression tests for this.

Regression Test Proposal:

1. Initiate a DM with any user
2. Compose a message
3. Place the cursor in the middle of the composed message
4. Press the fn key and press Delete button on the keyboard
5. Verify Expected Result: Pressing the delete key should delete characters after the cursor, then the cursor should remain in its current opsition.

Do we agree 👍 or 👎