T/52: Code

[pomek]

Suggested merge commit message (convention)

Feature: Introduced the Code plugin. Closes ckeditor/ckeditor5#5547.

---

Additional information

Autoformatting will be done in https://github.com/ckeditor/ckeditor5-autoformat/issues/35.

[szymonkups]

There are some problems with single space handling:

• create empty paragraph, change to code style, try to enter space,
• in non-empty paragraph create code at the end of it, type some letters and try to type space.

It looks like space handling at the beggining/end of the code element is broken.

[pomek]

I tried to change the name of the element in view (tested on div, span, tt and even with custom elements _code and code_) and everything works fine.

IMO it looks like the code element has some super attributes and the editor does not render the spaces at the end of the element. In the model, these spaces are adding properly.

[Reinmar]

This problem with spaces deserves a separate ticket in ckeditor5-engine. And should not block this ticket.

[pomek]

Reported in https://github.com/ckeditor/ckeditor5-engine/issues/1116. So, could we merge this PR?

[pomek]

I added some styles but I am not CSS master, so it may require some changes.

[Reinmar]

@oleq, do you think we could ship with such code styles? They fix <code> rendering (GH and Umberto use similar ones).

[Reinmar]

Like... what?

[Reinmar]

This comment is unclear. This line is needed to fix empty code rendering in empty block.

But it was a good catch :)

[Reinmar]

I'll fix the those two issues that I found in the theme.scss.

[Reinmar]

The styling of the code element (the CSS generated spaces), unfortunately, add another selection step in Firefox:

OTOH, it works pretty well with it.

[Reinmar]

On Safari an error is thrown when typing spaces at the end of the <code> element. Might be related to https://github.com/ckeditor/ckeditor5-engine/issues/1116, so I'll add there a note.

[Reinmar]

@oleq, could you take a look at that SASS file and on how it affects FF? I mean – are we fine with that?

[Reinmar]

Hm, I have a better idea, actually:

1. Let's remove those styles – to KISS,

2. This means that spaces at the boundaries of <code> will be completely lost which leads to editor throwin an error if you type multiple of them:

   input.js:294 Uncaught TypeError: Cannot read property 'index' of undefined
       at MutationHandler._handleTextNodeInsertion (input.js:294)
       at MutationHandler.handle (input.js:152)
       at Input._handleMutations (input.js:105)
       at Document.listenTo (input.js:50)
       at Document.fire (emittermixin.js:281)
       at MutationObserver._onMutations (mutationobserver.js:248)
   

   I don't know why this error is actually throw. I suppose that it may have something to do with https://github.com/ckeditor/ckeditor5-engine/issues/1125 because I saw similar stack error there.

3. But the way to prevent that is to remove code from here:

   https://github.com/ckeditor/ckeditor5-engine/blob/0bc1097d0448c13bf7e7480ceafb5e738d1eb93f/src/view/domconverter.js#L68

   After doing this, multiple spaces in <code> elements will be rendered as a mix of normal spaces and non-breaking spaces.

   

[Reinmar]

To make it clear, with the proposed changes, such data will be generated:

<p>Fo <code>o &nbsp; b &nbsp; bom &nbsp; </code>ar</p>

Is it ok?

[fredck]

I think that not only this is ok but it is also the very right way to do it, in the HTML point of view.

I was just wondering what happens if one copies and pastes this code into real code... would nbsp characters end up there?

[Reinmar]

I was just wondering what happens if one copies and pastes this code into real code... would nbsp characters end up there?

In the model you'll have normal spaces. In the DOM you'll have the same as I showed above. Basically – on DOM -> view conversion (upon pasting) unnecessary  s will be turned to normal spaces.

[Reinmar]

The second <code> was the pasted one:

[fredck]

In the model you'll have normal spaces. In the DOM you'll have the same as I showed above. Basically – on DOM -> view conversion (upon pasting) unnecessary  s will be turned to normal spaces.

This doesn't help much, because one will still have to render the output HTML inside browsers... and the only way to do that is doing exactly how we do it.

[Reinmar]

This doesn't help much, because one will still have to render the output HTML inside browsers... and the only way to do that is doing exactly how we do it.

??? But you get data through getData() and see this:

<p><i>This</i> <code>is &nbsp; &nbsp; an</code> <strong>editor</strong> <u>ins</u><code>is &nbsp; &nbsp; an</code><u>tance</u>.</p>

Which renders correctly.

[fredck]

Actually, what HTML the editor itself would output for this?

[fredck]

Ok then... so we have to confirm that copy and paste from that produces "pure" spaces.

[Reinmar]

@fredck please see my above comments ;) I confirmed exactly that:

#55 (comment)

And also what data will be returned:

#55 (comment)

[fredck]

That's wonderful then... so your proposal is the very right way for it ;)

[fredck]

To note that other converters, like Markdown, will have a sequence of plain spaces there instead. It'll be the job of the third-party MD-to-HTML libraries to convert those spaces on something that renders right inside browsers.

[Reinmar]

@pomek – let's close https://github.com/ckeditor/ckeditor5-engine/issues/1126 first. And then, let's clean up this PR.

[Reinmar]

To note that other converters, like Markdown, will have a sequence of plain spaces there instead. It'll be the job of the third-party MD-to-HTML libraries to convert those spaces on something that renders right inside browsers.

Haha, it's not like that. GFM e.g. leaves spaces as they are and GH or other websites using it need to fix this using that hacky CSS I posted in https://github.com/ckeditor/ckeditor5-engine/issues/1116#issuecomment-326243007. We've been there with Umberto too ;<

[fredck]

Haha, it's not like that. GFM e.g. leaves spaces as they are and GH or other websites using it need to fix this using that hacky CSS I posted in ckeditor/ckeditor5-engine#1116 (comment). We've been there with Umberto too ;<

Yes, ofc. What I meant to say is that systems that use CKEditor 5 to output Markdown will have to implement their MD-to-HTML scripts in a way that spaces are properly converted IF they want to have browsers rendering their data in the way their users see it in the editor.

This means that we can already predict that many systems will be incompatible with that and that a plugin that avoids multiple spaces will be born to "solve" this problem, the other way around.

[pomek]

@Reinmar, after merging changes in engine, this PR is ready to review once again.