[Pay meow][$250] Web - Composer - Cursor positioned at the start if pasting text which contains a new line #42216

kbecciv · 2024-05-15T20:28:40Z

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: 1.4.74-0
Reproducible in staging?: y
Reproducible in production?: y
Issue reported by: Applause - Internal Team

Action Performed:

Navigate to staging.new.expensify.com
Open a chat and type a one line text and create an empty new line
Select all using keyboard ctrl + A and cut the text
Paste it inside the compose box

Expected Result:

The cursor gets positioned at the end of the pasted text, no console error

Actual Result:

The cursor gets positioned at the start of the pasted text if the pasted text contains a new empty line, console error is showing

Workaround:

n/a

Platforms:

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

Screenshots/Videos

Add any screenshot/video evidence

Bug6481716_1715782332852.bandicam_2024-05-15_17-07-29-894.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~01cc45b59ee9c4ae02
Upwork Job ID: 1792451156946927616
Last Price Increase: 2024-08-08

melvin-bot · 2024-05-15T20:28:51Z

Triggered auto assignment to @Christinadobrzyn (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.

kbecciv · 2024-05-15T20:29:16Z

We think that this bug might be related to #wave-collect - Release 1

kbecciv · 2024-05-15T20:49:24Z

@Christinadobrzyn FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors.

Christinadobrzyn · 2024-05-16T05:29:00Z

Very similar to #42112 - asking if the fix will be the same

moonstar-95515 · 2024-05-16T12:59:48Z

Proposal

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

Web - Composer - Cursor positioned at the start if pasting text which contains a new line

What is the root cause of that problem?

We are setting cursor position in below part when pasting.

App/src/hooks/useHtmlPaste/index.ts

Lines 19 to 20 in fb14d5a

    
           range.setStart(node, node.length); 
        
           range.setEnd(node, node.length);

When pasted text contains empty line, the value of node.length is greater than it's actually length because the length of line break at the end(\n\n) will be caculated as 2.
So console error occurs and cursor is not set because we are trying to set cursor to non-exist position.

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

We have to set range with node.length - 1 when it contains empty line at the end.

What alternative solutions did you explore? (Optional)

melvin-bot · 2024-05-16T12:59:52Z

📣 @moonstar-95515! 📣
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:

Make sure you've read and understood the contributing guidelines.
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.
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.
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>

Christinadobrzyn · 2024-05-20T07:03:44Z

Doesn't seem to be related to #42112 as that is resolved. I can still reproduce this - adding external

melvin-bot · 2024-05-20T07:03:55Z

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

melvin-bot · 2024-05-20T07:03:59Z

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

rakhaxor · 2024-05-20T10:05:14Z

Proposal

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

Web - Composer - Cursor positioned at the start if pasting text which contains a new line

What is the root cause of that problem?

In the text param, newline character(s) at the end (if present) are wrongly interpreted by the cursorUtils library because node.length is calculated including the \n but \n is not inserted in the document node which makes the range go out of bounds.

App/src/hooks/useHtmlPaste/index.ts

Line 10 in e6c9f06

const insertAtCaret = (target: HTMLElement, text: string) => {

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

Creating a new text variable with stripped newline characters at the end or modifying reusing the same text would work fine.

const plainText = text.replace(/\n+$/, '');
And use this in all the places we are using text:

const node = document.createTextNode(text);
insertByCommand(text);

What alternative solutions did you explore? (Optional)

None

RickDeb2004 · 2024-05-20T11:20:31Z

Proposal

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

when pasting text containing a new empty line into the chat compose box, the cursor incorrectly positions itself at the start of the text instead of the end and a console error occurs.

What is the root cause of that problem?

The root cause of the problem is that the cursor positioning logic in the insertAtCaret function incorrectly handles the insertion of text containing new lines. Specifically, the cursor is not being moved to the correct position (the end of the newly inserted text) when the text includes empty new lines, which causes the cursor to appear at the start of the text. Additionally, there is no proper error handling to catch and log any potential issues during the paste operation, leading to console errors.

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

a)
original code :

range.setStart(node, node.length);
       range.setEnd(node, node.length);
       selection.removeAllRanges();
       selection.addRange(range);

modified code:

 range.setStartAfter(node);
        range.setEndAfter(node);
        selection.removeAllRanges();
        selection.addRange(range);

b)
original code :

target.dispatchEvent(new Event('paste', {bubbles: true}));
       // Dispatch input event to trigger Markdown Input to parse the new text
       target.dispatchEvent(new Event('input', {bubbles: true}));
   } else {
       insertByCommand(text);
   };

modified code:

 target.dispatchEvent(new Event('input', {bubbles: true}));
    } else {
        insertByCommand(text);
    }

c)
original code :

const paste = useCallback((text: string) => {
       try {
           const textInputHTMLElement = textInputRef.current as HTMLElement;
           if (textInputHTMLElement?.hasAttribute('contenteditable')) {
               insertAtCaret(textInputHTMLElement, text);
           } else {
               insertByCommand(text);
           }

           // Pointer will go out of sight when a large paragraph is pasted on the web. Refocusing the input keeps the cursor in view.
           textInputRef.current?.blur();
           textInputRef.current?.focus();
           // eslint-disable-next-line no-empty
       } catch (e) {}
       // We only need to set the callback once.
       // eslint-disable-next-line react-hooks/exhaustive-deps
   }, []);

modified code:

 const paste = useCallback((text: string) => {
        try {
            const textInputHTMLElement = textInputRef.current as HTMLElement;
            if (textInputHTMLElement?.hasAttribute('contenteditable')) {
                insertAtCaret(textInputHTMLElement, text);
            } else {
                insertByCommand(text);
            }

            // Pointer will go out of sight when a large paragraph is pasted on the web. Refocusing the input keeps the cursor in view.
            textInputRef.current?.blur();
            textInputRef.current?.focus();
        } catch (e) {
            // ** Added console error log here **
            console.error('Error during paste handling:', e);
        }
    }, []);

Summary of Changes

1.Cursor Positioning Fix:

Updated the insertAtCaret function to use range.setStartAfter(node) and range.setEndAfter(node) to correctly position the cursor at the end of the newly inserted text, especially when new lines are involved.

2.Error Handling:

Added console.error logging within the paste function to handle and log any exceptions that occur during the paste operation, making it easier to debug issues.
These changes ensure that the cursor is positioned correctly after pasting text with new lines and improve error visibility during paste operations.

What alternative solutions did you explore? (Optional)

None

Contributor details
Your Expensify account email: debanjanrick04@gmail.com
Upwork Profile Link: https://www.upwork.com/freelancers/~017738fb94de003574

melvin-bot · 2024-05-20T11:20:35Z

📣 @RickDeb2004! 📣
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:

Make sure you've read and understood the contributing guidelines.
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.
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.
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>

Christinadobrzyn · 2024-05-21T05:13:00Z

hi @eVoloshchak can you take a peek at these proposals when you have a moment? TY!

eVoloshchak · 2024-05-21T17:27:18Z

We have to set range with node.length - 1 when it contains empty line at the end.
@moonstar-95515, that works for this specific case of testing steps, but wouldn't work if pasted text end with 3 (or more) new lines

I think we should proceed with @rakhaxor's proposal, it's clean and straightforward. It does cut off all of the new lines at the end of the pasted text, but that's what the backend does when you send the message, so this can be considered a small improvement

🎀👀🎀 C+ reviewed!

melvin-bot · 2024-05-21T17:27:31Z

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

melvin-bot · 2024-05-21T21:14:02Z

📣 @rakhaxor You have been assigned to this job!
Please apply to the Upwork job and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Once you apply to this job, your Upwork ID will be stored and you will be automatically hired for future jobs!
Keep in mind: Code of Conduct | Contributing 📖

moonstar-95515 · 2024-05-21T21:28:35Z

@eVoloshchak

@moonstar-95515, that works for this specific case of testing steps, but wouldn't work if pasted text end with 3 (or more) new lines

My proposal works when it has 3 or more new lines at the end, as the text will be hello 111\n\n\n\n when it has 3 new lines.
Only last new line is presented with two \n.

moonstar-95515 · 2024-05-21T22:01:40Z

@eVoloshchak cc: @Christinadobrzyn @AndrewGable

It does cut off all of the new lines at the end of the pasted text, but that's what the backend does when you send the message, so this can be considered a small improvement

I can't sure if I understood correctly, but I think only new lines at the end of the text will be cut on backend side.
If we copy text with new lines in the middle of the compose box and paste it again, new lines will be deleted. I think this is not what we want.

test.mp4

rakhaxor · 2024-05-22T13:18:35Z

@eVoloshchak Thank you for accepting my proposal! This is my first time contributing and I just applied to the Upwork job like 3-4 hours ago. Do I need to do some further action before my upwork proposal is accepted and do I need to wait for it to be accepted before I could raise my PR?

moonstar-95515 · 2024-08-09T00:43:11Z

@mallenexpensify
I'm really sorry but I have some problem with applying the job.
Could you please send me offer?
Sorry for inconvenience. Thank you for your effort.

melvin-bot · 2024-08-12T18:05:55Z

@AndrewGable, @eVoloshchak, @rakhaxor, @Christinadobrzyn, @moonstar-95515 Whoops! This issue is 2 days overdue. Let's get this updated quick!

Christinadobrzyn · 2024-08-13T19:21:55Z

Hi @moonstar-95515 sorry for the delay here - I just created a new offer in this Upwork job post - https://www.upwork.com/nx/wm/offer/103512886

Can you see if you can accept that for payment? Thank you!

moonstar-95515 · 2024-08-14T03:14:53Z

@Christinadobrzyn
Thank you.
I've accepted offer.

Christinadobrzyn · 2024-08-15T20:38:23Z

Thanks @moonstar-95515!

I'm a little lost on the payments due for this. I think this is the summary, can we get a review.

Payouts due:

Contributor: $125 @moonstar-95515 (Paid in Upwork - https://www.upwork.com/nx/wm/workroom/38089229/overview)
Contributor: $125 @rakhaxor (Paid in Upwork - https://www.upwork.com/nx/wm/offer/103593092)
Contributor+: $250 @eVoloshchak (paid through NewDot)

moonstar-95515 · 2024-08-15T23:47:01Z

@Christinadobrzyn
Thank you for your summary.
@ rakhaxor was mis-assigned at the first time and not unassigned yet.

mallenexpensify · 2024-08-16T00:46:36Z

@rakhaxor had an accepted proposal and was initially assigned/hired so $125 seems fair, even with @moonstar-95515 working on the PR/issue later.

moonstar-95515 · 2024-08-16T02:21:18Z

@mallenexpensify
Thank you for your explanation.

Of course, I know that I have no face to say anything because my solution has not been reflected in production.
But I still have some unclear points.
Can contributors be compensated even if it is due to a misjudgement of C+ as long as they are assigned to an issue?

rakhaxor · 2024-08-16T04:35:14Z

Hello @Christinadobrzyn, Thank you for the offer. Here’s my upwork: https://www.upwork.com/freelancers/~01f94219e6101472c1

moonstar-95515 · 2024-08-16T05:40:36Z

@mallenexpensify , @Christinadobrzyn
Sorry for bothering again.
Of course, I've already agreed 50% of compensation here.
But I think it's not fair considering my work and @rakhaxor's work in this issue.

Here's summary:

My proposal was the first one and perfect.
But then @rakhaxor 's proposal was selected due to misjudgment. He worked on this issue until it was assigned to me.
Anyway, my proposal was selected here.
Then, I've created PR and I worked on the PR for over a month.

I think I put much more effort in this issue and the same compensation is not fair.
Could you please reconsider about compensation?

rakhaxor · 2024-08-16T07:00:10Z

@moonstar-95515 What’s your problem with me man? I have done all the tests and made screen recordings. You were assigned later on, I did all the work there and was asking to submit PR. I didn’t even comment even after doing all the work and not being able to submit it until now when I was asked to submit my profile link. Anyways, I just need you to not talk about me anymore. Thank you

moonstar-95515 · 2024-08-16T07:55:54Z

@rakhaxor
I don't have any problem with you. I'm just talking about my rights.
Do you really think you have right to be compensated?
I've commented just after you were assigned and you may know that your proposal was wrong. I can't understand why you kept working.
But I'm not saying you shouldn't be compensated, I'm just saying that I put more effort and there must be difference in compensation.
That's all. Thank you.

mallenexpensify · 2024-08-19T18:00:39Z

The decision to compensate @rakhaxor is an edge case, based on the fact they were assigned to the issue and hired. It was the responsibility of Expensify to hire/assign. Since this action is considered a source-of-truth and gives the green light for the contributor to invest more time into the issue (assuming compensation), we feel it's fair to compensate. Since their proposal was initially accepted, we decided on 50%.

@moonstar-95515 , your compensation on this issue, and the value associated with it, is completely unrelated to the compensation for @rakhaxor. Please keep in mind our documented code of conduct when communicating with others, both internal and with fellow contributors.

We pride ourselves as a company and as individual employees on our core qualities: Talent, Ambition, & Humility. The environment we have developed uses these qualities to adhere to our two golden rules of Expensify:

Get Shit Done

Don’t Ruin it for Everyone Else

Thanks

melvin-bot · 2024-08-19T18:06:42Z

@AndrewGable, @eVoloshchak, @rakhaxor, @Christinadobrzyn, @moonstar-95515 Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

Christinadobrzyn · 2024-08-19T20:11:04Z

Quick follow up - based on a review (as Matt outlined) we are going to continue with this payment summary.

@eVoloshchak can you accept this Upwork offer - https://www.upwork.com/nx/wm/offer/103544327
@rakhaxor can you accept this Upwork offer - https://www.upwork.com/nx/wm/offer/103593092

TY!

rakhaxor · 2024-08-20T06:57:39Z

Hello @Christinadobrzyn! I have accepted the offer on Upwork. Thank you

eVoloshchak · 2024-08-20T16:19:38Z

@Christinadobrzyn, declined the Upwork offer, requested the payment on NewDot
Thanks!

Christinadobrzyn · 2024-08-20T16:27:15Z

Awesome! Thanks! The payment summary is here.

Feel free to submit your NewDot payment @eVoloshchak. Also, I don't see that it was asked but do we need a regression test for this?

JmillsExpensify · 2024-08-20T17:21:52Z

$250 approved for @eVoloshchak

Christinadobrzyn · 2024-08-20T22:37:57Z

@eVoloshchak can you let us know about a regression test? Then I think this is good to close!

eVoloshchak · 2024-08-21T17:17:46Z

Yes, a regression test would be useful and easy to perform

Regression Test Proposal

Navigate to staging.new.expensify.com
Open a chat and type a one-line text and create a few empty lines

test

Select all using keyboard ctrl + A and cut the text
Paste it inside the composer
Verify the cursor is positioned at the end of the pasted text

Do we agree 👍 or 👎

Christinadobrzyn · 2024-08-21T17:31:41Z

Regression test is created - https://github.com/Expensify/Expensify/issues/422290

Closing!

melvin-bot · 2024-08-21T17:31:44Z

@AndrewGable @Christinadobrzyn Be sure to fill out the Contact List!

kbecciv added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels May 15, 2024

melvin-bot bot assigned Christinadobrzyn May 15, 2024

Christinadobrzyn mentioned this issue May 16, 2024

[$250] Cursor is misplaced after pasting a multiline message on Description field in request Money flow. #42112

Closed

6 tasks

Christinadobrzyn added the External Added to denote the issue can be worked on by a contributor label May 20, 2024

melvin-bot bot changed the title ~~Web - Composer - Cursor positioned at the start if pasting text which contains a new line~~ [$250] Web - Composer - Cursor positioned at the start if pasting text which contains a new line May 20, 2024

melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label May 20, 2024

melvin-bot bot assigned eVoloshchak May 20, 2024

melvin-bot bot assigned AndrewGable May 21, 2024

AndrewGable assigned rakhaxor May 21, 2024

melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label May 21, 2024

melvin-bot bot added the Overdue label Aug 12, 2024

melvin-bot bot removed the Overdue label Aug 13, 2024

melvin-bot bot added Daily KSv2 and removed Daily KSv2 labels Aug 14, 2024

melvin-bot bot added the Overdue label Aug 19, 2024

melvin-bot bot removed the Overdue label Aug 19, 2024

Christinadobrzyn closed this as completed Aug 21, 2024

[Pay meow][$250] Web - Composer - Cursor positioned at the start if pasting text which contains a new line #42216

[Pay meow][$250] Web - Composer - Cursor positioned at the start if pasting text which contains a new line #42216

Comments

kbecciv commented May 15, 2024 • edited by melvin-bot bot Loading

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

Screenshots/Videos

melvin-bot bot commented May 15, 2024

kbecciv commented May 15, 2024

kbecciv commented May 15, 2024

Christinadobrzyn commented May 16, 2024

moonstar-95515 commented May 16, 2024

Proposal

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

What is the root cause of that problem?

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

What alternative solutions did you explore? (Optional)

melvin-bot bot commented May 16, 2024

Christinadobrzyn commented May 20, 2024

melvin-bot bot commented May 20, 2024

melvin-bot bot commented May 20, 2024

rakhaxor commented May 20, 2024 • edited Loading

Proposal

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

What is the root cause of that problem?

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

What alternative solutions did you explore? (Optional)

RickDeb2004 commented May 20, 2024 • edited Loading

Proposal

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

What is the root cause of that problem?

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

Summary of Changes

What alternative solutions did you explore? (Optional)

melvin-bot bot commented May 20, 2024

Christinadobrzyn commented May 21, 2024

eVoloshchak commented May 21, 2024

melvin-bot bot commented May 21, 2024

melvin-bot bot commented May 21, 2024

moonstar-95515 commented May 21, 2024 • edited Loading

moonstar-95515 commented May 21, 2024 • edited Loading

rakhaxor commented May 22, 2024

moonstar-95515 commented Aug 9, 2024

melvin-bot bot commented Aug 12, 2024

Christinadobrzyn commented Aug 13, 2024

moonstar-95515 commented Aug 14, 2024

Christinadobrzyn commented Aug 15, 2024 • edited Loading

moonstar-95515 commented Aug 15, 2024

mallenexpensify commented Aug 16, 2024

moonstar-95515 commented Aug 16, 2024

rakhaxor commented Aug 16, 2024

moonstar-95515 commented Aug 16, 2024

rakhaxor commented Aug 16, 2024

moonstar-95515 commented Aug 16, 2024

mallenexpensify commented Aug 19, 2024

melvin-bot bot commented Aug 19, 2024

Christinadobrzyn commented Aug 19, 2024

rakhaxor commented Aug 20, 2024

eVoloshchak commented Aug 20, 2024

Christinadobrzyn commented Aug 20, 2024

JmillsExpensify commented Aug 20, 2024

Christinadobrzyn commented Aug 20, 2024

eVoloshchak commented Aug 21, 2024

Christinadobrzyn commented Aug 21, 2024

melvin-bot bot commented Aug 21, 2024

kbecciv commented May 15, 2024 •

edited by melvin-bot bot

Loading

rakhaxor commented May 20, 2024 •

edited

Loading

RickDeb2004 commented May 20, 2024 •

edited

Loading

moonstar-95515 commented May 21, 2024 •

edited

Loading

moonstar-95515 commented May 21, 2024 •

edited

Loading

Christinadobrzyn commented Aug 15, 2024 •

edited

Loading