Implement isText() helper #218
Conversation
| * @returns {Boolean} | ||
| */ | ||
| export default function isText( obj ) { | ||
| return Object.prototype.toString.call( obj ) == '[object Text]'; |
There was a problem hiding this comment.
Not sure if it's better checking nodeType or calling toString(). IMO both options should be good here.
There was a problem hiding this comment.
Checking nodeType seems to be a too naive duck typing. Calling toString() is slightly safer. Eventually, we could switch to instanceof but starting with toString() seems to be simpler.
There was a problem hiding this comment.
Simple instanceof check easily breaks in iframes, so it would be a little more complicated to implement it correctly.
There was a problem hiding this comment.
Yep, I know. I meant instanceof check that we used in isDomNode().
There was a problem hiding this comment.
BTW: I wonder why isDomNode instead of isNode? It's under src/dom/. If we go with isText we should go with isNode too.
There was a problem hiding this comment.
I forgot to comment on this – yep, isNode() would be better. @ma2ciek could you report a separate ticket for it and do the change?
There was a problem hiding this comment.
Sure, I'll handle it.
There was a problem hiding this comment.
Suggested merge commit message (convention)
Other: Introduced
isText()helper. Closes ckeditor/ckeditor5#4989.Additional information
PRs in other packages: