Renamed VirtualSelection to Highlight. #1091
Conversation
| * @return {Function} | ||
| */ | ||
| export function convertElementsInsideMarker( selectionDescriptor ) { | ||
| export function convertElementsInsideMarker( highlightDescriptor ) { |
There was a problem hiding this comment.
Since most of the methods names here focus on what will be done on the view and highlight works well as a verb, maybe a better name would be highlightElement and highlightText?
| @@ -475,9 +475,9 @@ export function convertElementsInsideMarker( selectionDescriptor ) { | |||
|
|
|||
| const viewElement = conversionApi.mapper.toViewElement( modelItem ); | |||
| const addMarker = evt.name.split( ':' )[ 0 ] == 'addMarker'; | |||
| const selectionHandlingMethod = addMarker ? 'setVirtualSelection' : 'removeVirtualSelection'; | |||
| const highlightHandlingMethod = addMarker ? 'setHighlight' : 'removeHighlight'; | |||
There was a problem hiding this comment.
I'm not sure about setHighlight. First sounds strange that it's setHighlight instead of just highlight. Second, if we have remove the better pair might be add, especially when you can add multiple highlights. So I would go with addHighlight instead of setHighlight.
There was a problem hiding this comment.
Especially, when its default action is stack.add and stack.remove.
|
Please do remember that these changes are internal, not breaking, because this whole API has appeared in this milestone. |
|
@Reinmar - true, I'll remove breaking changes from suggested merge commit message. |
…lightElements and convertTextsInsideMarker to highlightTexts.
| convertTextsInsideMarker, | ||
| convertElementsInsideMarker | ||
| highlightTexts, | ||
| highlightElements |
There was a problem hiding this comment.
All other methods here are singular (wrapItem, removeUIElement). I think this should be too.
Suggested merge commit message (convention)
Other: Renamed virtual selection to highlight. Closes ckeditor/ckeditor5#4151.
Additional information
Depending branch in widget repository: https://github.com/ckeditor/ckeditor5-widget/tree/t/ckeditor5-engine/1085