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: apply widget syntax in WYSIWYG replaceSelection API and remove widget syntax in getMarkdown() API #1423

Merged
merged 6 commits into from
Mar 9, 2021

Conversation

js87zz
Copy link
Contributor

@js87zz js87zz commented Mar 8, 2021

Please check if the PR fulfills these requirements

  • It's the right issue type on the title
  • When resolving a specific issue, it's referenced in the PR's title (e.g. fix #xxx[,#xxx], where "xxx" is the issue number)
  • The commit message follows our guidelines
  • Tests for the changes have been added (for bug fixes/features)
  • Docs have been added/updated (for bug fixes/features)
  • It does not introduce a breaking change or has a description of the breaking change

Description

  • applied widget syntax to WYSIWYG replaceSelection API when matched with widget text rules
  • removed widget syntax($$widget0 ... $$) on calling getMarkdown() API

Thank you for your contribution to TOAST UI product. 🎉 😘 ✨

@js87zz js87zz requested review from shiren and seonim-ryu March 8, 2021 01:18
@seonim-ryu
Copy link
Member

이견 없습니다. 고생하셨어요~

Copy link
Member

@shiren shiren left a comment

Choose a reason for hiding this comment

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

수고하셨습니다.

@@ -29,7 +29,7 @@ export function createWidgetContent(info: string, content: string) {
export function widgetToDOM(info: string, content: string) {
const { rule, toDOM } = widgetRuleMap[info];

content = trailingWidgetSyntax(content).match(rule)![0];
content = unwrapWidgetSyntax(content).match(rule)![0];
Copy link
Member

Choose a reason for hiding this comment

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

저 위쪽은 text고 여기는 왜 content에요? 미묘하게 다른 것이 있나요??

Copy link
Contributor Author

Choose a reason for hiding this comment

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

아 동일합니다. 이쪽을 미처 변경못했네요~text로 변경해둘게요

@js87zz js87zz force-pushed the fix/remove-widget-syntax branch from 0e15994 to 0b0d310 Compare March 9, 2021 06:37
@js87zz js87zz force-pushed the fix/remove-widget-syntax branch from 3999fd5 to 8c31c6b Compare March 9, 2021 10:38
@js87zz js87zz merged commit 0c31202 into next Mar 9, 2021
@js87zz js87zz deleted the fix/remove-widget-syntax branch March 9, 2021 10:38
@juanpbarani
Copy link

Hi team, is it possible that this change is creating this bug: #2663 ?

For some reason, the widget syntax is just not being removed at all :-(

Thanks!

ahamelers pushed a commit to ahamelers/tui.editor that referenced this pull request Aug 21, 2023
… widget syntax in `getMarkdown()` API (nhn#1423)

* fix: apply widget rules in WYSIWYG API(replaceSelection)

* fix: unwrap widget text in getMarkdown() API

* refactor: change trailingWidgetSyntax to unwrapWidgetSyntax

* chore: add widget node test case(getMarkdown(), replaceSelection(), insertText() in WYSIWYG)

* chore: apply code review

* chore: apply code review - 2
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.

4 participants