fix bold texts loosing formatting after converting to markdown and back to wysiwyg #15
Conversation
|
@jywarren @sagarpreet-chadha @NitinBhasneria @shreyaa-sharmaa @keshav234156 please take a look. @sagarpreet-chadha very happy to be having you around!!🎉 |
|
#17 Should be merged before this, I think. |
src/woofmark.js
Outdated
| @@ -218,7 +218,21 @@ function woofmark (textarea, options) { | |||
| if (currentMode === 'html') { | |||
| textarea.value = parse('parseHTML', textarea.value).trim(); | |||
| } else { | |||
| textarea.value = parse('parseHTML', editable).trim(); | |||
| var regexp = /\*\*[A-Z][^*]+ \*\*/gi; | |||
There was a problem hiding this comment.
Can we have a better name for this regex? Instead of using regex and regex2 as variable name, can we have a separate utils file where each regex is defined (can be done in a separate PR)
There was a problem hiding this comment.
was intending to comment the code, and will think of something more suitable to rename the variable with like your're suggesting. About a utils file, do you mean putting this code in a function and importing from a utils file?
There was a problem hiding this comment.
Yes only this regex line but it can be done later. No worries 👍
src/woofmark.js
Outdated
| var reg = textarea.value.match(regexp); | ||
|
|
||
| for (let i = 0; i <= reg.length - 1; i++) { | ||
| var regexp2 = /\*\*[A-Z][^*]+ \*\*/i; |
There was a problem hiding this comment.
we are re declarating this regex on each for iteration.
There was a problem hiding this comment.
regexp2 should go outside the iteration sorry. will fix this.
Okay! @sagarpreet-chadha, only the codebase uses |
|
Yes i believe we can use const 👍 Thanks :) |
|
Hey @Shulammite-Aso can you explain what the issue is? |
so @shreyaa-sharmaa sometimes when you make some texts bold in Rich mode and change to markdown Noticed this usually happens when you have space between the last character and the closing bold tag, like this; **text ** instead of text. One can write texts like this in rich mode without knowing it, and woofmark has no way to remove or ignore the space, so that it can be properly read as a bold formatted text when parsing it to HTML. |
|
just recalling that this may also be happening with italic. Checked and confirmed it is. Will try to also fix this for italic texts |
|
@sagarpreet-chadha @jywarren please do check this out. For the test, i made a different test.html file instead of using the existing index.html. This is so that we can simulate the typing in a clean editor area. The two are exactly the same, only difference is that there are no texts in the textarea. |
|
if we agree that everything is okay with this, i can now open the PR at Thanks!!! |
|
We can use the same index.html also to avoid redundancy, using id of textarea and using JS, we can remove text in the starting of it block of that test. Would you like to try that? |
|
oh i like that idea, Sagarpreet. Thanks to both of you for thinking through
how this should best work!
…On Mon, Jul 20, 2020 at 11:20 AM Sagarpreet Chadha ***@***.***> wrote:
We can use the same index.html also to avoid redundancy, using id of
textarea and using JS, we can remove text in the starting of it block of
that test. Would you like to try that?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#15 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAF6J6A33T3TGSVXRCICVLR4ROEPANCNFSM4O4IX2NA>
.
|
have just removed the test html file @sagarpreet-chadha @jywarren and it works fine. |
|
Nicely done - if you'd like to add some more assertions as mentioned above, let's do it in a follow-up! Thanks! |
Will be adding test for this as well.