Skip to content

Commit

Permalink
Merge pull request #9889 from ckeditor/ck/9513-do-not-render-selectio…
Browse files Browse the repository at this point in the history
…n-while-selecting

Fix (engine): Markers should not be split in view on the caret position. Closes #9513.

Fix (engine): `FocusObserver` should not force the view to render in random moments. See #9513.
  • Loading branch information
scofalik committed Jun 17, 2021
2 parents e45b27f + 8e096a7 commit be066b4
Show file tree
Hide file tree
Showing 8 changed files with 207 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -341,6 +341,8 @@ export default class DowncastDispatcher {
this.fire( 'selection', { selection }, this.conversionApi );

if ( !selection.isCollapsed ) {
this._clearConversionApi();

return;
}

Expand Down Expand Up @@ -414,6 +416,8 @@ export default class DowncastDispatcher {
// Do not fire events for each item inside the range if the range got consumed.
//
if ( !consumable.test( markerRange, eventName ) ) {
this._clearConversionApi();

return;
}

Expand Down
5 changes: 5 additions & 0 deletions packages/ckeditor5-engine/src/conversion/modelconsumable.js
Original file line number Diff line number Diff line change
Expand Up @@ -320,5 +320,10 @@ export default class ModelConsumable {
function _normalizeConsumableType( type ) {
const parts = type.split( ':' );

// Markers are identified by the whole name (otherwise we would consume the whole markers group).
if ( parts[ 0 ] == 'addMarker' || parts[ 0 ] == 'removeMarker' ) {
return type;
}

return parts.length > 1 ? parts[ 0 ] + ':' + parts[ 1 ] : parts[ 0 ];
}
10 changes: 7 additions & 3 deletions packages/ckeditor5-engine/src/view/observer/focusobserver.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,10 @@ export default class FocusObserver extends DomEventObserver {
// overwrite new DOM selection with selection from the view.
// See https://github.com/ckeditor/ckeditor5-engine/issues/795 for more details.
// Long timeout is needed to solve #676 and https://github.com/ckeditor/ckeditor5-engine/issues/1157 issues.
this._renderTimeoutId = setTimeout( () => view.forceRender(), 50 );
//
// Using `view.change()` instead of `view.forceRender()` to prevent double rendering
// in a situation where `selectionchange` already caused selection change.
this._renderTimeoutId = setTimeout( () => view.change( () => {} ), 50 );
} );

document.on( 'blur', ( evt, data ) => {
Expand All @@ -46,8 +49,9 @@ export default class FocusObserver extends DomEventObserver {
if ( selectedEditable === null || selectedEditable === data.target ) {
document.isFocused = false;

// Re-render the document to update view elements.
view.forceRender();
// Re-render the document to update view elements
// (changing document.isFocused already marked view as changed since last rendering).
view.change( () => {} );
}
} );

Expand Down
5 changes: 5 additions & 0 deletions packages/ckeditor5-engine/src/view/view.js
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,11 @@ export default class View {
this.listenTo( this.document.selection, 'change', () => {
this._hasChangedSinceTheLastRendering = true;
} );

// Trigger re-render if only the focus changed.
this.listenTo( this.document, 'change:isFocused', () => {
this._hasChangedSinceTheLastRendering = true;
} );
}

/**
Expand Down
Loading

0 comments on commit be066b4

Please sign in to comment.