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

[hold for payment 2024-07-30] [$250] Chat - Incorrect format is copied to clipboard after copying an edited header with bold markdown #43931

Closed
1 of 6 tasks
lanitochka17 opened this issue Jun 18, 2024 · 36 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review

Comments

@lanitochka17
Copy link

lanitochka17 commented Jun 18, 2024

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.85-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: N/A
Issue reported by: Applause - Internal Team

Action Performed:

  1. Navigate to staging.new.expensify.com
  2. Inside the compose box paste a header with bold markdown and post it: (e.g. # header)
  3. Edit the posted comment and append some letters after a space. (e.g # header aa)
  4. Triple click on the edited comment on step 2
  5. Paste it inside the compose box

Expected Result:

The correct format gets pasted

Actual Result:

The header markdown is pasted on the first line and the rest of the text is pasted on a new line

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

Add any screenshot/video evidence

Bug6517371_1718723531842.bandicam_2024-06-18_16-23-17-279.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01c865bc9dffbb5438
  • Upwork Job ID: 1806067556929314722
  • Last Price Increase: 2024-07-03
Issue OwnerCurrent Issue Owner: @hayata-suenaga
@lanitochka17 lanitochka17 added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Jun 18, 2024
Copy link

melvin-bot bot commented Jun 18, 2024

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

@lanitochka17
Copy link
Author

@garrettmknight 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

@lanitochka17
Copy link
Author

We think that this bug might be related to #vip-vsp

@garrettmknight
Copy link
Contributor

Adding to live markdown per this

@garrettmknight garrettmknight moved this to Todo in Live Markdown Jun 19, 2024
@thienlnam thienlnam removed the status in Live Markdown Jun 19, 2024
@thienlnam thienlnam moved this to MEDIUM in Live Markdown Jun 19, 2024
@melvin-bot melvin-bot bot added the Overdue label Jun 21, 2024
Copy link

melvin-bot bot commented Jun 24, 2024

@garrettmknight Huh... This is 4 days overdue. Who can take care of this?

@garrettmknight
Copy link
Contributor

Waiting for SWM to work on this one.

@melvin-bot melvin-bot bot removed the Overdue label Jun 24, 2024
@BartoszGrajdek
Copy link
Contributor

Hi, I’m Bartosz from SWM react-native-live-markdown team. 😄

I believe this issue is something that could be handled by external contributors, especially considering that it's probably going to require changes just in E/App. WDYT @garrettmknight ?

@garrettmknight garrettmknight added the External Added to denote the issue can be worked on by a contributor label Jun 26, 2024
Copy link

melvin-bot bot commented Jun 26, 2024

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

@melvin-bot melvin-bot bot changed the title Chat - Incorrect format is copied to clipboard after copying an edited header with bold markdown [$250] Chat - Incorrect format is copied to clipboard after copying an edited header with bold markdown Jun 26, 2024
@melvin-bot melvin-bot bot added Overdue Help Wanted Apply this label when an issue is open to proposals by contributors labels Jun 26, 2024
Copy link

melvin-bot bot commented Jun 26, 2024

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

@Amoralchik
Copy link
Contributor

Amoralchik commented Jun 27, 2024

Hi there, I can't reproduce it on my latest version. (050641a: Update version to 9.0.2-3)

Still Reproduced

I tried to fix this issue and found that the problem lies in how we are converting the text entered by the user into the input. When the user copies our text, it includes HTML markup unlike what you can copy here. As a result, when this markup gets into htmlString.split (here) it adds \n to the end of the line.

I don't know where we should try to fix this, on the input or rendering sides.

That is not last update, I am still working on it

# *Header* s

@Amoralchik
Copy link
Contributor

Amoralchik commented Jun 28, 2024

Proposal

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

We are adding \n after # in the input when header includes additional tags like _Text_ or *Text* is present in the text.

What is the root cause of that problem?

We add \n at the end of the line in replaceBlockElementWithNewLine: here

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

I got stable and successful results changed the first "if" statement conditions to:

if (text.match(/[\n|>][>]?[\s]?$/) || index === splitText.length - 1 || text === "# ") {
  joinedText += text;
} 

This works because if a \n follows the #, we will add it regardless. Therefore, I added a check to prevent adding \n after the #.

2024-06-28.13-15-20.mp4

Testing string:

# _Text_ # *Real Header*
# 
*Text*
*Header* s *another* _emphasis_ **AAAA** __AAAA__
_emphasis_ **AAAA** __AAAA__ [example link](https://example.com/)

@melvin-bot melvin-bot bot added the Overdue label Jul 1, 2024
@garrettmknight
Copy link
Contributor

@rushatgabhane can you give this one a look when you get a chance?

@melvin-bot melvin-bot bot removed the Overdue label Jul 1, 2024
@rushatgabhane
Copy link
Member

@Amoralchik's proposal looks good #43931 (comment)

we have unit tests on the library so it should be good 👍

🎀👀🎀

@melvin-bot melvin-bot bot added the Weekly KSv2 label Jul 11, 2024
@Amoralchik
Copy link
Contributor

I Created Pull Request to update expensify-common to version 2.0.42. This version includes the following fix: Expensify/expensify-common#744

@Amoralchik
Copy link
Contributor

Hi there, The changes made here are now available on staging and ready for testing, by this bump expensify-common

@garrettmknight garrettmknight removed their assignment Jul 25, 2024
@garrettmknight garrettmknight added Bug Something is broken. Auto assigns a BugZero manager. and removed Bug Something is broken. Auto assigns a BugZero manager. labels Jul 25, 2024
Copy link

melvin-bot bot commented Jul 25, 2024

Triggered auto assignment to @joekaufmanexpensify (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 melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Jul 25, 2024
@garrettmknight
Copy link
Contributor

@joekaufmanexpensify just need you to watch this one while I'm out - I'll pick back up when I'm back if it's still going. Thanks!

@garrettmknight garrettmknight self-assigned this Jul 25, 2024
@joekaufmanexpensify
Copy link
Contributor

Sounds good! Moving back to weekly as I see PR is in review.

@joekaufmanexpensify joekaufmanexpensify added Weekly KSv2 and removed Daily KSv2 labels Jul 25, 2024
@rushatgabhane
Copy link
Member

Hi, the changes are in production #45556 (comment)

@Amoralchik
Copy link
Contributor

Amoralchik commented Jul 26, 2024

Hi, @joekaufmanexpensify The PR in the App has been closed as the expensify-common update has been completed here: #45556 (comment). We can either proceed to testing or close it with full payment per the example here: #44835 (comment).

As mentioned in #44835 (comment) :

#45556 updated the version without following a proper process for issues tracking, and this wasn't the only issue affected. See, for example: #37357 (comment) and #44032 (comment.

Original PR to Expensify-common: Expensify/expensify-common#744

@joekaufmanexpensify
Copy link
Contributor

Got it. Just to make sure my understanding is correct:

Is that right? If so, payment would be due 7 days after the fix went to production, which would be 2024-07-30.

@joekaufmanexpensify
Copy link
Contributor

Awesome. TY! So payment is due tomorrow in that case. We need to pay:

@joekaufmanexpensify joekaufmanexpensify changed the title [$250] Chat - Incorrect format is copied to clipboard after copying an edited header with bold markdown [hold for payment 2024-07-30] [$250] Chat - Incorrect format is copied to clipboard after copying an edited header with bold markdown Jul 29, 2024
@joekaufmanexpensify
Copy link
Contributor

@Amoralchik offer sent for $250!

@Amoralchik
Copy link
Contributor

@joekaufmanexpensify accepted

@JmillsExpensify
Copy link

$250 approved for @rushatgabhane

@joekaufmanexpensify joekaufmanexpensify added Daily KSv2 and removed Weekly KSv2 labels Jul 30, 2024
@joekaufmanexpensify
Copy link
Contributor

@Amoralchik $250 sent and contract ended!

@joekaufmanexpensify
Copy link
Contributor

Upwork job closed

@joekaufmanexpensify
Copy link
Contributor

All set, thanks everyone!

@BartoszGrajdek BartoszGrajdek moved this from MEDIUM to Done in Live Markdown Jul 31, 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. Daily KSv2 External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review
Projects
Status: Done
Development

No branches or pull requests

8 participants