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

fix(markdown): handle newlines after indentation groups #6941

Merged
merged 2 commits into from
Dec 6, 2023

Conversation

wanlingt
Copy link
Contributor

@wanlingt wanlingt commented Dec 1, 2023

Problem

Given

title 1
- item 1
- item 2

title 2

Title 2 will be rendered under the same indentation as item 2 as it did not break out of the <li> group.

title 1
- item 1
- item 2

  title 2

Replacing \n with an empty space prevents markdown from detecting \n\n that signifies a break from the list item.

The previous PR which tackled this issue #6917 was reverted as older versions of Safari could not handle Lookbehind in regular expressions

Solution

Replace &nbsp; \n that comes after a list item with \n

Breaking Changes

  • No - this PR is backwards compatible

Before & After Screenshots

BEFORE:
Screenshot 2023-12-03 at 9 41 11 PM

AFTER:
image

Tests

  • In the form builder page, create a new Paragraph question
  • For the Paragraph's content, insert the following (like the screenshot)
title 1
- item 1
- item 2
- item 3


title 2
  • In the form builder, title 2 should not be indented
  • In the public form page, title 2 should not be indented

@wanlingt wanlingt force-pushed the fix/markdown-newline branch from f27789f to 97dde9c Compare December 3, 2023 13:40
@wanlingt wanlingt marked this pull request as ready for review December 4, 2023 01:47
*/
children
.replace(/\n/gi, '&nbsp; \n')
.replace(/(\n(-|\d+\.|\*)\s.*\n)(&nbsp; \n)/gi, '$1 \n')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quite a smart approach actually! Wondering why we went with the lookbehind in the first place now. 😄
nit: the i in /gi indicates case-insensitive matching, we can actually remove them since newline characters dont have a case.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch! removed here bba36a8

@wanlingt wanlingt enabled auto-merge (squash) December 6, 2023 09:12
@wanlingt wanlingt merged commit fb1569d into develop Dec 6, 2023
20 of 22 checks passed
@wanlingt wanlingt deleted the fix/markdown-newline branch December 6, 2023 09:31
@sebastianwzq sebastianwzq mentioned this pull request Dec 7, 2023
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants