Skip to content
This repository has been archived by the owner on Jun 26, 2020. It is now read-only.

Commit

Permalink
Merge pull request #60 from ckeditor/i/6192
Browse files Browse the repository at this point in the history
Other: Implemented lazy loading for the font color and background color dropdowns. This will reduce editor initialization time. Closes ckeditor/ckeditor5#6192.
  • Loading branch information
Reinmar authored Feb 12, 2020
2 parents c3e7673 + c73c882 commit 165417c
Show file tree
Hide file tree
Showing 4 changed files with 66 additions and 27 deletions.
74 changes: 47 additions & 27 deletions src/ui/colortableview.js
Original file line number Diff line number Diff line change
Expand Up @@ -118,19 +118,21 @@ export default class ColorTableView extends View {
* Preserves the reference to {@link module:ui/colorgrid/colorgrid~ColorGridView} used to create
* the default (static) color set.
*
* The property is loaded once the the parent dropdown is opened the first time.
*
* @readonly
* @member {module:ui/colorgrid/colorgrid~ColorGridView}
* @member {module:ui/colorgrid/colorgrid~ColorGridView|undefined} staticColorsGrid
*/
this.staticColorsGrid = this._createStaticColorsGrid();

/**
* Preserves the reference to {@link module:ui/colorgrid/colorgrid~ColorGridView} used to create
* the document colors. It remains undefined if the document colors feature is disabled.
*
* The property is loaded once the the parent dropdown is opened the first time.
*
* @readonly
* @member {module:ui/colorgrid/colorgrid~ColorGridView}
* @member {module:ui/colorgrid/colorgrid~ColorGridView|undefined} documentColorsGrid
*/
this.documentColorsGrid;

/**
* Helps cycling over focusable {@link #items} in the list.
Expand All @@ -152,6 +154,15 @@ export default class ColorTableView extends View {
}
} );

/**
* Document color section's label.
*
* @private
* @readonly
* @type {String}
*/
this._documentColorsLabel = documentColorsLabel;

this.setTemplate( {
tag: 'div',
attributes: {
Expand All @@ -164,29 +175,6 @@ export default class ColorTableView extends View {
} );

this.items.add( this._removeColorButton() );
this.items.add( this.staticColorsGrid );

if ( documentColorsCount ) {
// Create a label for document colors.
const bind = Template.bind( this.documentColors, this.documentColors );
const label = new LabelView( this.locale );

label.text = documentColorsLabel;
label.extendTemplate( {
attributes: {
class: [
'ck',
'ck-color-grid__label',
bind.if( 'isEmpty', 'ck-hidden' )
]
}
} );

this.items.add( label );

this.documentColorsGrid = this._createDocumentColorsGrid();
this.items.add( this.documentColorsGrid );
}
}

/**
Expand Down Expand Up @@ -252,6 +240,38 @@ export default class ColorTableView extends View {
this.keystrokes.listenTo( this.element );
}

/**
* Appends {@link #staticColorsGrid} and {@link #documentColorsGrid} views.
*/
appendGrids() {
if ( this.staticColorsGrid ) {
return;
}

this.staticColorsGrid = this._createStaticColorsGrid();

this.items.add( this.staticColorsGrid );

if ( this.documentColorsCount ) {
// Create a label for document colors.
const bind = Template.bind( this.documentColors, this.documentColors );
const label = new LabelView( this.locale );
label.text = this._documentColorsLabel;
label.extendTemplate( {
attributes: {
class: [
'ck',
'ck-color-grid__label',
bind.if( 'isEmpty', 'ck-hidden' )
]
}
} );
this.items.add( label );
this.documentColorsGrid = this._createDocumentColorsGrid();
this.items.add( this.documentColorsGrid );
}
}

/**
* Focuses the first focusable element in {@link #items}.
*/
Expand Down
3 changes: 3 additions & 0 deletions src/ui/colorui.js
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,9 @@ export default class ColorUI extends Plugin {
} );

dropdownView.on( 'change:isOpen', ( evt, name, isVisible ) => {
// Grids rendering is deferred (#6192).
dropdownView.colorTableView.appendGrids();

if ( isVisible ) {
if ( documentColorsCount !== 0 ) {
this.colorTableView.updateDocumentColors( editor.model, this.componentName );
Expand Down
4 changes: 4 additions & 0 deletions tests/ui/colortableview.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ describe( 'ColorTableView', () => {
documentColorsLabel: 'Document colors',
documentColorsCount: 4
} );
colorTableView.appendGrids();
colorTableView.render();
} );

Expand Down Expand Up @@ -363,6 +364,7 @@ describe( 'ColorTableView', () => {
removeButtonLabel: 'Remove color',
documentColorsCount: 0
} );
colorTableView.appendGrids();
colorTableView.render();
} );

Expand Down Expand Up @@ -418,6 +420,8 @@ describe( 'ColorTableView', () => {
model = editor.model;

dropdown = editor.ui.componentFactory.create( 'testColor' );
dropdown.isOpen = true;
dropdown.isOpen = false;
dropdown.render();

staticColorsGrid = dropdown.colorTableView.staticColorsGrid;
Expand Down
12 changes: 12 additions & 0 deletions tests/ui/colorui.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

import TestColorPlugin from '../_utils/testcolorplugin';
import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph';
import ColorGridView from '@ckeditor/ckeditor5-ui/src/colorgrid/colorgridview';
import global from '@ckeditor/ckeditor5-utils/src/dom/global';
import ClassicTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/classictesteditor';
import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils';
Expand Down Expand Up @@ -116,6 +117,7 @@ describe( 'ColorUI', () => {
beforeEach( () => {
command = editor.commands.get( 'testColorCommand' );
dropdown = editor.ui.componentFactory.create( 'testColor' );
dropdown.isOpen = true;

dropdown.render();
} );
Expand Down Expand Up @@ -152,6 +154,15 @@ describe( 'ColorUI', () => {
expect( colorTableView.documentColorsCount ).to.equal( 3 );
} );

it( 'does not initialize grids when not open', () => {
const localDropdown = editor.ui.componentFactory.create( 'testColor' );
localDropdown.render();

for ( const item of localDropdown.colorTableView.items ) {
expect( item ).not.to.be.instanceOf( ColorGridView );
}
} );

describe( 'model to command binding', () => {
it( 'isEnabled', () => {
command.isEnabled = false;
Expand Down Expand Up @@ -308,6 +319,7 @@ describe( 'ColorUI', () => {

colors.forEach( test => {
it( `tested color "${ test.color }" translated to "${ test.label }".`, () => {
dropdown.isOpen = true;
const colorGrid = dropdown.colorTableView.items.get( 1 );
const tile = colorGrid.items.find( colorTile => test.color === colorTile.color );

Expand Down

0 comments on commit 165417c

Please sign in to comment.