Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added handling of editable focus on beforeinput event. #16267

Merged
merged 2 commits into from
Apr 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
85 changes: 55 additions & 30 deletions packages/ckeditor5-engine/src/view/observer/focusobserver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import DomEventObserver from './domeventobserver.js';
import type DomEventData from './domeventdata.js';
import type View from '../view.js';
import type { ViewDocumentInputEvent } from './inputobserver.js';

/**
* {@link module:engine/view/document~Document#event:focus Focus}
Expand Down Expand Up @@ -48,35 +49,14 @@ export default class FocusObserver extends DomEventObserver<'focus' | 'blur'> {
this.useCapture = true;
const document = this.document;

document.on<ViewDocumentFocusEvent>( 'focus', () => {
this._isFocusChanging = true;

// Unfortunately native `selectionchange` event is fired asynchronously.
// We need to wait until `SelectionObserver` handle the event and then render. Otherwise rendering will
// 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.
//
// Using `view.change()` instead of `view.forceRender()` to prevent double rendering
// in a situation where `selectionchange` already caused selection change.
this._renderTimeoutId = setTimeout( () => {
this.flush();
view.change( () => {} );
}, 50 );
} );

document.on<ViewDocumentBlurEvent>( 'blur', ( evt, data ) => {
const selectedEditable = document.selection.editableElement;

if ( selectedEditable === null || selectedEditable === data.target ) {
document.isFocused = false;
this._isFocusChanging = false;

// Re-render the document to update view elements
// (changing document.isFocused already marked view as changed since last rendering).
view.change( () => {} );
document.on<ViewDocumentFocusEvent>( 'focus', () => this._handleFocus() );
document.on<ViewDocumentBlurEvent>( 'blur', ( evt, data ) => this._handleBlur( data ) );

document.on<ViewDocumentInputEvent>( 'beforeinput', () => {
if ( !document.isFocused ) {
this._handleFocus();
}
} );
}, { priority: 'highest' } );
}

/**
Expand All @@ -100,11 +80,56 @@ export default class FocusObserver extends DomEventObserver<'focus' | 'blur'> {
* @inheritDoc
*/
public override destroy(): void {
this._clearTimeout();
super.destroy();
}

/**
* The `focus` event handler.
*/
private _handleFocus(): void {
this._clearTimeout();
this._isFocusChanging = true;

// Unfortunately native `selectionchange` event is fired asynchronously.
// We need to wait until `SelectionObserver` handle the event and then render. Otherwise rendering will
// 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.
//
// Using `view.change()` instead of `view.forceRender()` to prevent double rendering
// in a situation where `selectionchange` already caused selection change.
this._renderTimeoutId = setTimeout( () => {
this._renderTimeoutId = 0;
this.flush();
this.view.change( () => {} );
}, 50 );
}

/**
* The `blur` event handler.
*/
private _handleBlur( data: DomEventData<FocusEvent> ): void {
const selectedEditable = this.document.selection.editableElement;

if ( selectedEditable === null || selectedEditable === data.target ) {
this.document.isFocused = false;
this._isFocusChanging = false;

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

/**
* Clears timeout.
*/
private _clearTimeout(): void {
if ( this._renderTimeoutId ) {
clearTimeout( this._renderTimeoutId );
this._renderTimeoutId = 0;
}

super.destroy();
}
}

Expand Down
40 changes: 40 additions & 0 deletions packages/ckeditor5-engine/tests/view/observer/focusobserver.js
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,46 @@ describe( 'FocusObserver', () => {

expect( viewDocument.isFocused ).to.be.false;
} );

it( 'should set isFocused to true on beforeinput after 50ms', () => {
expect( viewDocument.isFocused ).to.be.false;

observer.onDomEvent( { type: 'beforeinput', target: domMain } );
expect( viewDocument.isFocused ).to.be.false;

clock.tick( 50 );
expect( viewDocument.isFocused ).to.be.true;
} );

it( 'should set isFocused to true on beforeinput after flush', () => {
expect( viewDocument.isFocused ).to.be.false;

observer.onDomEvent( { type: 'beforeinput', target: domMain } );
expect( viewDocument.isFocused ).to.be.false;

observer.flush();
expect( viewDocument.isFocused ).to.be.true;
} );

it( 'should not set isFocused to true on beforeinput on other element after 50ms', () => {
expect( viewDocument.isFocused ).to.be.false;

observer.onDomEvent( { type: 'beforeinput', target: document } );
expect( viewDocument.isFocused ).to.be.false;

clock.tick( 50 );
expect( viewDocument.isFocused ).to.be.true;
} );

it( 'should not set isFocused to true on beforeinput on focused document after 50ms', () => {
viewDocument.isFocused = true;

observer.onDomEvent( { type: 'beforeinput', target: document } );
expect( viewDocument.isFocused ).to.be.true;

clock.tick( 50 );
expect( viewDocument.isFocused ).to.be.true;
} );
} );

describe( 'handle _isFocusChanging property of the document', () => {
Expand Down