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

Add/copy rendered markdown #579

Merged
merged 3 commits into from
Jul 6, 2017
Merged

Add/copy rendered markdown #579

merged 3 commits into from
Jul 6, 2017

Conversation

dmsnell
Copy link
Member

@dmsnell dmsnell commented Jun 30, 2017

Markdown: Add ability to copy rendered note HTML

When viewing a note's Markdown preview and with no selection, if you
press the copy shortcut then the rendered HTML for the note will fill
the clipboard buffer. If there is some selected text then the copy will
behave as normal.

Testing

Open a note preview and hit +C or Ctrl+C. When pasting you should see HTML generated from the note. If you select some of the rendered content before copying you should find that when you paste it will be some kind of text view of the contents.

@Copons
Copy link
Contributor

Copons commented Jul 3, 2017

Testing works as expected, but I'm having a weird behaviour on selection.

The attached gif should be clear, but this is what seems to happen.
In both Text and Markdown Edit modes, once I select a piece of text, the selection "origin" gets stuck. So, if I try to select another piece of text, the selection always starts from the same position of the first selection starting point.
Clicking outside the selection does not reset it. The only way to remove this behaviour is to move the cursor around with the keyboard.

jul-03-2017 11-02-49

Though, this happens on master as well, so it's not caused by any changes made here.
Might be worth opening an issue, I guess.

@dmsnell
Copy link
Member Author

dmsnell commented Jul 3, 2017

Thanks @Copons. I saw that behavior too and I think it could relate to draft-js. Note also that I have a PR to pull it out #568.

Did you also experience this in the preview mode? I'm not sure what "Markdown edit" mode means here.

@Copons
Copy link
Contributor

Copons commented Jul 3, 2017

Thanks @Copons. I saw that behavior too and I think it could relate to draft-js. Note also that I have a PR to pull it out #568.

Yay! I totally missed that review request, gonna give it a look asap!

Did you also experience this in the preview mode? I'm not sure what "Markdown edit" mode means here.

Means that it only happen in Edit mode (which only exists for Markdown, therefore "Markdown Edit" 🙂), but not in Preview.
So yeah, definitely all hints point to Draft.

@Copons
Copy link
Contributor

Copons commented Jul 3, 2017

As noted here, I tried with a fresh node_modules to see if something breaks up after removing marked, since it's left in https://github.com/Automattic/simplenote-electron/blob/master/webpack.config.dll.js#L15.

I got no issues, apparently because marked is imported by some other module (it's there in node_modules anyway).
I guess it should be removed from the dll though.

dmsnell added 3 commits July 4, 2017 16:21
`marked` doesn't appear to have been maintained for a while while
`showdown` has more recent contributions. Further, `marked` was missing
some functionality that we wanted. Namely, we wanted an equivalence with
Github-flavored Markdown (gfm) and `showdown` does it better.

In this patch we've changed the way we are inserting the content.
Instead of using `dangerouslySetInnerHTML` we're statefully rendering
the content into the DOM and then manipulating it. This change provides
an easier way to interact with the rendered content because we can
operate at the DOM level instead of at the string level. For example,
this is how highlighting now gets inserted.

**Notable changes**
 - Todo items render (though they can only be changed in the plaintext
   view)
 - Tables render
 - Code blocks highlight based on their language
 - Certain dirty/malicious content gets sanitized before display

**Questions**
 - Since the todo items render but aren't editable it could be confusing
   for someone to use, misleading them into thinking it's supposed to
   work
When viewing a note's Markdown preview and with no selection, if you
press the copy shortcut then the rendered HTML for the note will fill
the clipboard buffer. If there is some selected text then the copy will
behave as normal.
@dmsnell dmsnell force-pushed the add/copy-rendered-markdown branch from 5e4f58a to e117434 Compare July 4, 2017 14:22
@dmsnell
Copy link
Member Author

dmsnell commented Jul 4, 2017

Updated @Copons so that now the vendor bundle builds with showdown instead of marked

It appears like jest is actually pulling in marked but only for development. Shouldn't make it into the final bundle.

npm ls marked

@taimaiduc
Copy link

Is work on window version?

2017_07_05_19_01_39_

@dmsnell dmsnell merged commit 70f41db into master Jul 6, 2017
@dmsnell dmsnell deleted the add/copy-rendered-markdown branch July 6, 2017 13:55
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.

3 participants