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

feat: Adding Custom Markdown Renderer option to Editor (close #485) #1188

Closed
wants to merge 2 commits into from
Closed

feat: Adding Custom Markdown Renderer option to Editor (close #485) #1188

wants to merge 2 commits into from

Conversation

dvargas92495
Copy link
Contributor

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

I was interested in resolving the issue mentioned in #485. In case there are any future markdown inconsistencies from going WYSIWYG first, I thought to add a custom markdown renderer based on the feedback here: #485 (comment). This should resolve all issues of this type in the future and not be blocked on the underlying squire refactor. Let me know what you think!


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

}
});

expect(editor.getMarkdown()).toBe('a\n\na');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@parthshah31 LMK if this covers your use case!

@dvargas92495
Copy link
Contributor Author

Hey @js87zz, any thoughts on this option?

@js87zz
Copy link
Contributor

js87zz commented Sep 10, 2020

@dvargas92495
Sorry for late replying.
I checked the content of your PR.
It's a great option for customizing the markdown renderer and thank for your writing the detail guide and contributing! 👍
However, Although this option is a very convenient option, we decided that the option will be not applied. Because we have a plan to change the our structure totally in our next major version and to-mark libs will be removed. Then perhaps this option will also be deprecated in next major version. This is why we decided not to apply this good option.
If you have any other differences, write your opinion.
Thank you very much for your detailed contribution! 😁

@dvargas92495
Copy link
Contributor Author

Thanks for the feedback!

I think it depends on the timeline of the major version bump. If the major version release is happening in the next couple of months, then we could try to hack around on our end until then. If it will be a couple of years, we would want to have the option available since it's affecting our end users.

@js87zz
Copy link
Contributor

js87zz commented Sep 11, 2020

@dvargas92495
It won't take years. Our goal is to release in the first half of next year.

@dvargas92495
Copy link
Contributor Author

Sounds good! Feel free to close the PR and issue then!

@js87zz js87zz closed this Sep 16, 2020
@dvargas92495 dvargas92495 deleted the Parse_Breaks_Markdown branch September 16, 2020 01:24
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