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

Feature: Markdown support as a Markdown-specific Gutenberg Block #9357

Closed
wants to merge 58 commits into
base: master
from

Conversation

Projects
None yet
5 participants
@nuzzio

nuzzio commented Apr 19, 2018

Description

This PR will need code from a corresponding PR WordPress/gutenberg#6491.

The reasoning behind this feature addition is to add the ability to save and edit Markdown in posts when using Gutenberg. This would restore the spirit of the classic editor Markdown implementation.

Additional reasons are:

By @MichaelArestad

There are times (like in p2s) where it’s much more complex to go back and edit a post than to initially create it. This is because p2 removes the markdown formatting replacing it with html. I think there is value in preserving the original content. It would also allow one to compose elsewhere and just paste in valid markdown.

By @mtias

People who prefer to write markdown want to retain the text as markdown.

Addresses #9201

Implementation Approach:

First Iteration
Implement a Markdown block as a simple field with no preview functionality.

markdown-block-v1

Changes proposed in this Pull Request:

First Iteration

  • Implement a Markdown block as a simple field with no preview functionality.
  • Render Markdown on the server, reusing existing functionality.
  • Deliver Markdown content to the Gutenberg Block
  • Clean up coding artifacts and documentation.

Issues

  • Footnotes do not render properly.
  • Validate reliability of determining when a post is coming from Gutenberg.
  • Paragraphs not generated when Gutenberg post is previewed.

Testing instructions:

Run the following:

vendor/bin/phpunit --filter WP_Test_Markdown tests/php/modules/markdown/test_class.markdown.php

Proposed changelog entry for your changes:

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code has proper inline documentation.

@nuzzio nuzzio requested a review from Automattic/jetpack as a code owner Apr 19, 2018

@nuzzio nuzzio self-assigned this Apr 19, 2018

nuzzio added some commits Apr 19, 2018

Added patterns to protect non-Markdown blocks
Added patterns that will attempt to preserve all  Gutenberg non-Markdown blocks from the Markdown parser.
Remove the PlainText block wrapper
Remove the textarea wrapper to avoid rendering it as a textarea form element for the reader.
Reuse markdown content on Meta Box update
The idea here is to reuse the existing original Markdown, on the second post for Meta Boxes following a `Save Draft` event. This way when the markdown parser runs a second time it does not parse the rendered markup in `post_content` and overwrite the original Markdown in `post_content_filtered`.
Merge branch 'add/gutenberg-markdown-block' of https://github.com/nuz…
…zio/jetpack into add/gutenberg-markdown-block

# Conflicts:
#	modules/markdown/assets/js/jetpack-markdown-block.jsx
@nuzzio

This comment has been minimized.

nuzzio commented Apr 29, 2018

Added corresponding PR WordPress/gutenberg#6491.

@brentlogan

This comment has been minimized.

brentlogan commented Apr 30, 2018

I note the issue: "Footnotes do not render properly." What is the intended location for the footnotes?

  • End of the block,
  • End of the post, or
  • Some arbitrary location defined by an optional "Markdown footnote block"?

End of the block is probably the easiest to implement. End of the post might be what most people expect. An arbitrary location would provide more flexibility, and as long as it was optional with the default being one of the prior two choices, wouldn't be any harder to use.

I'm excited to see this being worked on.

@nuzzio

This comment has been minimized.

nuzzio commented Apr 30, 2018

@brentlogan

What is the intended location for the footnotes?

My initial thinking was that since the Markdown processing is happening on the whole post every time we could try to take the best aspects of both experiences.

  1. On editing we can make our footnotes at the end of the block. Which may be nice for editors keeping their references close to content.
  2. On presentation to the reader we can show the parsed version with the footnotes at the end of the post.
@brentlogan

This comment has been minimized.

brentlogan commented Apr 30, 2018

@nuzzio

That sounds perfect to me!

@ehg

I get the general impression that using regex parsing isn't going to serve us well.

I'm not sure how we could get rid of these, maybe another parser, leveraging Gutenberg's parser?

I think an option is thinking of Markdown for Gutenberg support more separated from the classic editor. This could open up more appropriate places to store the markdown/rendered HTML — which may obviate the need for regex/post_content_filtered. Do you have any ideas here? Maybe some more Gutenberg specific storage structures?

/**
* External dependencies
*/
//const React = react;

This comment has been minimized.

@ehg

ehg May 17, 2018

Member

Why is this here?

This comment has been minimized.

@nuzzio

nuzzio May 19, 2018

Oh, I think that was a quick way of making my IDE warning, for React needing to be in scope for JSX, go away. I had commented it out in favor of the /* global React:true */ ESLint parser option. I will clean that up.

@@ -440,6 +449,25 @@ public function wp_insert_post_data( $post_data, $postarr ) {
// revisions are already in the right place, except when we're restoring, but that's taken care of elsewhere
// also prevent quick edit feature from overriding already-saved markdown (issue https://github.com/Automattic/jetpack/issues/636)
if ( 'revision' !== $post_data['post_type'] && ! isset( $_POST['_inline_edit'] ) ) {
/**

This comment has been minimized.

@ehg

ehg May 17, 2018

Member

Formatting.

This comment has been minimized.

@nuzzio
* The regex here will try to guess if it is an incomplete HTML tag
*
* <bold>Important Text</bold>
*

This comment has been minimized.

@ehg

ehg May 17, 2018

Member

Formatting.

This comment has been minimized.

@nuzzio
<?php
require_once( dirname( __FILE__ ) . '/../../../../modules/markdown.php');
class WP_Test_Markdown extends WP_UnitTestCase {

This comment has been minimized.

@ehg

ehg May 17, 2018

Member

Thanks for adding tests!

@nuzzio

This comment has been minimized.

nuzzio commented May 19, 2018

@ehg I think I would consider the first iteration complete for the most part at this point. I still think testing could be improved and JS testing needs to be implemented when the Gutenberg components are published independently for testing. That can be done in the final solution.

I get the general impression that using regex parsing isn't going to serve us well.

I don't normally have an issue with well commented regex used appropriately, we have 3 regex instances introduced and they function as follows:

  1. Remove the Markdown content wrapper, which is set by the JS plugin in this module and is tested.

  2. Preserve the non-Markdown content as specified by the original module, with regex patterns, in order to avoid munging Gutenberg blocks.

  3. Perform wpautop on each Markdown content block to achieve parity for with classic Markdown spacing rendering in the front of the site.

I would like a cleaner solution as well. Can you help me understand your concerns with regex?

I'm not sure how we could get rid of these, maybe another parser, leveraging Gutenberg's parser?
I think an option is thinking of Markdown for Gutenberg support more separated from the classic editor. This could open up more appropriate places to store the markdown/rendered HTML — which may obviate the need for regex/post_content_filtered. Do you have any ideas here? Maybe some more Gutenberg specific storage structures?

Yes, I agree that this is the ideal scenario. This was the vision of the 2nd and 3rd iterations in the original issue #9201.

I remember we spoke of the calypso support using a tinymce plugin. My intention had been to look into the possibility of using the same plugin and modifying the RichText component to become a Markdown editor.

Today I poked around the HTML block and noticed it seems to be using codemirror. If that is the case, perhaps we can try to use it and leverage Markdown Mode to try and supply the new functionality.

That would address the editing/preview experience and browser based parsing. Persistence of the Markdown data is another matter. I'll be looking at this today and post my thoughts a little later.

@nuzzio

This comment has been minimized.

nuzzio commented May 21, 2018

@ehg I implemented a quick version of the block today using codemirror and markdown-it. It looks promising, integrated pretty seamlessly. Saving the original markdown and the resulting html separately is a little challenging. I'll be exploring those options in the next two days.

@egh

This comment has been minimized.

egh commented May 21, 2018

@nuzzio wrong person :) I think you are looking for @ehg

@nuzzio

This comment has been minimized.

nuzzio commented May 21, 2018

Thank you! I updated the comment.

@ehg ehg closed this May 30, 2018

@Ferdev Ferdev referenced this pull request Jun 7, 2018

Closed

Markdown Gutenberg block for Jetpack #9705

8 of 11 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment