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

Acknowledge inline elements #1096

Closed
Reinmar opened this issue Jun 20, 2018 · 5 comments
Closed

Acknowledge inline elements #1096

Reinmar opened this issue Jun 20, 2018 · 5 comments
Assignees
Labels
package:engine package:typing type:bug This issue reports a buggy (incorrect) behavior.
Milestone

Comments

@Reinmar
Copy link
Member

Reinmar commented Jun 20, 2018

So far we didn't consider inline elements in the model. Blocks could only contain text. It changed since we introduces support for line breaks (<br>s).

It caused a couple of bugs. Most notable ones are:

So far we managed to resolve #1024 in a quite clear way. #1083 was fixed in an ugly way, though (@scofalik explained that in ckeditor/ckeditor5-typing#156) and works with <softBreak> elements only. We need a more general and bulletproof solution.

PS. There's a chance that the issues which one might experience with inline widgets are related to this – we don't handle any other node than text in ckeditor5-typing.

@scofalik
Copy link
Contributor

scofalik commented Sep 5, 2018

The documentation for ContainerElement is a bit misleading at this moment. It says:

 * There might be a need to mark `<span>` element as a container node, for example in situation when it will be a
 * container of an inline widget:
 *
 *		<span color="red">foobar</span>		// attribute
 *		<span data-widget>foobar</span>		// container

But it doesn't mention any other additional steps, so it looks like all you need to do is create ContainerElement with span name. Which is not true -- that span will behave like any other ContainerElement. For starters, you might need to overwrite this.getFillerOffset property so an empty span won't get a bogus br. But I don't know if it will get a ZWS then? 🤔

@jodator
Copy link
Contributor

jodator commented Jan 24, 2019

Some issues that I've found so far:

  • the &nbsp handling - they're added around widget
  • the ZWS (probably?) added if inline widget is at the end of block
    selection_170
  • copy&paste: you cannot copy selected IW bug or missing Clipboard
  • selection with mouse: click after the IW, expand selection over the widget to the beginning of a text -> error model-nodelist-offset-out-of-bounds
  • pasting selection with IW inside allows the IW to inserted directly in root (is not wrapped by paragraph) I cannot reproduce - maybe bug
  • DOM selection is rendered over IW - looks better if disabled in CSS:
    placeholder::selection{
    	display: none;
    }
  • findOptimalPositon() is buggy: I've selected IW and inserted table> the header was split
  • The ctrl+x to work when selection contain IW bug in sample
  • ctrl + behavior needs to be reviewed/fixed - right now it omits inline widget if surrounded by spaces but acknowledges it if it is inside a word:

peek 2019-01-25 17-20

What's work OK:

  • caret navigation around IW
  • inline styles: is disabled on IW, the attribute breaks before and after IW
  • works similarly inside table as in root:
    selection_171

@jodator
Copy link
Contributor

jodator commented Jan 29, 2019

After F2F talks we concluded that in order to make inline widgets usable we need to:

  1. Introduce the isInline schema object type that will allow to mark elements to be treated as text.
    • softBreak and $text will be marked as isInline also.
  2. the view to model selection conversion must be fixed because DOM selection on Chrome is wrongly mapped to the model (model-nodelist-offset-out-of-bounds error).
  3. Check why the fake selection is not rendered for inline widgets

@Reinmar
Copy link
Member Author

Reinmar commented Jan 30, 2019

2. the view to model selection conversion must be fixed because DOM selection on Chrome is wrongly mapped to the model (model-nodelist-offset-out-of-bounds error).

Step 1.

How to convert this view selection:

<p>Foo<widget>bar[</widget>x]</p>

To this model selection:

<paragraph>Foo<widget name=bar>[</widget>x]</paragraph>

I think we can use editing.mapper.on( 'viewToModelPosition' ) to control this now, but it’d be good if it was somehow automated.

Step 2.

We need to fix this model selection:

<paragraph>Foo<widget name=bar>[</widget>x]</paragraph>

Into:

<paragraph>Foo[<widget name=bar></widget>x]</paragraph>

or:

<paragraph>Foo<widget name=bar></widget>[x]</paragraph>

This may happen already (selection post-fixer).

Reinmar added a commit to ckeditor/ckeditor5-widget that referenced this issue Feb 18, 2019
Other: Introduce support and utils for creating inline widgets. Closes [ckeditor/ckeditor5#1096](ckeditor/ckeditor5#1096).
@xinglie
Copy link

xinglie commented Nov 19, 2020

The documentation for ContainerElement is a bit misleading at this moment. It says:

 * There might be a need to mark `<span>` element as a container node, for example in situation when it will be a
 * container of an inline widget:
 *
 *		<span color="red">foobar</span>		// attribute
 *		<span data-widget>foobar</span>		// container

But it doesn't mention any other additional steps, so it looks like all you need to do is create ContainerElement with span name. Which is not true -- that span will behave like any other ContainerElement. For starters, you might need to overwrite this.getFillerOffset property so an empty span won't get a bogus br. But I don't know if it will get a ZWS then? 🤔

Yes , i wrote a plugin use span by createContainerElement method .
when user delete all text in the span , the CKE will put a <br data-cke-filler="true"> in the span, and break lines .
so how to avoid this behave

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package:engine package:typing type:bug This issue reports a buggy (incorrect) behavior.
Projects
None yet
Development

No branches or pull requests

4 participants