Introduce new CodeMirror block for editing code. #665

dmsnell · 2017-05-04T17:24:37Z

While demonstrating Gutenberg to my team the question was asked about
editing code blocks: "it's so terribly difficult in the WordPress editor."

Wanting to demonstrate some of the flexibility that the block system
provides I created a new block type which turns a normal block into an
instance of the CodeMirror editor.

Issues:

How do we determine which modes to show? I think we should allow for all of them but have a better way to select the common modes. Maybe an auto-completing drop-down menu in the inspector.
Travis fails on CodeMirror's CSS but it runs locally

Also shown here with a degraded network connection to show the async code loading in when requested

BE-Webdesign · 2017-05-19T20:38:13Z

does anyone care about this? any reason to keep it around?

I think this would be an awesome thing to have in the editor. This could also potentially become a good test case for building a plugin that registers a block. I doubt the vast majority of WordPress users will need/want this kind of content block, but any programmer would probably really like this!

dmsnell · 2017-05-19T20:47:25Z

thanks @BE-Webdesign - your comment makes me realize that I should create this at some point outside of the project as a new-editor plugin!~

closing for now then

ellatrix · 2017-06-11T19:57:04Z

blocks/library/code-mirror/index.js

+const { html } = query;
+
+const toEditor = s => he.decode( s.split( '<br>' ).join( '\n' ) );
+const fromEditor = s => he.encode( s, { useNamedReferences: true } ).split( '\n' ).join( '<br>' );


If you use text to query for the attribute, and <pre><code>, this is all not needed? React will do it by itself? That's what I did with the plain code block.

…will investigate, thanks!

have you not had any problems with special characters or whitespace? can I get rid of he altogether?

(note also that he has a few cool points, like being able to choose how and when characters get encoded)

I think you can remove it entirely, yes. It will convert to named entities.

(note also that he has a few cool points, like being able to choose how and when characters get encoded)

It's also a behemoth of a library, at least 24kb minified + gzipped. Might be a point toward dynamically enqueueing block scripts separately based on presence in content.

thanks! I pulled it in initially because it was a prototype. I'm very glad you were aware of how <pre><code> changed the semantics. much better now!

ellatrix · 2017-06-11T19:57:23Z

blocks/library/code-mirror/index.js

+	edit( { attributes, setAttributes } ) {
+		return (
+			<div>
+				<select onBlur={ ( { target: { value } } ) => setAttributes( { language: value } ) }>


Wouldn't onChange be better?

had onChange to begin with but eslint yapped at me.

used onChange - please see my new comment asking about this…

ellatrix · 2017-06-11T19:59:09Z

blocks/library/code-mirror/index.js

+					{ [ 'css', 'diff', 'javascript', 'markdown', 'pegjs', 'php' ].map( language => (
+						<option
+							key={ language }
+							selected={ language === attributes.language }


Getting a warning:

Warning: Use the defaultValue or value props on <select> instead of setting selected on <option>.

thanks. I honestly didn't pay any attention to this (obviously). it seems like I need to make it a setting in the page inspector too but at the time I didn't know about that

used value

ellatrix · 2017-06-11T19:59:31Z

post-content.js

@@ -163,6 +163,10 @@ window._wpGutenbergPost = {
 			'https://make.wordpress.org/core/2017/01/17/editor-technical-overview/',
 			'<!-- /wp:core/embed -->',

+			'<!-- wp:core/code-mirror language="php" -->',


…will change to Elm 😉

changed to Elm 😄

dmsnell · 2017-06-13T21:28:00Z

I've got some difficulty understanding the CSS errors. They are coming in via CodeMirror.

Honestly I would prefer to be able to dynamically load the CSS but I'm not sure how that would happen. if someone would be willing to help me through these errors/async issues I'll buy you an 🍦

dmsnell · 2017-06-13T21:30:02Z

blocks/library/code-mirror/index.js

+require( 'codemirror/mode/markdown/markdown' );
+require( 'codemirror/mode/pegjs/pegjs' );
+require( 'codemirror/mode/php/php' );
+require( 'codemirror/lib/codemirror.css' );


there are more of these. if we had a way to async load them I would do so on-demand and provide the full suite with a better language selector

Did you try using import instead of require here? It should teach Webpack to async load all those files. I guess we would still have to wrap CodeMirror into its own component to make sure those deps are loaded only when it mounts.

I tried using import() and it didn't seem to recognize it. In d957054 I added the use of require.ensure() which seems to mostly work. There is still an issue that it's bundling the entirety of the modes directory into one chunk instead of many.

westonruter · 2017-06-15T14:47:28Z

FYI: @georgestephanis has a feature plugin for introducing CodeMirror into other areas of WP. The block perhaps could be made part of that plugin and re-use the copy of CodeMirror that it includes.

See https://github.com/georgestephanis/codemirror-wp

georgestephanis · 2017-06-15T14:56:04Z

Just for a separation of concerns, and to keep Gutenberg a little bit lighter, it might be wise for the Gutenberg version of a code editor block to just be a plain textarea, and have the CodeMirror-WP plugin handle turning it more interactive and engaging?

I'm happy to add anyone to the cm repo, just ask.

dmsnell · 2017-06-15T15:17:13Z

and have the CodeMirror-WP plugin handle turning it more interactive and engaging?

thanks @georgestephanis. the block editor is a very good place, in my opinion, to pull in code mirror. since it is implementation specific I would imagine a WordPress plugin that both registers the editor block and includes the script into the browser.

this PR isn't going to be considered ready regardless until we can asynchronously load in CodeMirror and its modes. it would keep Gutenberg lightweight and make code editing first-class.

actually I would prefer to generalize it as something like "syntax editor" or "code editor" in its name so we could swap out CodeMirror if we wanted. (it started out this way as "code" until @iseulde took that name)

gziolo

Do we have async loading of the library yet?

Is this the only remaining blocker here? Do we want to have it in core or rather as a plugin? I'm happy to help with this one.

gziolo · 2017-09-22T07:39:09Z

Reopening for growing interest

I'm not surprised :) Using CodeMirror was my first thought when I saw the existing code block 😄 Thanks for opening this PR!

mtias · 2017-09-22T11:57:21Z

@gziolo WordPress 4.9 will ship with it, so we can enqueue it from there, which would make things easier. It'd be nice to still async load it, if that's something you want to explore.

ellatrix · 2017-09-22T12:07:42Z

If core adds it, let's add it to the default code block, but also keep a "plain" type (no language). Also note we'll need highlighting on the front-end. Prism is quite nice.

mtias · 2017-09-22T12:11:33Z

We used it for the docs: http://gutenberg-devdoc.surge.sh/block-api/

With a tweaked stylesheet.

mtias · 2017-09-22T12:11:57Z

And agreed that if it lands in core we should make it part of the main block.

dmsnell · 2017-09-24T01:18:27Z

This has been updated to lazy-load CodeMirror and also the language modes. There are some problems indicated inside the code itself on those. Notably:

When dynamically loading the CSS it ends up rendering wrong so I have left the CSS as a static import
The mode files are all being bundled into one file instead of having separate chunks for each mode. This is related to require.context() and webpack statically anticipating the import. I believe that by doing some twiddling we can get what we want. I'm having a hard time really understanding what webpack is doing. After some rework I was able to split the mode files into separate bundles by adding statically-assigned System.import() statements

At first I tried manually inserting a new <script> tag into the document head, which was loading files as requested, but the paths were screwy and I had issues with the loaded files expecting CodeMirror to be a global in a specific state which didn't make sense. Ideally I think we'd rely on webpack to handle the glue for us.

The tests are still failing and I didn't really bother to look at why. The PR has known issues to address before fixing the tests.

For some reason the demo content is showing "This block appears to have been modified externally" but I don't know why. Also, I cannot click on the options in that dialog. I can tab to the buttons "Overwrite" and "Convert to classic text" but the clicks are completely lost. Also, I don't know what "Overwrite" means there. The block was missing the CSS class in post-content.js but that has been added and the warning is gone.

gziolo · 2017-09-25T08:51:30Z

blocks/library/code-mirror/index.js

+// @see node_modules/codemirror/modes
+// when supported we can probably replace with standard `import()` syntax
+const modes = {
+	apl: { label: 'APL', req: () => System.import( 'codemirror/mode/apl/apl' ) },


System.import is deprecated. You should use dynamic import instead`. See webpack 3 announcement post: https://medium.com/webpack/webpack-3-official-release-15fd2dd8f07b:

import(/* webpackChunkName: "my-chunk-name" */ 'module');

I just noticed we are on Webpack 2.7, but I think it still supports import, but without this magic comments.

See the comment. import() was throwing errors as not supported

I can double check though too

Maybe we miss something in Babel config ¯_(ツ)_/¯

@gziolo and @aduth I'm getting this when trying to use import()

ERROR in ./blocks/library/code-mirror/index.js Module build failed: SyntaxError: Unexpected token (24:33)

Not seeing a source of that error either.

babel/babel-loader#493

according to that ^^^ it seems like it's still experimental in babel so even though we have webpack support for it we're not going to get it unless we pull in a new dependency. I'm happy leaving this PR without additional dependencies.

https://github.com/tc39/proposal-dynamic-import - it's stage 3. It learned yesterday that Gutenberg uses only features that are accepted by ECMA TC39. So this one needs to wait. It's good as it is :)

gziolo · 2017-10-06T06:20:51Z

blocks/library/code-mirror/index.js

+// when this CSS was imported dynamically
+// it was causing major visual glitches to
+// the editor which completely broke it
+require( 'codemirror/lib/codemirror.css' );


All styles are loaded using the following syntax:

import './style.scss';

Webpack uses the following pattern test: /style\.s?css$/, to match styles. So it should work here, too. Unless this is the reason why it breaks :)

actually I think it had to do with race conditions on the mounted components. I was comparing to async loading as the dynamic bits though vs. import statement.

mainly I meant to convey that this stylesheet seemed to need to load before the editor and I wasn't sure how best to do that

gziolo · 2017-10-06T06:52:32Z

The tests are still failing and I didn't really bother to look at why. The PR has known issues to address before fixing the tests.

Yes, it's enforced to add fixtures for all blocks in here: https://github.com/WordPress/gutenberg/blob/master/blocks/test/full-content.js#L198.

While demonstrating Gutenberg to my team the question was asked about editing code blocks: "it's so terribly difficult in the WordPress editor." Wanting to demonstrate some of the flexibility that the block system provides I created a new block type which turns a normal block into an instance of the CodeMirror editor. This should eventually replace the standard `code` block once it's ready for production. At the moment it lazy-loads the `CodeMirror` editor _and_ the mode files, so thankfully this keeps the initial builds small.

karmatosed · 2018-03-07T14:31:47Z

As talked about in this week's chat, closing this as now Codemirror is in for one block it needs iterating.

gziolo · 2018-03-07T14:33:46Z

https://github.com/WordPress/gutenberg/blob/master/components/code-editor/index.js - we have CodeMirror integrated here, we should try to use a similar approach or even better extend the existing one to make it more flexible.

dmsnell · 2018-03-07T18:27:37Z

😢

this was my favorite block

dmsnell force-pushed the try/new-block-codemirror branch from 7d0defe to 32275e5 Compare May 4, 2017 17:25

mtias added the [Status] In Progress Tracking issues with work in progress label May 4, 2017

dmsnell closed this May 19, 2017

dmsnell deleted the try/new-block-codemirror branch May 19, 2017 20:47

dmsnell restored the try/new-block-codemirror branch June 11, 2017 12:25

dmsnell reopened this Jun 11, 2017

dmsnell force-pushed the try/new-block-codemirror branch 4 times, most recently from 19674f4 to 06893cb Compare June 11, 2017 13:08

dmsnell requested review from aduth, mtias and ellatrix June 11, 2017 13:09

dmsnell force-pushed the try/new-block-codemirror branch from 086e7b4 to d75a2fc Compare June 11, 2017 13:54

dmsnell added [Feature] Rich Text Related to the Rich Text component that allows developers to render a contenteditable and removed [Status] In Progress Tracking issues with work in progress labels Jun 11, 2017

ellatrix reviewed Jun 11, 2017

View reviewed changes

dmsnell force-pushed the try/new-block-codemirror branch from d75a2fc to a331c56 Compare June 13, 2017 20:44

dmsnell added the [Feature] Block API API that allows to express the block paradigm. label Jun 13, 2017

dmsnell commented Jun 13, 2017

View reviewed changes

dmsnell force-pushed the try/new-block-codemirror branch from f1f4488 to b88aa08 Compare June 15, 2017 11:12

dmsnell force-pushed the try/new-block-codemirror branch from f42a1e3 to 5660dd1 Compare July 4, 2017 14:04

gziolo reviewed Sep 22, 2017

View reviewed changes

gziolo assigned gziolo and unassigned gziolo Sep 22, 2017

gziolo mentioned this pull request Sep 22, 2017

Block and Component Lazy Loading #2768

Open

3 tasks

dmsnell force-pushed the try/new-block-codemirror branch from 5660dd1 to 272c6e6 Compare September 23, 2017 22:39

dmsnell force-pushed the try/new-block-codemirror branch from c344cb1 to fab4ec9 Compare September 24, 2017 01:25

gziolo reviewed Sep 25, 2017

View reviewed changes

dmsnell force-pushed the try/new-block-codemirror branch 3 times, most recently from d85bc27 to 6b222b5 Compare October 4, 2017 16:05

gziolo reviewed Oct 6, 2017

View reviewed changes

aduth mentioned this pull request Oct 9, 2017

Blocks: Revisit pattern for enqueuing scripts and styles #2756

Closed

dmsnell force-pushed the try/new-block-codemirror branch from 6b222b5 to 0bec9a2 Compare December 2, 2017 19:17

noisysocks mentioned this pull request Jan 8, 2018

Add CodeMirror to the HTML block #4348

Merged

dmsnell added 2 commits January 25, 2018 12:12

Update with project

94abcdc

dmsnell force-pushed the try/new-block-codemirror branch from a714d0d to 94abcdc Compare January 25, 2018 17:12

karmatosed closed this Mar 7, 2018

gziolo deleted the try/new-block-codemirror branch March 7, 2018 14:33

Introduce new CodeMirror block for editing code. #665

Introduce new CodeMirror block for editing code. #665

Conversation

dmsnell commented May 4, 2017 • edited Loading

BE-Webdesign commented May 19, 2017

dmsnell commented May 19, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dmsnell commented Jun 13, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

westonruter commented Jun 15, 2017

georgestephanis commented Jun 15, 2017

dmsnell commented Jun 15, 2017 • edited Loading

gziolo left a comment

Choose a reason for hiding this comment

gziolo commented Sep 22, 2017 • edited Loading

mtias commented Sep 22, 2017

ellatrix commented Sep 22, 2017

mtias commented Sep 22, 2017

mtias commented Sep 22, 2017

dmsnell commented Sep 24, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dmsnell Oct 4, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gziolo commented Oct 6, 2017

karmatosed commented Mar 7, 2018

gziolo commented Mar 7, 2018

dmsnell commented Mar 7, 2018

dmsnell commented May 4, 2017 •

edited

Loading

dmsnell commented Jun 15, 2017 •

edited

Loading

gziolo commented Sep 22, 2017 •

edited

Loading

dmsnell commented Sep 24, 2017 •

edited

Loading

dmsnell Oct 4, 2017 •

edited

Loading