App: [HOLD for payment 2023-05-25] [$1000] Line break before heading is missing in editing mode
If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!
Action Performed:
- Go to any chat
- Send a new comment
test
# heading
- Edit the comment and notice the text in editing composer is
test
# heading
Expected Result:
The line break before heading shouldn’t be missing
Actual Result:
The line break before heading is missing
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 / Chrome
- iOS / native
- iOS / Safari
- MacOS / Chrome / Safari
- MacOS / Desktop
Version Number: 1.3.5-5 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 Notes/Photos/Videos: Any additional supporting documentation
Expensify/Expensify Issue URL: Issue reported by: @eh2077 Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1682388776064199
Upwork Automation - Do Not Edit
- Upwork Job URL: https://www.upwork.com/jobs/~01ea44f382867687f1
- Upwork Job ID: 1651431771500584960
- Last Price Increase: 2023-04-27
About this issue
- Original URL
- State: closed
- Created a year ago
- Comments: 39 (25 by maintainers)
The PR that introduced the bug has been identified. Link to the PR: https://github.com/Expensify/expensify-common/pull/468
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/expensify-common/pull/468/files#r1212251731
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: This is a typical markdown issue, we already have an item in the reviewer checklist to catch similar issues sooner
If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.Determine if we should create a regression test for this bug. Thinking about it again, we don’t need a regression test I think. We already have unit tests for this, no need to have a redundant manual test for something that’s already automated
Will finish the BZ checklist tomorrow
I think the regression test is needed here, this is something that could easily break, will propose a solutionThanks for helping with this while I as away @Christinadobrzyn!!
Paid out the jobs in Upwork
@eh2077 - reporting bonus + speed bonus + contributor @eVoloshchak speed bonus + C+
@eVoloshchak - can you let me know about a regression test? Thanks!
Lets add the bonus as i was at AppJS
Reassigning to get this paid tomorrow as I’m OOO.
@eVoloshchak @eh2077 merged the PR, been at conference so took a while
Hi @eVoloshchak , as @kadiealexander has confirmed we should fix the removing ending line breaks issue too, I think we can apply the same regex piece to fix both removing line breaks issue together. And I found another reason that we should them is that we apply newline rule before heading rule, so it’s obviously buggy to removing starting and ending line breaks of heading.