Skip to content
This repository has been archived by the owner on Jun 26, 2020. It is now read-only.

Introduced .isBefore() and .isAfter() in model.Node and view.Node #1368

Merged
merged 7 commits into from
Mar 21, 2018

Conversation

scofalik
Copy link
Contributor

@scofalik scofalik commented Mar 19, 2018

Suggested merge commit message (convention)

Feature: Introduced #isBefore and #isAfter in model.Node and view.Node. Additionally, model.Node#is and view.Node#is and view.Node#getPath were added. Closes ckeditor/ckeditor5#4318.

@coveralls
Copy link

coveralls commented Mar 19, 2018

Coverage Status

Coverage remained the same at 100.0% when pulling a163b72 on t/1365 into f8dec1e on master.

src/view/node.js Outdated
/**
* Clones this node.
*
* @method #clone
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be protected.

}

// In other cases, just check if the `node` is before, and return the opposite.
return !this.isBefore( node );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clever :)

}
// Get path from root to position's parent element. "Indexify" the paths (change elements to indices).
const thisPath = this.getAncestors().map( element => element.index );
const otherPath = otherPosition.getAncestors().map( element => element.index );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WDYT about using just introduced Node#getPath()?

const thisPath = this.parent.is( 'node' ) ? this.parent.getPath() : [];
const otherPath = otherPosition.parent.is( 'node' ) ? otherPosition.parent.getPath() : []

and shifting null from array won't be necessary.

@oskarwrobel
Copy link
Contributor

I left 2 small comments but works and looks fine in general 👍

@scofalik
Copy link
Contributor Author

Fixed according to comments.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants