getNearestSelectionPosition changes #717
Conversation
… to getNearestSelectionRange. Added tests and required fixes to other methods using this.
| selection.setRanges( insertion.getSelectionRanges() ); | ||
| const newRange = insertion.getSelectionRange(); | ||
|
|
||
| /* istanbul ignore else */ |
There was a problem hiding this comment.
I'd like to have every case like this commented. We're not testing else because it's a safe check for some unpredictable edge cases. Also, having a log.warning() would be good there, like in the other cases in this method.
| * * `forward` - searching will be performed only forward, | ||
| * * `backward` - searching will be performed only backward. | ||
| * | ||
| * When valid selection range cannot be found, `null` value is returned. |
| if ( this.schema.check( { name: '$text', inside: step.value.nextPosition } ) ) { | ||
| return step.value.nextPosition; | ||
| } | ||
| for ( let data of getWalkersData( backwardWalker, forwardWalker ) ) { |
There was a problem hiding this comment.
Walker has no data. Walker walks ;). Maybe combineWalkers()?
|
Added requested fixes. |
|
|
||
| test( | ||
| 'should return null if there is no valid range', | ||
| '<image></image>', |
There was a problem hiding this comment.
This case is a bit strange. Isn't this pattern missing initial selection? And if it does, shouldn't the image be selected?
There was a problem hiding this comment.
OK... Image is not an object in this test suite. This is confusing. Could we name it like... damn, I can't find a name, because this case shouldn't really occur :D. Perhaps we can call it <emptyBlock> then?
There was a problem hiding this comment.
This case should be changed indeed. Initial selection should be added.
|
Added requested changes. |
|
There's a warning logged by the tests: See https://travis-ci.org/ckeditor/ckeditor5-engine/builds/183855169 If it's expected that it's logged, it should be stubbed. If not, then it means that either we don't need that istanbul comment or that something works incorrectly. |
Fixes ckeditor/ckeditor5#3907.