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

UI lazy init #13066

Merged
merged 34 commits into from
Jan 9, 2023
Merged
Show file tree
Hide file tree
Changes from 33 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
40bc7c7
Reference UI init time performance test.
niegowski Nov 17, 2022
04af907
Testing UI init performance with lazy init non-visible views.
niegowski Nov 17, 2022
c306c5b
Testing UI init performance with lazy init non-visible views.
niegowski Nov 17, 2022
65ecbde
Widget toolbars should be rendered only on first show.
niegowski Nov 18, 2022
f1a1564
Merge branch 'master' into ck/12682-performance-lazy-init
niegowski Dec 7, 2022
b456696
Merge branch 'master' into ck/12682-performance-lazy-init
niegowski Dec 12, 2022
707fe75
Merge branch 'master' into ck/12682-performance-lazy-init
niegowski Dec 13, 2022
55d77f1
ContextualBalloon lazy view initialization.
niegowski Dec 13, 2022
8efeb77
Simplified dropdown panel initialization.
niegowski Dec 13, 2022
06ecfa4
Lazy init of toolbars in dropdowns.
niegowski Dec 13, 2022
d794c3d
Lazy init of lists in dropdowns.
niegowski Dec 13, 2022
137c0c7
Fixed nested toolbar lazy init.
niegowski Dec 13, 2022
48dcf69
Merge branch 'master' into ck/12682-performance-lazy-init
niegowski Dec 15, 2022
bdad3ee
Merge branch 'master' into ck/12682-performance-lazy-init
niegowski Dec 22, 2022
abd8811
Code updated to upstream changes.
niegowski Dec 22, 2022
2e601c4
Rollback changes from _src files.
niegowski Dec 22, 2022
5915a0a
Disabled toolbar auto grouping.
niegowski Dec 22, 2022
3db6c73
Code cleanup.
niegowski Dec 22, 2022
f2b70d2
Updated docs.
niegowski Dec 22, 2022
4231288
Reverted changes in all-features manual test.
niegowski Dec 22, 2022
4bc1578
Adding tests.
niegowski Dec 28, 2022
37545ac
Adding tests.
niegowski Dec 29, 2022
abc29f5
Adding tests.
niegowski Dec 29, 2022
3e7ca68
Adding tests.
niegowski Dec 30, 2022
fc9f550
Adding tests.
niegowski Jan 3, 2023
5b866ab
Adding tests.
niegowski Jan 4, 2023
abe9d86
Adding tests.
niegowski Jan 4, 2023
65c817e
Updated tests.
niegowski Jan 4, 2023
43cee2e
Merge branch 'master' into ck/12682-performance-lazy-init
niegowski Jan 4, 2023
5bfd47d
Added changes after review.
niegowski Jan 5, 2023
1bd3d7b
Fixed listening to UI update event by table cell properties.
niegowski Jan 5, 2023
e05956f
Added tests.
niegowski Jan 9, 2023
276b4eb
Merge branch 'master' into ck/12682-performance-lazy-init
niegowski Jan 9, 2023
3a9b1f4
Fixed issue with chained observables.
niegowski Jan 9, 2023
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
30 changes: 12 additions & 18 deletions packages/ckeditor5-alignment/src/alignmentui.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,18 +76,22 @@ export default class AlignmentUI extends Plugin {
const dropdownView = createDropdown( locale );

// Add existing alignment buttons to dropdown's toolbar.
const buttons = options.map( option => componentFactory.create( `alignment:${ option.name }` ) ) as Array<ButtonView>;
addToolbarToDropdown( dropdownView, buttons, { enableActiveItemFocusOnDropdownOpen: true } );
addToolbarToDropdown(
dropdownView,
() => options.map( option => componentFactory.create( `alignment:${ option.name }` ) ) as Array<ButtonView>,
{
enableActiveItemFocusOnDropdownOpen: true,
isVertical: true,
ariaLabel: t( 'Text alignment toolbar' )
}
);

// Configure dropdown properties an behavior.
dropdownView.buttonView.set( {
label: t( 'Text alignment' ),
tooltip: true
} );

dropdownView.toolbarView!.isVertical = true;
dropdownView.toolbarView!.ariaLabel = t( 'Text alignment toolbar' );

dropdownView.extendTemplate( {
attributes: {
class: 'ck-alignment-dropdown'
Expand All @@ -96,23 +100,13 @@ export default class AlignmentUI extends Plugin {

// The default icon depends on the direction of the content.
const defaultIcon = locale.contentLanguageDirection === 'rtl' ? iconsMap.get( 'right' ) : iconsMap.get( 'left' );
const command = editor.commands.get( 'alignment' )!;

// Change icon to reflect current selection's alignment.
dropdownView.buttonView.bind( 'icon' ).toMany( buttons, 'isOn', ( ...areActive ) => {
// Get the index of an active button.
const index = areActive.findIndex( value => value );

// If none of the commands is active, display either defaultIcon or the first button's icon.
if ( index < 0 ) {
return defaultIcon;
}

// Return active button's icon.
return buttons[ index ].icon;
} );
dropdownView.buttonView.bind( 'icon' ).to( command, 'value', value => iconsMap.get( value ) || defaultIcon );

// Enable button if any of the buttons is enabled.
dropdownView.bind( 'isEnabled' ).toMany( buttons, 'isEnabled', ( ...areEnabled ) => areEnabled.some( isEnabled => isEnabled ) );
dropdownView.bind( 'isEnabled' ).to( command, 'isEnabled' );

// Focus the editable after executing the command.
// Overrides a default behaviour where the focus is moved to the dropdown button (#12125).
Expand Down
66 changes: 66 additions & 0 deletions packages/ckeditor5-alignment/tests/alignmentui.js
Original file line number Diff line number Diff line change
Expand Up @@ -242,13 +242,25 @@ describe( 'Alignment UI', () => {
} );

it( '#toolbarView has the basic properties', () => {
// Make sure that toolbar view is not created before first dropdown open.
expect( dropdown.toolbarView ).to.be.undefined;

// Trigger toolbar view creation (lazy init).
dropdown.isOpen = true;

const toolbarView = dropdown.toolbarView;

expect( toolbarView ).to.have.property( 'isVertical', true );
expect( toolbarView ).to.have.property( 'ariaLabel', 'Text alignment toolbar' );
} );

it( 'should hold defined buttons', () => {
// Make sure that toolbar view is not created before first dropdown open.
expect( dropdown.toolbarView ).to.be.undefined;

// Trigger toolbar view creation (lazy init).
dropdown.isOpen = true;

const items = [ ...dropdown.toolbarView.items ].map( item => item.label );

expect( items ).to.have.length( 4 );
Expand All @@ -259,7 +271,45 @@ describe( 'Alignment UI', () => {
expect( items.includes( 'Justify' ) ).to.be.true;
} );

it( 'should use icon related to current command value', () => {
// Make sure that toolbar view is not created before first dropdown open.
expect( dropdown.toolbarView ).to.be.undefined;

// Trigger toolbar view creation (lazy init).
dropdown.isOpen = true;

expect( dropdown.buttonView.icon ).to.equal( alignLeftIcon );

command.value = 'right';

expect( dropdown.buttonView.icon ).to.equal( alignRightIcon );
} );

it( 'should be disabled if command is not enabled', () => {
// Make sure that toolbar view is not created before first dropdown open.
expect( dropdown.toolbarView ).to.be.undefined;

// Trigger toolbar view creation (lazy init).
dropdown.isOpen = true;

command.isEnabled = true;
expect( dropdown.isEnabled ).to.be.true;

command.isEnabled = false;
expect( dropdown.isEnabled ).to.be.false;
} );

it( 'should focus the first active button when dropdown is opened', () => {
dropdown.render();
document.body.appendChild( dropdown.element );

// Make sure that toolbar view is not created before first dropdown open.
expect( dropdown.toolbarView ).to.be.undefined;

// Trigger toolbar view creation (lazy init).
dropdown.isOpen = true;
dropdown.isOpen = false;

const buttonAlignLeft = dropdown.toolbarView.items.get( 0 );
const buttonAlignRight = dropdown.toolbarView.items.get( 1 );
const spy = sinon.spy( buttonAlignRight, 'focus' );
Expand All @@ -268,9 +318,17 @@ describe( 'Alignment UI', () => {
buttonAlignRight.isOn = true;
dropdown.isOpen = true;
sinon.assert.calledOnce( spy );

dropdown.element.remove();
} );

it( 'should return focus to editable after executing a command', () => {
// Make sure that toolbar view is not created before first dropdown open.
expect( dropdown.toolbarView ).to.be.undefined;

// Trigger toolbar view creation (lazy init).
dropdown.isOpen = true;

const buttonAlignLeft = dropdown.toolbarView.items.get( 0 );
const spy = sinon.spy( editor.editing.view, 'focus' );
dropdown.render();
Expand Down Expand Up @@ -302,6 +360,12 @@ describe( 'Alignment UI', () => {
} );

it( 'should hold only defined buttons', () => {
// Make sure that toolbar view is not created before first dropdown open.
expect( dropdown.toolbarView ).to.be.undefined;

// Trigger toolbar view creation (lazy init).
dropdown.isOpen = true;

const items = [ ...dropdown.toolbarView.items ].map( item => item.label );

expect( items ).to.have.length( 2 );
Expand All @@ -311,6 +375,7 @@ describe( 'Alignment UI', () => {
} );

it( 'should have default icon set (LTR content)', () => {
command.value = undefined;
expect( dropdown.buttonView.icon ).to.equal( alignLeftIcon );
} );

Expand All @@ -328,6 +393,7 @@ describe( 'Alignment UI', () => {
} )
.then( newEditor => {
dropdown = newEditor.ui.componentFactory.create( 'alignment' );
editor.commands.get( 'alignment' ).value = undefined;

expect( dropdown.buttonView.icon ).to.equal( alignRightIcon );

Expand Down
2 changes: 1 addition & 1 deletion packages/ckeditor5-code-block/src/codeblockui.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ export default class CodeBlockUI extends Plugin {
dropdownView.class = 'ck-code-block-dropdown';
dropdownView.bind( 'isEnabled' ).to( command );

addListToDropdown( dropdownView, this._getLanguageListItemDefinitions( normalizedLanguageDefs ) );
addListToDropdown( dropdownView, () => this._getLanguageListItemDefinitions( normalizedLanguageDefs ) );

return dropdownView;
} );
Expand Down
15 changes: 15 additions & 0 deletions packages/ckeditor5-code-block/tests/codeblock-integration.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
* For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license
*/

/* global document */

import ClassicTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/classictesteditor';
import Enter from '@ckeditor/ckeditor5-enter/src/enter';
import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph';
Expand Down Expand Up @@ -133,6 +135,17 @@ describe( 'CodeBlock - integration', () => {
it( 'should create a second code block with the same language as the first one', () => {
const dropdown = editor.ui.componentFactory.create( 'codeBlock' );
const codeBlock = dropdown.buttonView;

dropdown.render();
document.body.appendChild( dropdown.element );

// Make sure that toolbar view is not created before first dropdown open.
expect( dropdown.listView ).to.be.undefined;

// Trigger list view creation (lazy init).
dropdown.isOpen = true;
dropdown.isOpen = false;

const listView = dropdown.panelView.children.first;
const cSharpButton = listView.items.get( 2 ).children.first;

Expand All @@ -153,6 +166,8 @@ describe( 'CodeBlock - integration', () => {
// Clicking the button once again should create the code block with the C# language instead of the default (plaintext).
codeBlock.fire( 'execute' );
expect( getData( editor.model ) ).to.equal( '<codeBlock language="cs">[]</codeBlock>' );

dropdown.element.remove();
} );
} );

Expand Down
33 changes: 33 additions & 0 deletions packages/ckeditor5-code-block/tests/codeblockui.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,16 @@ describe( 'CodeBlockUI', () => {

it( 'executes the command when executed one of the available language buttons from the list', () => {
const dropdown = editor.ui.componentFactory.create( 'codeBlock' );

dropdown.render();
document.body.appendChild( dropdown.element );

// Make sure that list view is not created before first dropdown open.
expect( dropdown.listView ).to.be.undefined;

// Trigger list view creation (lazy init).
dropdown.isOpen = true;

const executeSpy = sinon.stub( editor, 'execute' );
const focusSpy = sinon.stub( editor.editing.view, 'focus' );
const listView = dropdown.panelView.children.first;
Expand All @@ -82,11 +92,20 @@ describe( 'CodeBlockUI', () => {
language: 'cs',
forceValue: true
} );

dropdown.element.remove();
} );

describe( 'language list', () => {
it( 'corresponds to the config', () => {
const dropdown = editor.ui.componentFactory.create( 'codeBlock' );

// Make sure that list view is not created before first dropdown open.
expect( dropdown.listView ).to.be.undefined;

// Trigger list view creation (lazy init).
dropdown.isOpen = true;

const listView = dropdown.panelView.children.first;

expect( listView.items
Expand Down Expand Up @@ -157,6 +176,13 @@ describe( 'CodeBlockUI', () => {

it( 'sets item\'s #isOn depending on the value of the CodeBlockCommand', () => {
const dropdown = editor.ui.componentFactory.create( 'codeBlock' );

// Make sure that list view is not created before first dropdown open.
expect( dropdown.listView ).to.be.undefined;

// Trigger list view creation (lazy init).
dropdown.isOpen = true;

const listView = dropdown.panelView.children.first;

expect( listView.items.get( 2 ).children.first.isOn ).to.be.false;
Expand All @@ -177,6 +203,13 @@ describe( 'CodeBlockUI', () => {
const editor = newEditor;

const dropdown = editor.ui.componentFactory.create( 'codeBlock' );

// Make sure that list view is not created before first dropdown open.
expect( dropdown.listView ).to.be.undefined;

// Trigger list view creation (lazy init).
dropdown.isOpen = true;

const listView = dropdown.panelView.children.first;

expect( listView.items.first.children.first.label ).to.equal( 'Zwykły tekst' );
Expand Down
19 changes: 12 additions & 7 deletions packages/ckeditor5-find-and-replace/src/findandreplaceui.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,17 @@ export default class FindAndReplaceUI extends Plugin {
// Register the toolbar dropdown component.
editor.ui.componentFactory.add( 'findAndReplace', locale => {
const dropdown = createDropdown( locale );
const formView = this.formView = new FindAndReplaceFormView( editor.locale );

// Dropdown should be disabled when in source editing mode. See #10001.
dropdown.bind( 'isEnabled' ).to( editor.commands.get( 'find' ) );
dropdown.panelView.children.add( formView );

dropdown.once( 'change:isOpen', () => {
this.formView = new FindAndReplaceFormView( editor.locale );

dropdown.panelView.children.add( this.formView );

this._setupFormView( this.formView );
} );

// Every time a dropdown is opened, the search text field should get focused and selected for better UX.
// Note: Using the low priority here to make sure the following listener starts working after
Expand All @@ -68,19 +74,18 @@ export default class FindAndReplaceUI extends Plugin {
// and no longer should be marked in the content.
dropdown.on( 'change:isOpen', ( event, name, isOpen ) => {
if ( isOpen ) {
formView.disableCssTransitions();
this.formView.disableCssTransitions();

formView.reset();
formView._findInputView.fieldView.select();
this.formView.reset();
this.formView._findInputView.fieldView.select();

formView.enableCssTransitions();
this.formView.enableCssTransitions();
} else {
this.fire( 'searchReseted' );
}
}, { priority: 'low' } );

this._setupDropdownButton( dropdown );
this._setupFormView( formView );

return dropdown;
} );
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ describe( 'FindAndReplace', () => {
// (#10014).
editor.setData( LONG_TEXT );

toolbarDropdownView.buttonView.arrowView.fire( 'execute' );
toolbarDropdownView.buttonView.fire( 'execute' );
findAndReplaceUI.formView._findInputView.fieldView.value = 'nothingtobefound';
findAndReplaceUI.formView._findButtonView.fire( 'execute' );

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,17 @@ describe( 'FindAndReplaceUI', () => {
.then( newEditor => {
editor = newEditor;
dropdown = editor.ui.componentFactory.create( 'findAndReplace' );
form = dropdown.panelView.children.get( 0 );
findCommand = editor.commands.get( 'find' );
plugin = editor.plugins.get( 'FindAndReplaceUI' );

dropdown.render();
global.document.body.appendChild( dropdown.element );

// Trigger lazy init.
dropdown.isOpen = true;
dropdown.isOpen = false;

form = dropdown.panelView.children.get( 0 );
} );
} );

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,12 @@ describe( 'FindAndReplaceFormView', () => {
} );

describe( 'options dropdown', () => {
beforeEach( () => {
// Trigger lazy init.
view._optionsDropdown.isOpen = true;
view._optionsDropdown.isOpen = false;
} );

it( 'should be a dropdown', () => {
expect( view._optionsDropdown ).to.be.instanceOf( DropdownView );
expect( view._optionsDropdown.class ).to.equal( 'ck-options-dropdown' );
Expand Down Expand Up @@ -764,10 +770,14 @@ describe( 'FindAndReplaceFormView', () => {
toolbar: [ 'findAndReplace' ]
} );

view = editor.plugins.get( 'FindAndReplaceUI' ).formView;
dropdown = editor.ui.view.toolbar.items
.find( item => item.buttonView && item.buttonView.label == 'Find and replace' );

// Trigger lazy init.
dropdown.isOpen = true;

view = editor.plugins.get( 'FindAndReplaceUI' ).formView;

findInput = view._findInputView;
matchCounterElement = findInput.element.firstChild.childNodes[ 2 ];
replaceInput = view._replaceInputView;
Expand All @@ -777,7 +787,12 @@ describe( 'FindAndReplaceFormView', () => {
replaceButton = view._replaceButtonView;
replaceAllButton = view._replaceAllButtonView;

// Trigger lazy init.
view._optionsDropdown.isOpen = true;
view._optionsDropdown.isOpen = false;

const optionsListView = view._optionsDropdown.panelView.children.get( 0 );

matchCaseSwitch = optionsListView.items.get( 0 ).children.get( 0 );
wholeWordsOnlySwitch = optionsListView.items.get( 1 ).children.get( 0 );
} );
Expand Down
Loading