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

Problems with text attributes in newly created lines #4562

Closed
Reinmar opened this issue Mar 10, 2017 · 15 comments · Fixed by ckeditor/ckeditor5-enter#63
Closed

Problems with text attributes in newly created lines #4562

Reinmar opened this issue Mar 10, 2017 · 15 comments · Fixed by ckeditor/ckeditor5-enter#63
Assignees
Labels
package:enter type:bug This issue reports a buggy (incorrect) behavior.
Milestone

Comments

@Reinmar
Copy link
Member

Reinmar commented Mar 10, 2017

mar-10-2017 13-41-57

You can see a couple of things which got wrong here:

  1. The style of the first line was not copied to the next line.
  2. The style applied when a line was empty (so it was applied to the block) was copied – which means that we're copying block element styles (which is fine, in general) but it also means that once we started typing in that empty block the style was not moved to the text (which isn't fine).
  3. The problem mentioned in 2. explains why after removing bold from the text and pressing enter, there's bold in the last block – it's taken from the paragraph itself (which shouldn't have the style at that point).

So, things to fix:

  1. The style should be removed from a block once something is inserted in it. Who's job should it be? Can we actually understand which attrs applied to a block are inline attrs? I remeber we discussed this but I don't know how we implemented it.
  2. Enter feature should copy style of the selection when moving it to the new block.

👍 if you want this issue to be fixed

@Reinmar
Copy link
Member Author

Reinmar commented Mar 10, 2017

We talked briefly with @scofalik and it seems that the listener which clears the element attributes (they can be recognised by the selection: prefix) will need to be placed in the Document.

It should be fairly simple – it needs to listen to insert and move operations and just clear attributes of an element to which insertion happens. It doesn't even need to check if it's empty or not – if, by any coincidence the attributes weren't cleared before they can be cleared now.

@Reinmar Reinmar self-assigned this Jul 5, 2017
@Reinmar
Copy link
Member Author

Reinmar commented Jul 5, 2017

Enter feature should copy style of the selection when moving it to the new block.

This point means a bit more work than I thought because other features which override the enter behaviour need to do this as well. This will require tests in heading, list and blockquote.

@Reinmar
Copy link
Member Author

Reinmar commented Jul 5, 2017

Next minute, next thought – this is even more tricky. The linkHref attribute should not be copied with other attributes. Actually, the selection at the end of a link should behave differently, but since in general, I think that you must be able to somehow type inside a link, pressing enter in such a situation should not cause copying that attribute.

So, we need some... schema configuration? Or link feature changing the behaviour of the enter command? Or an extensible DataController.insertLineBreak()?

@Reinmar Reinmar changed the title Problems with text attributes in the newly created lines Problems with text attributes in newly created lines Jul 5, 2017
@Reinmar
Copy link
Member Author

Reinmar commented Jul 5, 2017

BTW, I'm not sure whether we should copy attributes to new lines at all. At least not the semantical ones. From the content semantics perspective, if you press enter you start a new block and a new thought. Why continuing e.g. bold? Just like – why continuing a link?

So, I'd vote for not implementing this feature until we need to support font colors, sizes, etc. This would mean that for now we need to only fix this bit:

The style should be removed from a block once something is inserted in it.

@fredck
Copy link
Contributor

fredck commented Jul 5, 2017

Agree. This is not totally critical and can be left like this for the first release, although it would be nice to have.

I would not wait too much for this though. It should be dealt as soon as the critical stuff is closed, imho.

@Reinmar
Copy link
Member Author

Reinmar commented Jul 5, 2017

So, we need some... schema configuration? Or link feature changing the behaviour of the enter command? Or an extensible DataController.insertLineBreak()?

As for how to differentiate between attributes which need to be copied and these which shouldn't, I'd make this controlled by the schema. This will leave it configurable for the end developers (and I think that it was requested in the past that this behaviour is configurable) and will allow implementing the entire behaviour in one place.

@Reinmar
Copy link
Member Author

Reinmar commented Jul 5, 2017

I moved the engine part to https://github.com/ckeditor/ckeditor5-engine/issues/1001 because it's a bug and should be fixed now.

@Reinmar Reinmar removed their assignment Jul 5, 2017
@scofalik
Copy link
Contributor

scofalik commented Jul 6, 2017

For me, as far as user experience goes, I'd certainly expect that bold and italic are retained after creating new paragraph. If I'd see they are not, I'd think it's a bug.

So I am of opinion that we will have to do it sooner or later.

@Reinmar
Copy link
Member Author

Reinmar commented Jul 6, 2017

For me, as far as user experience goes, I'd certainly expect that bold and italic are retained after creating new paragraph. If I'd see they are not, I'd think it's a bug.

I thought the same initially. But then I started thinking – am I just used to that or is it really needed?

MS Word, GDocs and other editors implement this behaviour because in most of them the visual aspect of the content (the styling) is equally important as the content itself. That's WYSIWYG stuff.

CKEditor 5 is not meant to be a WYSIWYG. We're always thinking about the semantics. And I can't think of a reason why you'd like to continue styles like bold and link in a new paragraph. Once we'll implement styling options like font color or size, the story will be different. But for now, with the options that we have, I don't see why these styles should be continued.

@Reinmar
Copy link
Member Author

Reinmar commented Mar 23, 2018

DUP in #919

And there's a bit different type of issue in the ckeditor5-alignment pkg: https://github.com/ckeditor/ckeditor5-alignment/issues/8

@Reinmar
Copy link
Member Author

Reinmar commented Aug 30, 2018

DUP in #1222.

@Reinmar
Copy link
Member Author

Reinmar commented Feb 11, 2019

We're discussing a mechanism for describing whether attributes convey styling or formatting in #908.

@Reinmar
Copy link
Member Author

Reinmar commented Apr 15, 2019

There are two attributes today which shouldn't be copied to a new line: linkHref and mention. However, mention is not really a problem today because mention is automatically removed from selection attributes, so you can never extend an existing mention.

However, we need to take 3rd party plugins into consideration so we cannot hardcode this. I'd, therefore, use a new attr prop called e.g. copyOnEnter.

@msamsel
Copy link
Contributor

msamsel commented May 28, 2019

This point means a bit more work than I thought because other features which override the enter behaviour need to do this as well. This will require tests in heading, list and blockquote.

@Reinmar what's an expected behaviour of such cases? Right now I have solution which copy styles regardless of previous block. Should styles from blockquotes, lists, heading be preserved when user leave given block?

I saw that google docs clear styles for heading, however lists and block quotes styles remain untouched.

@msamsel
Copy link
Contributor

msamsel commented Jul 12, 2019

I've noticed one thing during work on this issue. There is a slight inconsistency with the behaviour of enter and shiftenter commands. The shiftenter command works in a reversed way to the enter command. It preserver all styling even when you remove them.

Current state

For situation when there is collapsed selection at the end of a line.

Enter

When Enter key is pressed then no styles are copied to a new line.

Shift + Enter

When Shift+Enter is pressed then all styles are copied to a new line.

Introducing copyOnEnter attributes.

For this case, we want to copy styles from a given line to the next one. But only those whos attributes has property copyForEnter. For example, if the editor is configured to copyOnEnter Italic, but not bold, then only italic is copied to the next line.

However, the question is how the editor should behave for Shift+Enter? As current behaviour copies all attributes anyway. It's highly noticeable in a situation like this:

  1. Create a text in the editor with some styles. For example bold, italic, underline.
  2. Move collapsed selection to the end of line
  3. Remove all styles
  4. Press Shift+Enter and start to typing

Actual result:

All styles re-appear in the editor.

Expected:

What should be expected result in such a situation? Should be unified the behaviour of the enter and the shiftenter command in relation to text attributes?

Jul-12-2019 11-14-07


@Reinmar, @mlewand maybe you have some thoughts about this case?

jodator referenced this issue in ckeditor/ckeditor5-enter Jul 16, 2019
Feature: Add support for `copyOnEnter` attribute's parameter. Closes #40.
@mlewand mlewand transferred this issue from ckeditor/ckeditor5-enter Oct 9, 2019
@mlewand mlewand added this to the iteration 26 milestone Oct 9, 2019
@mlewand mlewand added status:confirmed type:bug This issue reports a buggy (incorrect) behavior. package:enter labels Oct 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package:enter type:bug This issue reports a buggy (incorrect) behavior.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants