[HOLD for payment 2024-06-20] [$250] Web - Markdown - Cursor overlaps with certain letters if quote markdown is applied

[lanitochka17]

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
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. Open a chat
3. Click on + icon > submit an expense > Manual > Enter an amount and click next
4. Open description input field
5. Write "> Hey"

Expected Result:

The cursor doesn't overlap with the letter "y"

Actual Result:

The cursor overlaps with the letter "Y" if quote markdown is applied

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

Bug6481602_1715776736663.bandicam_2024-05-15_15-38-17-267.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0114f52f7e90fdd65f
• Upwork Job ID: 1793680288067104768
• Last Price Increase: 2024-05-30
• Automatic offers:
• alitoshmatov | Reviewer | 102575234

Issue Owner

Current Issue Owner: @abekkala

[melvin-bot]

Triggered auto assignment to @abekkala (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]

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

[abekkala]

Able to reproduce

[melvin-bot]

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

[melvin-bot]

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

[dominictb]

Proposal

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

The cursor overlaps with the letter "Y" if quote markdown is applied

What is the root cause of that problem?

The display of the blockquote section is inline-block, which means the cursor will appear inside the right border of the blockquote span, i.e, you might see some overlap of the cursor with the last character. We can see the difference

1. With display: block span, the cursor is outside of the border of the span

2. With display: inline-block, the cursor is inside the border of the span

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

If we have a small padding-right for the blockquote span, the cursor will appear right after the last character, and still before the right border, i.e: it will be on the left-test of the padding-right area (See the image below)

Testing code in the react-native-live-markdown WebExample

const [markdownStyle, setMarkdownStyle] = React.useState({
    blockquote: {
      paddingRight: '10px'
    }
  });

So the solution is to add the paddingRight style in the markdownStyle blockquote. As long as the paddingRight > cursor.width, we shouldn't have the overlapping issue. We can add this in the useMarkdownStyle hook here

blockquote: {
  ....,
  paddingRight: 1
}

What alternative solutions did you explore? (Optional)

NA

[alitoshmatov]

Thank you for your proposal @dominictb, why exactly cursor is overlapping when display is inline-block. Would setting display as block solve the issue? I mean setting padding right seems like a workaround and we are not addressing the root cause

[dominictb]

@alitoshmatov

why exactly cursor is overlapping when display is inline-block

As mentioned in my proposal , the cursor will be right inside the border, and if there's no space between the last letter and the border, the caret will overlap with the last letter

Would setting display as block solve the issue?

Yes, but that comes with the challenge: it will break the format if you add new line, which I believe it's because we are changing from inline display to block display. Change todisplay: inline would cause the blockquote format to break if the text is too long

Screen.Recording.2024-05-27.at.08.49.23.mov

I mean setting padding right seems like a workaround and we are not addressing the root cause

I could say that my solution is qualified as a solution, not workaround. I mean, the root cause here is not the display: inline-block of the span, it is the caret position of the inline-block span, so any idea that either update the inline-block display, or update the caret positioning of the inline-block should be qualified as a solution.

1. Change the display CSS property of the blockquote -> I have proved above that it will break the format, so this is not ideal.

2. Update the caret positioning of the inline-block display: Add a small space between the last letter and the border, i.e: padding-right, allowing the caret to be fully placed behind the last letter. This is an elegant solution.

[abekkala]

@alitoshmatov can you review the above from @dominictb

[alitoshmatov]

@dominictb I am still not completely convinced that this the best solution here. Even if we agreed to apply this how we would know how much padding is right one, we don't have existing merit for this. In my opinion, this point alone indicates the solution is not ideal

[melvin-bot]

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

[melvin-bot]

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

• fix: add paddingRight for blockquote #43017

If no regressions arise, payment will be issued on 2024-06-18. 🎊

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

• @alitoshmatov requires payment automatic offer (Reviewer)
• @dominictb requires payment (Needs manual offer from BZ)

[melvin-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:

• [@alitoshmatov] The PR that introduced the bug has been identified. Link to the PR:
• [@alitoshmatov] 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:
• [@alitoshmatov] 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:
• [@alitoshmatov] Determine if we should create a regression test for this bug.
• [@alitoshmatov] 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.
• [@abekkala] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[melvin-bot]

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

• fix: add paddingRight for blockquote #43017

If no regressions arise, payment will be issued on 2024-06-20. 🎊

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

• @alitoshmatov requires payment automatic offer (Reviewer)
• @dominictb requires payment (Needs manual offer from BZ)

[melvin-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:

• [@alitoshmatov] The PR that introduced the bug has been identified. Link to the PR:
• [@alitoshmatov] 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:
• [@alitoshmatov] 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:
• [@alitoshmatov] Determine if we should create a regression test for this bug.
• [@alitoshmatov] 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.
• [@abekkala] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[alitoshmatov]

• The PR that introduced the bug has been identified. Link to the PR: Live Markdown Preview #34292
• 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: https://github.com/Expensify/App/pull/34292/files#r1645879763
• 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: No need
• Determine if we should create a regression test for this bug. No need

[alitoshmatov]

This one is ready for payment

[abekkala]

Not overdue melvin! Payment is tomorrow!

[abekkala]

PAYMENT SUMMARY FOR JUNE 20

• Fix: @dominictb [$250] Offer Link
• PR Review: @alitoshmatov [$250] Offer Link

[abekkala]

@alitoshmatov payment sent and contract ended - thank you! 🎉

[abekkala]

@dominictb if you can accept the offer in Upwork I can process payment and close this one out - thank you!

[abekkala]

@dominictb payment sent and contract ended - thank you! 🎉