Introduced model.isEmpty() method. DataController.get() method can now trim empty data
#1656
Conversation
src/controller/datacontroller.js
Outdated
| @@ -145,14 +154,21 @@ export default class DataController { | |||
| * | |||
| * @param {module:engine/model/element~Element|module:engine/model/documentfragment~DocumentFragment} modelElementOrFragment | |||
| * Element whose content will be stringified. | |||
| * @param {Boolean} [skipEmpty=false] Whether content considered empty should be skipped. The method | |||
There was a problem hiding this comment.
I think that this logic should stay in get(). Less API changes is better.
| * @returns {String} Output data. | ||
| */ | ||
| get( rootName = 'main' ) { | ||
| get( options ) { |
There was a problem hiding this comment.
This can be done with destructuring.
src/model/model.js
Outdated
| * {@link module:engine/model/element~Element element} is considered empty. | ||
| * | ||
| * The range or element is considered non-empty if it contains any: | ||
| * * EmptyElement |
There was a problem hiding this comment.
Can a model range contain a view empty element? ;)
src/model/model.js
Outdated
| * @param {module:engine/model/range~Range|module:engine/model/element~Element} rangeOrElement Range or element to check. | ||
| * @returns {Boolean} | ||
| */ | ||
| isEmpty( rangeOrElement ) { |
There was a problem hiding this comment.
The relation between isEmpty() and hasContent() needs to be clarified. Both these methods are defined on Model but they have different semantics. I wonder if we shouldn't rename hasContent() or isEmpty() to somehow differentiate between these two.
cc @pjasiun
There was a problem hiding this comment.
Let's merge isEmpty() into hasContent() (i.e. we will have a single method for that called hasContent()).
Right now there are 2 differences between them – check for markers and /\S+/. The former can be done automatically by hasContent(). The latter could be enabled by an option.trimWhitespaces.
src/controller/datacontroller.js
Outdated
| // Get model range. | ||
| return this.stringify( this.model.document.getRoot( rootName ) ); | ||
| return trim === 'empty' && !this.model.hasContent( root, { trimWhitespaces: true } ) ? '' : this.stringify( root ); |
|
I think the probelm in here is that there are no samples how to implement this in my project ClassicEditor.model returns undefined |
Suggested merge commit message (convention)
Feature: Introduced
model.isEmpty()method.DataController.get()method can now trim empty data. Closes ckeditor/ckeditor5#401.BREAKING CHANGE:
DataController.get()method now returns an empty string by default when editor content is empty.Additional information
See ckeditor/ckeditor5#401. I have also created https://github.com/ckeditor/ckeditor5/tree/t/401 branch for easier testing.
There are a few related PRs which should be closed together with this one:
getData()method now acceptsoptionsparameter ckeditor5-core#161getData()ckeditor5-block-quote#32getData()ckeditor5-enter#61getData()ckeditor5-paragraph#41getData()ckeditor5-table#169getData()ckeditor5-typing#180