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

Tests: Create utility functions for testing view tree. #3669

Closed
szymonkups opened this issue Apr 18, 2016 · 4 comments
Closed

Tests: Create utility functions for testing view tree. #3669

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

Comments

@szymonkups
Copy link
Contributor

This is a part of #3626.
We need to create utility functions that will allow to compare view data and selection to expected value, and set view data and selection to given value.

@szymonkups szymonkups self-assigned this Apr 18, 2016
@szymonkups
Copy link
Contributor Author

I'm almost finished with the tools but have problem defining final API that will suit all our needs, but still be easy to use.

getData()

getData( node [, options ] )

Returns HTML-like string representing TreeView begining from node.

getData( element );  // <a>foobar</a>
getData( node, selection [, options ] )

Returns HTML-like string representing TreeView begining from node and also includes ranges from provided Selection instance object.

getData( element, selection ) // <a>{foobar}</a>

Options object can include two boolean flags: showType and showPriority.
When showType will be set to true, AttributeElements and ContainerElements will be printed in a form of <attribute:a> and <container:div>.
When showPriority will be set to true, AttributeElements priority will be printed: <a:12>.

setData()

setData( data )

Parses provided data and returns nodes with all parsed elements.

setData( 'foobar'); // Returns Text node.
setData( '<a>foobar</a>' ); // Returns Element with text node inside.
setData( '<container:div></container:div>' ); // Returns ContainerElement.
setData( '<a:12></a:12>' ); // Returns AttributeElement that has priority set to `12`.
setData( '<a></a><b></b>' ); // Returns DocumentFragment that contains two nodes.

Selection ranges can be included. For example:

setData( '<a>{foobar}</a>', selection );

All ranges inside selection will be replaced by ranges that are set inside provided data.
Order of the ranges inside selection can be included. For example:

setData( '<a>{foo}{bar}</a>' ), selection, '21' );

All ranges inside selection will be replaces by ranges in desired order (first range placed as second, second as first).

Questions

  1. Should we include some other ways of getting the view data? For exampe: getData( treeView, rootName, selection [, options] ) - will return HTML-like string that include nodes from TreeView instance starting from given rootName and include provided selection ranges.
  2. Should setData also include a parameter that will allow to set selection order, something like lastRangeBackward?
  3. Should we include other ways of setting view data?
    setData( data, treeView, rootName, selection ...) - list of parameters are getting pretty long, maybe it is better to use options object or create separate method for it?
  4. Maybe you see a different way of defining view tools API. Please share.

@pjasiun
Copy link

pjasiun commented Apr 25, 2016

I think we need 2 levels of the API. One is set of tools to change view to String and back, and one to set editors view data.

Methods you described look like the first level. I am not sure if they should be called setData and getData. They are more like stringify and parse.

setData( '<a>{foobar}</a>', selection );

I do not like passing selection as a second argument to set it. We will need view.selection.setTo( otherSelection ); anyway, so I think that if the string contains selection setData/parse should return object with selection and data properties, which could be easily destructured.

setData( '<a>{foo}{bar}</a>' ), selection, '21' );

The last parameter should be an options object. It could handle also lastRangeBackward in the future and it would be more readable (when you see { order: [ 2, 1 ], lastRangeBackward: true } your know more them when you see '21', true. Also order should be an array. 21 looks like a magic number.

Then second level of the API works with the view or even with the editor. It could even be defined in core repository since engine knows nothing about the editor.

getData( treeView, rootName, selection [, options] )
setData( data, treeView, rootName, selection ...)

I think that it should be like:

getData( forWhere [, options] );
setData( data, where[, options] );

where|fromWhere parameter could be:

  • { element: root, selection: selection } - for the most custom usage, on the other hand you can use parse|stringify for such cases,
  • treeview - then the selection and the main root is taken from that treeview, in most cases we will have only one root.
  • editor - then edior.editing.view is taken.

@Reinmar
Copy link
Member

Reinmar commented Apr 25, 2016

I mostly agree with @pjasiun proposals. Just two notes:

  1. The split between core and engine API must be done in a way that we won't miss some methods when working with the engine. So as few code as possible should be kept in the core.
  2. We need to publish those methods in some accessible, global place. We were thinking about bender or console.cke globals. Let's try with bender as it's shorter. The way how to do that would be to create a module which expose these functions plus another one which imports them and exposes in that global value. The latter should always be loaded, so it needs to be done e.g. in https://github.com/ckeditor/ckeditor5/blob/master/dev/tasks/build/utils.js#L34 (if that template grows too much we should move it to a separate file).

@Reinmar
Copy link
Member

Reinmar commented Apr 29, 2016

Fixed in ckeditor/ckeditor5-engine#381.

@Reinmar Reinmar closed this as completed Apr 29, 2016
@mlewand mlewand transferred this issue from ckeditor/ckeditor5-engine Oct 9, 2019
@mlewand mlewand added this to the v0.1.0 milestone Oct 9, 2019
@mlewand mlewand added type:feature This issue reports a feature request (an idea for a new functionality or a missing option). package:engine labels Oct 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package:engine 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

4 participants