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

[$250] When editing comment, copy and paste text that has trailing \n, console error appears and send button changed to grey, can not send #43239

Closed
2 of 6 tasks
m-natarajan opened this issue Jun 7, 2024 · 19 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. External Added to denote the issue can be worked on by a contributor retest-weekly Apply this label if you want this issue tested on a Weekly basis by Applause Weekly KSv2

Comments

@m-natarajan
Copy link

m-natarajan commented Jun 7, 2024

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.80-9
Reproducible in staging?: Yes
Reproducible in production?: Yes
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: @badeggg
Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1717559130850559

Action Performed:

  1. Open any chat page
  2. in the composer box, copy text that has trailing a trailing line break, \n, paste it to the composer box, ie.
test


Expected Result:

No error logging in console and can send text

Actual Result:

Console error appears and send button changed to grey, can not send

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
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

cc.mp4

Snip - (9) New Expensify - Google Chrome

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01296f154c567ad13d
  • Upwork Job ID: 1802744039321176346
  • Last Price Increase: 2024-06-17
Issue OwnerCurrent Issue Owner: @mallenexpensify
@m-natarajan m-natarajan added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Jun 7, 2024
Copy link

melvin-bot bot commented Jun 7, 2024

Current assignee @mallenexpensify is eligible for the Bug assigner, not assigning anyone new.

@situchan
Copy link
Contributor

situchan commented Jun 7, 2024

Here's clear repro step:

  1. Copy this code block
test


  1. Paste into composer

@melvin-bot melvin-bot bot added the Overdue label Jun 10, 2024
Copy link

melvin-bot bot commented Jun 10, 2024

@mallenexpensify Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

@mallenexpensify
Copy link
Contributor

The issue appears to have to do with the trailing space or linebreak at the end, when it's removed, you can post. Does that sound right @badeggg and @situchan

w_e5779e27696d0562df5cc72192db49420213f1cc.mp4

@melvin-bot melvin-bot bot removed the Overdue label Jun 11, 2024
@mallenexpensify mallenexpensify added the Needs Reproduction Reproducible steps needed label Jun 11, 2024
@MelvinBot
Copy link

This has been labelled "Needs Reproduction". Follow the steps here: https://stackoverflowteams.com/c/expensify/questions/16989

@badeggg
Copy link
Contributor

badeggg commented Jun 11, 2024

The issue appears to have to do with the trailing space or linebreak at the end,

Not exactly, trailing line break can repro, trailing space can not.

when it's removed, you can post

Actually, you just need change the comment just a little bit after pasting (e.g. add letter 'a' in what ever place), you can post.

By the way, is this issue waiting proposals from external?

@badeggg
Copy link
Contributor

badeggg commented Jun 11, 2024

If you are using vscode, select two lines(one line is ok too) and cmd + c to copy, like this:
image
If you use vim, shift + v enter visual mode, shift + " + * + y to copy, like this:
image

Paste to composer box, error appears

@situchan
Copy link
Contributor

The issue appears to have to do with the trailing space or linebreak at the end, when it's removed, you can post. Does that sound right @badeggg and @situchan

w_e5779e27696d0562df5cc72192db49420213f1cc.mp4

The video you shared correctly explains the bug. Send button is disabled along with console error.

@melvin-bot melvin-bot bot added the Overdue label Jun 13, 2024
Copy link

melvin-bot bot commented Jun 14, 2024

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

@mallenexpensify mallenexpensify added External Added to denote the issue can be worked on by a contributor and removed Needs Reproduction Reproducible steps needed labels Jun 17, 2024
@melvin-bot melvin-bot bot changed the title When editing comment, copy and paste text that has trailing \n, console error appears and send button changed to grey, can not send [$250] When editing comment, copy and paste text that has trailing \n, console error appears and send button changed to grey, can not send Jun 17, 2024
Copy link

melvin-bot bot commented Jun 17, 2024

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Jun 17, 2024
Copy link

melvin-bot bot commented Jun 17, 2024

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

@melvin-bot melvin-bot bot removed the Overdue label Jun 17, 2024
@mallenexpensify
Copy link
Contributor

Opening this up for proposals, @badeggg , did you want to submit one?
I added trailing line break to the repro steps to be clear, since I wasn't sure what /n exactly meant.

@badeggg
Copy link
Contributor

badeggg commented Jun 18, 2024

Proposal

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

When copy some text with trailing line break(\n) and past to composer box, console error appears and send button changed to grey, can not send

What is the root cause of that problem?

"react-native-live-markdown" code here, condition if (nextChar !== '\n') is not checking textNodes[i + 1] might be undefined and range.setStart(textNodes[i + 1] as Node, 0); is using textNodes[i + 1] assuming it is a node entity.

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

Change this line from:

if (nextChar !== '\n') {

to

if (nextChar !== '\n' && textNodes[i + 1]) {

What alternative solutions did you explore? (Optional)

N/A

@BartoszGrajdek
Copy link
Contributor

BartoszGrajdek commented Jun 18, 2024

Hi, I’m Bartosz from SWM react-native-live-markdown team. We’ve recently decided that it would be best to actively participate in all of the issues related to the library we’re working on. That means reviewing proposals together with the Expensify team and supporting external contributors to ensure quality solutions that won’t introduce any additional regressions. 🙌🏻

I’m currently going through all of the issues that could require any changes to the library. Feel free to tag me if you need anything, later I’ll be diving into the issues more in-depth, but there’s quite a few of them so it can take a while for me to catch up. Oh and feel free to tag me in any live-markdown related issues in the future! 😄

@mallenexpensify mallenexpensify removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Jun 20, 2024
@melvin-bot melvin-bot bot added the Overdue label Jun 20, 2024
@mallenexpensify
Copy link
Contributor

Thanks @BartoszGrajdek , I added to the Live Markdown project, removed Help Wanted and bumped to weekly.
@badeggg , hang tight for now plz while we get a better handle on managing markdown improvements more holistically.

@mallenexpensify mallenexpensify added Weekly KSv2 and removed Daily KSv2 labels Jun 20, 2024
@melvin-bot melvin-bot bot removed the Overdue label Jun 20, 2024
@BartoszGrajdek
Copy link
Contributor

Thank you @mallenexpensify 🙌🏻

I've added a more in-depth update here. I'll be looking through proposals in the coming days. What @badeggg is suggesting to change is something that has already caused quite a few problems, so I'd rather test it carefully.

Copy link

melvin-bot bot commented Jun 21, 2024

@eVoloshchak @mallenexpensify 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!

@thienlnam thienlnam moved this to MEDIUM in Live Markdown Jun 24, 2024
@BartoszGrajdek
Copy link
Contributor

BartoszGrajdek commented Jun 24, 2024

I believe this was fixed here! Can we get it retested?

These changes are also coming into react-native-live-markdown later this week.

cc @mallenexpensify

@mallenexpensify mallenexpensify added the retest-weekly Apply this label if you want this issue tested on a Weekly basis by Applause label Jun 25, 2024
@mallenexpensify
Copy link
Contributor

Unable to reproduce on web, staging, Chrome. Closing. Thanks @BartoszGrajdek !

@BartoszGrajdek BartoszGrajdek moved this from MEDIUM to Done in Live Markdown Jun 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. External Added to denote the issue can be worked on by a contributor retest-weekly Apply this label if you want this issue tested on a Weekly basis by Applause Weekly KSv2
Projects
Status: Done
Development

No branches or pull requests

7 participants