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 support for shift+enter (soft line breaks) #4542

Closed
Reinmar opened this issue Apr 18, 2016 · 28 comments
Closed

Add support for shift+enter (soft line breaks) #4542

Reinmar opened this issue Apr 18, 2016 · 28 comments
Assignees
Labels
package:enter type:feature This issue reports a feature request (an idea for a new functionality or a missing option).
Milestone

Comments

@Reinmar
Copy link
Member

Reinmar commented Apr 18, 2016

We're not going to implement this in #4541 as it's not needed currently and we don't have line break support planned.

@VincentClair
Copy link

Hello,

Hope you could implement it soon for inline purpose. I think it could be very useful when ckeditor is applied to Hn, P, SPAN or other editable root elements that don't allowed P inside.

Thanks

@alexeckermann
Copy link

Hi. Is there any updates on where soft breaks is on the development timeline, level of importance?

@fredck
Copy link
Contributor

fredck commented Oct 16, 2017

I think it is very important to point out here realistic use cases for this feature. The ones described by @VincentClair are "theoretically" right but don't smell good, when it comes to quality content creation. For example, do I really want to have line breaks in a header? Or if I really expect line breaks to happen, why will I enable editing a <span>, instead of a <div> with paragraphs?

There must be a good semantic-oriented justification that motivates the existence of such feature and the will indicated that it is more important than other features that we have in the backlog.

In the other hand, this one fits well as a community contributed feature.

@alexeckermann
Copy link

@fredck thanks for the update. Great to see the care thought going into this. I've been working with CK5 in the past week and I am utterly blown away, it's amazing.

Just for some background; the content creation that I am developing for is around literary work — stories, poems, academic material, etc. In these cases authors tend to want to finesse the structure of the content and this tends to include the usage of <br /> soft breaks — legacy of typesetting done in Word. However, the difference between a soft break and a paragraph break — when it comes down to visual representation — could be almost identical if handled differently.

An idea I have to get around supporting br tags (of which im not sure the current data modelling supports?) would be to handle Shift+Enter as a new paragraph but with some kind of class or attribute to be handled by CSS to give a soft-break feel. Does that approach have merit as far as the CK5 structure goes? If so, that could just be a plugin away and no deep adjustments needed.

@fredck
Copy link
Contributor

fredck commented Oct 17, 2017

@alexeckermann, I clearly understand your use case. It's related to the artistical face of writing and such soft-breaks are indeed important.

In fact, the HTML5 specs even predict a proper use for <br>, even if it's a limited use case:

br elements must be used only for line breaks that are actually part of the content, as in poems or addresses.

What we want to avoid, by not providing such feature by default, is the other part of the spec:

br elements must not be used for separating thematic groups in a paragraph.

But this should be an edge-case. Therefore, bringing such feature to life makes total sense and it could live as a contributed plugin.

So here goes the challenge... don't you want to bring this plugin to life? This would be very helpful for us so we can have feedback from the community about the custom development of features.

When it comes to the implementation, there is no need to try to figure out crazy solutions for it. I see this as an empty break element in the model, which is converted to <br> in the view. This should work without many issues.

Maybe others would be happy to jump in and help if you are really interested in this.

@Reinmar
Copy link
Member Author

Reinmar commented Oct 17, 2017

We don't have a complete guide about implementing such a feature yet, so I'll try to give here some examples.

The code of a soft line break feature will be very similar to the bold's feature (see https://github.com/ckeditor/ckeditor5-basic-styles/blob/master/src/boldengine.js).

However, instead of registering an attribute on inline nodes you will be allowing line breaks
elements inside blocks (to make it clear when we're talking about the model and when about the view I called this element softLineBreak in the model):

doc.schema.allow( { name: 'softLineBreak', inside: '$block' } );

And instead of converting to/from model attribute your feature should be converting model element to view element (and back):

		// Build converter from model to view for data and editing pipelines.
		buildModelConverter().for( data.modelToView, editing.modelToView )
			.fromElement( 'softLineBreak' )
			.toElement( 'br' );

		// Build converter from view to model for data pipeline.
		buildViewConverter().for( data.viewToModel )
			.fromElement( 'br' )
			.toElement( 'softLineBreak' );

This should enable loading and rendering <br> elements.

Another step is to add a listener to editor.editing.view#keydown event and handle it. Let's say that the simplest implementation (without a command) will look like this:

import { keyCodes } from '@ckeditor/ckeditor5-utils/src/keyboard';
import ModelElement from '@ckeditor/ckeditor5-engine/src/model/element';

// In your plugin's init() method:
this.listenTo( editor.editing.view, 'keydown', ( evt, data ) => {
    if ( data.keyCode == keyCodes.enter && data.shiftKey ) {
        editor.data.insertContent( new ModelElement( 'softLineBreak' ), editor.document.selection );
    } 
} );

Hopefully, this is all that is needed. But since we didn't test editor for such cases yet there may be something I miss or something will simply break.

@alexeckermann
Copy link

@fredck @Reinmar Yup, im more than happy to create a plugin and share it. Thanks for the guidance. Will let you know when a public repo is up.

Before I commented on this issue I did have a quick attempt at handling <br> (using other plugins as examples) which kind of worked — it rendered with <br></br>. I'll take all the advice above and funnel that into something better.

@Reinmar
Copy link
Member Author

Reinmar commented Oct 18, 2017

Before I commented on this issue I did have a quick attempt at handling <br> (using other plugins as examples) which kind of worked — it rendered with <br></br>. I'll take all the advice above and funnel that into something better.

Interesting. @szymonkups , am I right that this might be because the model to view converter used a wrong view element type? I guess it uses a Container/AttributeElement while in this case we need an EmptyElement. So, this would mean that the code I posted above will not work as well (the toElement() section is wrong). Is it correct?

import EmptyElement from '@ckeditor/ckeditor5-engine/src/view/emptyelement';

// Build converter from model to view for data and editing pipelines.
buildModelConverter().for( data.modelToView, editing.modelToView )
	.fromElement( 'softLineBreak' )
	.toElement( new EmptyElement( 'br' ) );

@szymonkups
Copy link
Contributor

EmptyElement is a way to go here. BuildModelConverter uses ContainerElements by default when using fromElement().toElement().

@RedHatter
Copy link

Any progress on this?

@alexeckermann Were you ever able to get your plugin working correctly?

@alexeckermann
Copy link

@RedHatter It's still in progress. Unfortunately I have had to jostle around my priorities and this has been pushed back a little bit. However, because I need this in the current project we're working on its going to be done sometime soon.

@Reinmar Reinmar changed the title Add support for shift+enter Add support for shift+enter (soft line breaks) Nov 17, 2017
@alexeckermann
Copy link

alexeckermann commented Nov 23, 2017

I now have a repo public for a soft break plugin 🎉

However, it is currently not working because of a model conversion issue I just cant for the life of me see to fix.

You can access the prerelease branch here https://github.com/alexeckermann/ckeditor5-soft-break/tree/prerelease

Any assistance with resolving those two tests would be greatly appreciated. Im sure its something obvious…

Update: Found the problems with the test failing. Was a misunderstanding of the conversion process, based upon copying existing plugins but lacking the nuanced understanding.

@Reinmar
Copy link
Member Author

Reinmar commented Feb 14, 2018

The lack of soft line-break feature badly affects how pasting plain-text works: #766.

@theskyfloor
Copy link

Just adding a vote for how important this is for those of us that want to offer CKEditor 5 in our CMS

@Reinmar Reinmar assigned pomek and unassigned pomek May 4, 2018
@Reinmar
Copy link
Member Author

Reinmar commented May 4, 2018

@alexeckermann already proposed a PR in ckeditor/ckeditor5-enter#54, but there are a couple of things which we need to consider here:

  1. We need to take care of creating a proper setup of plugins. The entry point to this package is the Enter plugin and let's keep that to not introduce breaking changes. It should load both enter modes – hard and soft return support. So, it should be an empty plugin requiring two other plugins (one for each type). Now, I have a problem how to name them. HardReturn and SoftReturn or HardEnter and SoftEnter.

  2. Commands should be named accordingly too. There should be two separate commands because these features need to be treated separately. E.g. hard return will be disabled in captions while soft return can be enabled.

    One problem with commands is that we already have enter command. We can either introduce a breaking change here (by renaming it to hardEnter) or make enter an alias for hardEnter. Since we're on an early stage and this is a small change, I'd propose a BC.

  3. Make sure to implement a proper isEnabled refresh logic. E.g. the soft break command should be disabled if <br> is not allowed in this context.

  4. I'm not sure about the name of the model element. I'm leaving it up to the implementor.

  5. Both plugins should use the EnterObserver. This means that the enter observer should now fire the enter event with the isSoft property.

    I know there's a "TODO" comment in the code to use the keystroke handler... We may do that change now and remove the enter observer. I'm still unsure which way is better. I like observers and more semantical intent events (like enter or delete) and they are more future proof. In the future, once beforeinput is usable, we won't handle keystrokes but intent events. This will make the keystroke handler obsolete in its current form. It will need to be able to translate intent events to commands instead of only keystrokes to commands. Which, perhaps, means that we should leave the EnterObserver. Perhaps there's a space for both things. We have intent events -> commands mapping (currently, only programmatic, but in the future perhaps also declarative). Plus, you can handle keystrokes if you want.

    So, let's leave the EnterObserver :D

  6. We don't need the split to EnterEditing and EnterUI since this feature doesn't bring any UI.

@Reinmar
Copy link
Member Author

Reinmar commented May 4, 2018

Another thing to consider – what should happen if soft break is not loaded (see e.g. ckeditor/ckeditor5-block-quote#23 (comment)).

@scofalik
Copy link
Contributor

scofalik commented May 9, 2018

IMHO Enter and SoftBreak are explicit enough and we would not have to change anything. If we introduce the new feature as SoftBreak we can just stick with softBreak command name and leave enter as is. For element name, I propose <break> or <softBreak>.

@Reinmar
Copy link
Member Author

Reinmar commented May 14, 2018

Initially I thought that we need to name 3 plugins – SoftBreak, HardBreak and one which would load these two (it'd be called Enter so developers would find it first). But I recalled now that Enter is loaded by Essentials so very few people will rely on the Enter plugin directly. So, we could have this:

  • Enter
  • SoftEnter
  • Essentials loading both of them.

WDYT?

@pomek
Copy link
Member

pomek commented May 14, 2018

👍

It allows avoiding the BC with the enter command.

@AnnaTomanek
Copy link
Contributor

Just a thought, how about using SoftReturn instead of SoftEnter?

Reasoning: "soft return" is semantically closer to classic typesetting, while "Enter" and "line break" feel more connected with using and abusing HTML. Using "soft return" instead could maybe, hopefully hint to some users what the intended purpose of this feature is about.

@Reinmar
Copy link
Member Author

Reinmar commented May 15, 2018

The pair Enter and SoftReturn would look odd next to each other.

At the same time, SoftEnter is something I made up – there's only SoftReturn or SoftBreak.

This means that we can rule out pairs like:

  • Enter and SoftReturn
  • Enter and SoftBreak
  • Enter and SoftEnter

So, I've got two options:

  • Enter and ShiftEnter like in CKEditor 4. Such plugins are focused on how users interact with these features (by using these specific keystrokes) not on their semantics. (❤️ )
  • Enter (as an entry point) plus HardReturn and SoftReturn. Like in the earlier proposal. In this case, we'll lose the enter command, which will be backwards incompatible. (🎉 )

You can vote with the ❤️ / 🎉 reactions ;)

@joachimdoerr
Copy link

Any updates? @Reinmar
That is an very important base function and the community solution will not work with the current cke5 version.

@Reinmar
Copy link
Member Author

Reinmar commented Jun 4, 2018

The PR is mostly ready, but we'll need to iron out a couple of bugs before enabling it by default in all builds.

@joachimdoerr
Copy link

@Reinmar when will you create a new npm build that contain the shift+enter?

@Reinmar
Copy link
Member Author

Reinmar commented Jun 11, 2018

Within 2-3 weeks. We're polishing the upcoming release now. If you want to use it earlier, you should be able to do that quite safely from the development version of CKEditor 5.

@Xoduz
Copy link

Xoduz commented Jun 13, 2018

Sounds great! Would this shift + enter feature also work inside "code" elements, aka < code >?

@Reinmar
Copy link
Member Author

Reinmar commented Jun 13, 2018

You will be able to do this:

image

But the output is this:

"<p><i>This</i> <s>is</s></p><p><code>dzdfsdn</code><br><code>sdfsdfsdf</code><br><code>Sdfsdfsdf</code><br><code>sdfsdfsdf</code><br><code>sdfsdf</code></p><p>sdfsdfdsf<br>&nbsp;<strong>editor</strong> <u>instance</u>.</p>"

So I guess that what you want are code blocks and this is a separate feature (which is not yet available): #436

@mlewand mlewand transferred this issue from ckeditor/ckeditor5-enter Oct 9, 2019
@mlewand mlewand added this to the iteration 18 milestone Oct 9, 2019
@mlewand mlewand added status:confirmed type:feature This issue reports a feature request (an idea for a new functionality or a missing option). package:enter labels Oct 9, 2019
@indralekha
Copy link

how can i make the enter button behave in same way as Shift+enter (soft Break). I dont want line breaks, (on how enter key is working) but want it a soft break.
Using ShiftEnter Plugin, could solve the prob but it is impacting Bulletlist, numbered list on enter, as it is taking shift+enter and not adding a bullet point after pressing enter.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package:enter type:feature This issue reports a feature request (an idea for a new functionality or a missing option).
Projects
None yet
Development

No branches or pull requests