changed wrapping function for HTML-bold and italic

[keshav234156]

Resolves bold/unbold issues related to improper nesting of selected text during bolding operations: publiclab/PublicLab.Editor#307

[jywarren]

This is looking good. Can you add a brief text description to the pull request (just copying in from the issue) and also open this against bevacqua/woofmark and link this to that? Thank you!!!

[jywarren]

Just something like Resolves the issue where _______ -- thank you!

[jywarren]

And sorry to ask for such strictness on descriptions and such but we'll really want very clear, readable trail of changes across the 2 repos because of the complex downstream including we'll be doing! Thanks!

[keshav234156]

I think it would be better if we can first completly and thoroughly do a review and change it accordingly. And then we can open the final commit on bevacqua/woofmark, so that author can then merge it without any much difficulty and hesitation. What do you think about this?

[jywarren]

Hmm, this makes sense! Thank you! Can you request a review from another editor team member so we can be sure folks are synced with this workflow? Thanks!!!

…

On Tue, Jun 16, 2020, 2:50 PM keshav234156 ***@***.***> wrote: I think it would be better if we can first completly and thoroughly do a review and change it accordingly. And then we can open the final commit on bevacqua/woofmark, so that author can then merge it without any much difficulty and hesitation. What do you think about this? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#4 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAF6J77KFHQODXRDNLYQRDRW7LHNANCNFSM4N5SAXXQ> .

[Shulammite-Aso]

@keshav234156 @jywarren how we even make PR to bevacqua/woofmark and jywarren/woofmark with one fork of woofmark? I realised i can't fork this current repo because i already have a copy of bevacqua/woofmark, and this one is some commits behind the main repo.

Meanwhile i noticed because i was trying to pull in your PR Keshav to try locally, then saw your fork is different from mine. Any way around this that i don't know?

[keshav234156]

@Shulammite-Aso yes, it shows the error when we try to fork second repo

[jywarren]

Aha - ok, so 2 things:

1. first, i think you need to sync your fork to bevacqua/woofmark -- forcing it if necessary -- if running git pull https://github.com/bevacqua/woofmark.git master doesn't work, you can try rewinding your master branch -- you can do that by following a process similar to that outlined here, i believe: https://publiclab.org/wiki/contributing-to-public-lab-software#Rewinding+the+master+branch
2. then, I believe you can open a PR from any fork/branch to any other... will try to document here:

first:

then we need to click compare across forks:

we select our own fork and the right branch:

And i believe it can be a fork already used in another PR! Is this what you needed? Please reach out in the chatroom if you get stuck on this though. I could be misunderstanding. Thank you!

[jywarren]

I also expanded the description a bit to show the kind of detail we are looking for! Hope this helps!

[jywarren]

Is this something we could write a test for? I know woofmark doesn't have tests but... i'm interested in really ensuring that this fix works, and each woofmark fix would be really aided by a test to demonstrate the problem and how the new code resolves it.

What do you think it would take to add Jest testing along the lines of publiclab/PublicLab.Editor#541 to this repository? cc @Shreyaa-sharmaa !

[cypherean]

From what I've observed jest tests are easy to write and integrate once you get used to dealing with the errors. We can define bold.test.js (already working on it, would commit it in a day or two) and keep adding new tests for it for every bug fix and feature.
In this case, the test could resemble the sequence of steps that caused the error and we could check if the text/regex of textarea matches the expected output. Possibly by converting the mode as well.

[Shulammite-Aso]

@keshav234156 @jywarren Been doing some deep digging into this issue, and have made some observation and have some clearification to seek too.

Goal was to see if there's another way to resolve this while still being able to select and un-select commands the normal way, without having to hit ->. Reason is, people may not think of using -> when they want to exit a command (I'm not sure I would) , unless maybe it is stated there on the editor that that is how to exit a command. Before recently, I didn't know I can exit bold command by hit space before hitting the command again, i'v been thinking i can't exit bold command at all. My point is, it's not quite intuitive.

My observation so far is that, exiting out of all the commands works quite smoothly in Firefox. The bugs are only present in Chrome, from my end. Has anybody else noticed this??

Also want to be very sure what I'm thinking the bug here is, is the same with what everybody is trying to tackle. I assumed publiclab/PublicLab.Editor#307 is about not being able to easily exit out Bold command. And being taken to the previous command when we hit Enter. I do hope I'm inline?😃

So here in Firefox, I can smoothly exit the bold or Italics module by just selecting the command again, without having to further hit space or anything. Also if I clear a text I was typing after un-selecting module, it does not take me back to the module.

you can see this in the gif, including that hitting enter in code block, does not initialize another block like in chrome, it just continues the current block:

you exit out of the block by clicking outside it.

sorry i was typing really slow. i actually do type slow, plus some of my keys have become hard to press. (ants invaded my house recently and ruined my keyboard and other things😂)

[Shulammite-Aso]

So this here shows some of our bugs are browser specific. Also given that the HTLM mode at https://bevacqua.github.io/woofmark/ works fine even though it uses the same wrapping function. This has got to do with how different browsers render WYSIWYG

spent a lot of time trying to figure out the difference in the way each of the browsers renders different patterns of text, and the only consistent difference in there markup when you convert them to HTML is that in Chrome the SPACES surrounding the texts with Bold formatting are rendered as   while Firefox renders them as normal space, and firefox also inserts <br> at the end of any paragraph you type.

[Shulammite-Aso]

Was trying several ways to detect when there is   in our HTML to and replace it with normal space, to see if this fixes anything, but i can't seem to access the content of the editor. I've tried using editor.value from here https://github.com/bevacqua/woofmark/blob/83fc4f27bf8c7e64718a76e388c6dc5e39b6430f/src/woofmark.js#L88

also tried innerHTML of .wk-wysiwyg and its detecting and replacing the   i know from loging it to console, but if change to HTML mode i still see all the  . Wondering if there is a way to get what the browser renders as WYSIWYG???
i don't think its chunks , i think chunks is what you type out, but not exactly the markup when the browser renders it.

Just mentioning this so that maybe we can look in these directions for a fix, if possible.

[Shulammite-Aso]

Also noting that we still see the extra ** tags at publiclab.org/post in Firefox when you hit enter after using bold, but not at https://bevacqua.github.io/woofmark/ , i don't know why, but we can fix it downstream by extracting the ** tags

[keshav234156]

Great Finding @Shulammite-Aso!!

[jywarren]

people may not think of using -> when they want to exit a command (I'm not sure I would) , unless maybe it is stated there on the editor that that is how to exit a command. Before recently, I didn't know I can exit bold command by hit space before hitting the command again, i'v been thinking i can't exit bold command at all. My point is, it's not quite intuitive.

This is really a good observation. I think the -> convention is used in Slack, and is an OK baseline means. But I support finding an even more intuitive way. It doesn't have to be either-or, we can do both, i hope?

The observation that it works in Firefox is also critical. What I recommend is that we write a Jest test to demonstrate the error if possible. That will help us really clearly demonstrate it. If we really really want, we could also get a firefox jes test running.

sorry i was typing really slow. i actually do type slow, plus some of my keys have become hard to press. (ants invaded my house recently and ruined my keyboard and other things😂)

🏠 🐜 🐜 🐜 🐜 🐜 🐜 🐜 OMGGGGG ants have also invaded MY house!

https://bevacqua.github.io/woofmark/ works fine

In both Chrome and Firefox? Regarding this, perhaps it's a newer version of woofmark? Is it possible? I wonder what else could possibly affect our instance but not their demo? This is a big clue! 🕵️

Regarding  , the issue could be that in WYSIWYG mode, the content is literally editable HTML -- read more at https://developer.mozilla.org/en-US/docs/Web/Guide/HTML/Editable_content - and I wonder if spaces are simply interpreted as   in an editable HTML context? But I'm not sure.

Also noting that we still see the extra ** tags at publiclab.org/post in Firefox when you hit enter after using bold, but not at https://bevacqua.github.io/woofmark/ , i don't know why, but we can fix it downstream by extracting the ** tags

here again, the first place i would look is the exact version/release running in the demo at Woofmark. See if it's literally the same code, because that's a big clue. If it's not the same version, we could upgrade?

If it is the same version, what could we be doing that might interrupt it?

Thank you for the excellent detective work, @Shulammite-Aso ! You're really good at it and I appreciate your digging so deeply into this mysterious issue. Thanks!!!! 🎉 🎉 🎉 🎉 🎉

[Shulammite-Aso]

What I recommend is that we write a Jest test to demonstrate the error if possible. That will help us really clearly demonstrate it. If we really really want, we could also get a firefox jes test running.

Yes, we certainly should write tests demonstrating the errors. Wondering if i can make a commit to @Shreyaa-sharmaa's PR here publiclab/PublicLab.Editor#543 ?
About writing firefox jest test, i'm guessing it would mean a different jest-puppeteer.config.js file for firefox, it is? or can we specify two browsers in the config file??

[jywarren]

I think you should be able to specify 2 browsers. Do you think it's possible to set up Jest tests at the `woofmark` level? Maybe following Shreya's Editor implementation? That would make for a nice feedback cycle where we can write tests along with the `woofmark` feature PRs. Thanks!

…

On Thu, Jul 2, 2020 at 11:53 AM Shulammite-Aso ***@***.***> wrote: What I recommend is that we write a Jest test to demonstrate the error if possible. That will help us really clearly demonstrate it. If we really really want, we could also get a firefox jes test running. Yes, we certainly should write tests demonstrating the errors. Wondering if i can make a commit to @Shreyaa-sharmaa <https://github.com/shreyaa-sharmaa>'s PR here publiclab/PublicLab.Editor#543 <publiclab/PublicLab.Editor#543> ? About writing firefox jest test, i'm guessing it would mean a different jest-puppeteer.config.js file for firefox, it is? or can we specify two browsers in the config file?? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#4 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAF6J73EBPMR2QC22SZX5TRZSUPBANCNFSM4N5SAXXQ> .

[Shulammite-Aso]

Do you think it's possible to set up Jest tests at the woofmark level?
Maybe following Shreya's Editor implementation? That would make for a nice
feedback cycle where we can write tests along with the woofmark feature
PRs.

Oh, okay. Will do this then.

Thanks!!

[cypherean]

This was some great observation. Sorry, I misunderstood what you meant earlier @jywarren . Tag me if you need any help @Shulammite-Aso. All the best! And great work everyone! 🚀

[jywarren]

Awesome! Linking to #7 here!

[jywarren]

hi all! Was this resolved in #15, or are we still working on a few items from this? And if so, could we try to sum up what's left? Thank you!!!

[Shulammite-Aso]

hi all! Was this resolved in #15, or are we still working on a few items from this? And if so, could we try to sum up what's left? Thank you!!!

for me i think we still see that when we use newline after un-selecting a tool, e.g bold, the same tool is used again on the newline. And also if you try to un-select bold without using space first, it doesn't get unselected. But these two aren't issues in firefox, they work fine there.

On chrome i'm seeing the below console warning. I'm guessing it may be a pointer?

Something about the different way firefox works with selection and range in documents, which may be what woofmark was built in favor of.
Currently trying to understand more about Selection and Range from this article https://javascript.info/selection-range to see if some tweaks can be used to add better support for other browsers not firefox

[jywarren]

That's awesome. Thanks for the update, and looking forward to seeing what you come up with!

…

On Tue, Jul 21, 2020, 5:44 PM Shulammite-Aso ***@***.***> wrote: hi all! Was this resolved in #15 <#15>, or are we still working on a few items from this? And if so, could we try to sum up what's left? Thank you!!! for me i think we still see that when we use newline after un-selecting a tool, e.g *bold*, the same tool is used again on the newline. And also if you try to un-select bold without using space first, it doesn't get unselected. But these two aren't issues in firefox, they work fine there. On chrome i'm seeing the below console warning. I'm guessing it may be a pointer? [image: Screenshot from 2020-07-21 21-03-44] <https://user-images.githubusercontent.com/48386390/88110026-04da3480-cb9b-11ea-9c7c-7ac18f962b3a.png> Something about the different way firefox works with selection and range in documents, which may be what woofmark was built in favor of. Currently trying to understand more about Selection and Range from this article https://javascript.info/selection-range to see if some tweaks can be used to add better support for other browsers not firefox — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#4 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAF6J6WWPEPDF7HPGXJJDDR4YD4VANCNFSM4N5SAXXQ> .

[jywarren]

Just noting -- GitPod is now running and we can try woofmark out in each PR now. So if you'd like to rebase this it may help!

[jywarren]

Hi @keshav234156 @Shulammite-Aso and others, any updates here? Thank you!!!

[jywarren]

@keshav234156 i think we're still seeing what @Shulammite-Aso has pointed out:

when we use newline after un-selecting a tool, e.g bold, the same tool is used again on the newline

I'd like to get a test for that, in a new PR, and then to try different solutions to solve it. But a PR with a test would be an excellent first step!

[Shulammite-Aso]

@keshav234156 i think we're still seeing what @Shulammite-Aso has pointed out:

when we use newline after un-selecting a tool, e.g bold, the same tool is used again on the newline

I'd like to get a test for that, in a new PR, and then to try different solutions to solve it. But a PR with a test would be an excellent first step!

didn't find a fix for this even after reading this:

Currently trying to understand more about Selection and Range from this article https://javascript.info/selection-range to see if some tweaks can be used to add better support for other browsers not firefox

and others. A test will just be a good place to start again perhaps.

[jywarren]

Thanks for the update, @Shulammite-Aso! Did you mean "didn't find a fix" tho? 👍

[Shulammite-Aso]

Thanks for the update, @Shulammite-Aso! Did you mean "didn't find a fix" tho?

Oh yeah, meant "didn't find a fix" actually. Will just correct that

[jywarren]

Just noting that this code looks good, but is quite a bit more complex than the version before. I think we should try to rebase it so it gets built in GitPod, but also think we should try to demonstrate the original bug using a Jest test now that we have those. I've done something similar for a different bolding bug in publiclab/PublicLab.Editor#659 for reference.

[gitpod-io]

[jywarren]

Rebased!

[jywarren]

Noting that this looks to work OK in GitPod, but I went through each of the steps described in #307 and still got the same incorrect output:

****

****Hello**world**strong text******Hello** strong text**Hello**, world

I think we can translate the output into a test, now, though!