Skip to content

Comments

changed the wrapping function for HTML-bold and italic#1

Closed
keshav234156 wants to merge 4 commits intojywarren:masterfrom
keshav234156:wrapping
Closed

changed the wrapping function for HTML-bold and italic#1
keshav234156 wants to merge 4 commits intojywarren:masterfrom
keshav234156:wrapping

Conversation

@keshav234156
Copy link

@keshav234156
Copy link
Author

Hi have made the new wrapping function for bold, so that other modules are undisturbed It should most probably fix the publiclab/PublicLab.Editor#307 and may also cause some other bugs due to changes. So need to test it with different test cases as much as possible.

Another thing is that to exit out of module we need to press hotkey ie Ctrl + B for bold and
-> after that. Though -> may look inconsistent but that because we are using Zero width space, which I believe has its own pros and cons and that was the only solution that I could think of to exit out of code module

@keshav234156
Copy link
Author

@keshav234156
Copy link
Author

keshav234156 commented May 30, 2020

Copy link

@VladimirMikulic VladimirMikulic left a comment

Choose a reason for hiding this comment

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

Good, but needs to be documented for future devs.

@keshav234156
Copy link
Author

@VladimirMikulic Will surely do that, just trying now to get it tested with as much as possible with manual test cases

@Shulammite-Aso
Copy link

What is -> please?🙈 Enter??

Also, can you add a GIF of how the changes now works?

Thanks😆

@keshav234156
Copy link
Author

What is -> please? Enter??

Also, can you add a GIF of how the changes now works?

Thanks

-> is Right arrow. Will make GIF soon

@keshav234156
Copy link
Author

@Shulammite-Aso @NitinBhasneria @Shreyaa-s Please review and try to test it with as much as possible with manual test cases

@NitinBhasneria
Copy link

@keshav234156 it is a little difficult thing to understand all your changes. As @Shulammite-Aso said can you make the gif of the changes so that we can particularly check on the changes and its effect.
Thank you

@jywarren
Copy link
Owner

jywarren commented Jun 3, 2020

Oh interesting! Are you able to publish this on your gh-pages branch so we can try it out live? And, can you clarify about needing ctrl-B AND ->? why not just ->?

Thank you!

function boldOrItalic (chunks, type) {
wrapping(type === 'bold' ? 'strong' : 'em', strings.placeholders[type], chunks);
function boldOrItalic(chunks, type) {
wrappingBoldItalic(type === 'bold' ? 'strong' : 'em', strings.placeholders[type], chunks);
Copy link
Owner

Choose a reason for hiding this comment

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

Can you link us to the general wrapping() function for comparison? Thanks!

Copy link
Author

Choose a reason for hiding this comment

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

@keshav234156
Copy link
Author

@jywarren I tried deploying gh-page at https://keshav234156.github.io/woofmark-testing/examples/index.html But there is an error.Can you please help me to figure it out?

@keshav234156
Copy link
Author

@jywarren wrapping function is only called when we press the respective hotkey, for bold its Ctrl + B and we need to press -> as we have added zero width space ie &#8203. This was the only way I found to exit out of the module.

@keshav234156
Copy link
Author

Untitled_ Jun 8, 2020 4_43 PM

@keshav234156
Copy link
Author

@Shreyaa-s @NitinBhasneria @Shulammite-Aso any changes except documenting it.

@cypherean
Copy link

Since you're familiar with this part of the code, please have a look at this discussion and see if you can answer some of these questions publiclab/PublicLab.Editor#457. Any input would be appreciated! Thanks!

@jywarren
Copy link
Owner

This is looking great. In keeping with our idea of maintaining our own branch, would you be able to open this against jywarren/woofmark's plots2 branch, so that we have the master branch here still tracking the upstream bevacqua/woofmark? Does that make sense? I think it does but please correct me if you have a better idea, thank you!!!

@keshav234156
Copy link
Author

This is looking great. In keeping with our idea of maintaining our own branch, would you be able to open this against jywarren/woofmark's plots2 branch, so that we have the master branch here still tracking the upstream bevacqua/woofmark? Does that make sense? I think it does but please correct me if you have a better idea, thank you!!!

ok,made a new PR #4

@keshav234156
Copy link
Author

Since you're familiar with this part of the code, please have a look at this discussion and see if you can answer some of these questions publiclab/PublicLab.Editor#457. Any input would be appreciated! Thanks!

will take a look!!

@jywarren
Copy link
Owner

Deferring to copy of this in #4, moving over there!

@jywarren jywarren closed this Nov 17, 2020
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.

7 participants