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

Active font color is now visible in both color grids #53

Merged
merged 5 commits into from
Aug 9, 2019
Merged

Active font color is now visible in both color grids #53

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
9709397
Add checkmark in both color grid sections.
Aug 7, 2019
177cdc4
Improve unit tests names.
Aug 8, 2019
69e21e3
Correct typo in property name.
Aug 8, 2019
bf7096c
Unit test names improvements.
Aug 8, 2019
506d0ee
Update tests/ui/colortableview.js
oleq Aug 8, 2019
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
18 changes: 6 additions & 12 deletions src/ui/colortableview.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ export default class ColorTableView extends View {
* @readonly
* @member {module:ui/colorgrid/colorgrid~ColorGridView}
*/
this.documentColorGrid;
this.documentColorsGrid;

/**
* Helps cycling over focusable {@link #items} in the list.
Expand Down Expand Up @@ -222,23 +222,17 @@ export default class ColorTableView extends View {

/**
* Method refresh state of `selectedColor` in single or both {@link module:ui/colorgrid/colorgrid~ColorGridView}
* available in {@link module:font/ui/colortableview~ColorTableView}. It guarantees that selection will occur only in one of them.
* available in {@link module:font/ui/colortableview~ColorTableView}.
*/
updateSelectedColors() {
const documentColorsGrid = this.documentColorsGrid;
const staticColorsGrid = this.staticColorsGrid;
const selectedColor = this.selectedColor;

if ( !this.documentColors.isEmpty ) {
if ( this.documentColors.hasColor( selectedColor ) ) {
staticColorsGrid.selectedColor = null;
documentColorsGrid.selectedColor = selectedColor;
} else {
staticColorsGrid.selectedColor = selectedColor;
documentColorsGrid.selectedColor = null;
}
} else {
staticColorsGrid.selectedColor = selectedColor;
staticColorsGrid.selectedColor = selectedColor;

if ( documentColorsGrid ) {
documentColorsGrid.selectedColor = selectedColor;
}
}

Expand Down
115 changes: 69 additions & 46 deletions tests/ui/colortableview.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,71 +89,71 @@ describe( 'ColorTableView', () => {
testUtils.createSinonSandbox();

describe( 'constructor()', () => {
it( 'creates items collection', () => {
it( 'should create items collection', () => {
expect( colorTableView.items ).to.be.instanceOf( ViewCollection );
} );

it( 'store color definitions', () => {
it( 'should store colors\' definitions', () => {
expect( colorTableView.colorDefinitions ).to.be.instanceOf( Array );
expect( colorTableView.colorDefinitions ).to.deep.equal( colorDefinitions );
} );

it( 'creates focus tracker', () => {
it( 'should create focus tracker', () => {
expect( colorTableView.focusTracker ).to.be.instanceOf( FocusTracker );
} );

it( 'creates keystroke handler', () => {
it( 'should create keystroke handler', () => {
expect( colorTableView.keystrokes ).to.be.instanceOf( KeystrokeHandler );
} );

it( 'creates observable for selected color', () => {
it( 'should create observable for selected color', () => {
expect( colorTableView.selectedColor ).to.be.undefined;

colorTableView.set( 'selectedColor', 'white' );

expect( colorTableView.selectedColor ).to.equal( 'white' );
} );

it( 'sets tooltip for the remove color button', () => {
it( 'should set tooltip for the remove color button', () => {
expect( colorTableView.removeButtonLabel ).to.equal( 'Remove color' );
} );

it( 'sets number of drawn columns', () => {
it( 'should set number of drawn columns', () => {
expect( colorTableView.columns ).to.equal( 5 );
} );

it( 'creates collection of document colors', () => {
it( 'should create collection of document colors', () => {
expect( colorTableView.documentColors ).to.be.instanceOf( Collection );
} );

it( 'sets maximum number of document colors', () => {
it( 'should set maximum number of document colors', () => {
expect( colorTableView.documentColorsCount ).to.equal( 4 );
} );

it( 'creates focus cycler', () => {
it( 'should create focus cycler', () => {
expect( colorTableView._focusCycler ).to.be.instanceOf( FocusCycler );
} );

it( 'has correct class', () => {
it( 'should apply correct classes', () => {
expect( colorTableView.element.classList.contains( 'ck' ) ).to.be.true;
expect( colorTableView.element.classList.contains( 'ck-color-table' ) ).to.be.true;
} );

it( 'has correct amount of children', () => {
it( 'should have correct amount of children', () => {
expect( colorTableView.items.length ).to.equal( 4 );
} );
} );

describe( 'update elements in focus tracker', () => {
it( 'focuses the tile in DOM', () => {
describe( 'focus tracker', () => {
it( 'should focus first child of colorTableView in DOM', () => {
const spy = sinon.spy( colorTableView._focusCycler, 'focusFirst' );

colorTableView.focus();

sinon.assert.calledOnce( spy );
} );

it( 'focuses last the tile in DOM', () => {
it( 'should focuses the last child of colorTableView in DOM', () => {
const spy = sinon.spy( colorTableView._focusCycler, 'focusLast' );

colorTableView.focusLast();
Expand All @@ -169,18 +169,18 @@ describe( 'ColorTableView', () => {
removeButton = colorTableView.items.first;
} );

it( 'has proper class', () => {
it( 'should have proper class', () => {
expect( removeButton.element.classList.contains( 'ck-color-table__remove-color' ) ).to.be.true;
} );

it( 'has proper setting', () => {
it( 'should have proper settings', () => {
expect( removeButton.withText ).to.be.true;
expect( removeButton.icon ).to.equal( removeButtonIcon );
expect( removeButton.tooltip ).to.be.true;
expect( removeButton.label ).to.equal( 'Remove color' );
} );

it( 'executes event with "null" value', () => {
it( 'should execute event with "null" value', () => {
const spy = sinon.spy();
colorTableView.on( 'execute', spy );

Expand All @@ -198,12 +198,12 @@ describe( 'ColorTableView', () => {
staticColorTable = colorTableView.items.get( 1 );
} );

it( 'has added 3 children from definition', () => {
it( 'should have added 3 children from definition', () => {
expect( staticColorTable.items.length ).to.equal( 3 );
} );

colorDefinitions.forEach( ( item, index ) => {
it( `dispatch event to parent element for color: ${ item.color }`, () => {
it( `should dispatch event to parent element for color: ${ item.color }`, () => {
const spy = sinon.spy();
colorTableView.on( 'execute', spy );

Expand Down Expand Up @@ -256,7 +256,7 @@ describe( 'ColorTableView', () => {
} );

describe( 'model manipulation', () => {
it( 'adding items works properly', () => {
it( 'should add item to document colors', () => {
expect( documentColors.length ).to.equal( 0 );

documentColors.add( Object.assign( {}, colorBlack ) );
Expand All @@ -267,7 +267,7 @@ describe( 'ColorTableView', () => {
expect( documentColors.first.options.hasBorder ).to.be.false;
} );

it( 'adding multiple times same color should not populate items collection', () => {
it( 'should not add same item twice one after another', () => {
expect( documentColors.length ).to.equal( 0 );

documentColors.add( Object.assign( {}, colorBlack ) );
Expand All @@ -281,7 +281,7 @@ describe( 'ColorTableView', () => {
expect( documentColors.length ).to.equal( 1 );
} );

it( 'adding duplicated colors don\'t add it to model', () => {
it( 'should not add item if it\'s present on the documentColor list', () => {
expect( documentColors.length ).to.equal( 0 );

documentColors.add( Object.assign( {}, colorBlack ) );
Expand Down Expand Up @@ -313,7 +313,7 @@ describe( 'ColorTableView', () => {
} );

describe( 'events', () => {
it( 'added colors delegates execute to parent', () => {
it( 'should delegate execute to parent', () => {
const spy = sinon.spy();
colorTableView.on( 'execute', spy );

Expand All @@ -328,8 +328,8 @@ describe( 'ColorTableView', () => {
} );

describe( 'binding', () => {
it( 'add tile item when document colors model is updated', () => {
let itm;
it( 'should add new colorTile item when document colors model is updated', () => {
let colorTile;

expect( documentColors.length ).to.equal( 0 );
expect( documentColorsGridView.items.length ).to.equal( 0 );
Expand All @@ -338,17 +338,17 @@ describe( 'ColorTableView', () => {
expect( documentColors.length ).to.equal( 1 );
expect( documentColorsGridView.items.length ).to.equal( 1 );

itm = documentColorsGridView.items.first;
expect( itm ).to.be.instanceOf( ColorTileView );
expect( itm.label ).to.equal( 'Black' );
expect( itm.color ).to.equal( '#000000' );
expect( itm.hasBorder ).to.be.false;
colorTile = documentColorsGridView.items.first;
expect( colorTile ).to.be.instanceOf( ColorTileView );
expect( colorTile.label ).to.equal( 'Black' );
expect( colorTile.color ).to.equal( '#000000' );
expect( colorTile.hasBorder ).to.be.false;

documentColors.add( Object.assign( {}, colorEmpty ) );
itm = documentColorsGridView.items.get( 1 );
expect( itm ).to.be.instanceOf( ColorTileView );
expect( itm.color ).to.equal( 'hsla(0,0%,0%,0)' );
expect( itm.hasBorder ).to.be.true;
colorTile = documentColorsGridView.items.get( 1 );
expect( colorTile ).to.be.instanceOf( ColorTileView );
expect( colorTile.color ).to.equal( 'hsla(0,0%,0%,0)' );
expect( colorTile.hasBorder ).to.be.true;
} );
} );
} );
Expand Down Expand Up @@ -379,7 +379,7 @@ describe( 'ColorTableView', () => {
} );

describe( '_addColorToDocumentColors', () => {
it( 'add custom color from not defined colors', () => {
it( 'should add custom color', () => {
colorTableView._addColorToDocumentColors( '#123456' );
expect( colorTableView.documentColors.get( 0 ) ).to.deep.include( {
color: '#123456',
Expand All @@ -390,7 +390,7 @@ describe( 'ColorTableView', () => {
} );
} );

it( 'add already define color based on color value', () => {
it( 'should detect already define color based on color value and use', () => {
colorTableView._addColorToDocumentColors( 'rgb(255,255,255)' );
// Color values are kept without spaces.
expect( colorTableView.documentColors.get( 0 ) ).to.deep.include( {
Expand Down Expand Up @@ -435,7 +435,7 @@ describe( 'ColorTableView', () => {
return editor.destroy();
} );

it( 'checkmark is present in document colors section', () => {
it( 'should have color tile turned on in document colors and static colors section', () => {
const command = editor.commands.get( 'testColorCommand' );

setModelData( model,
Expand All @@ -445,17 +445,17 @@ describe( 'ColorTableView', () => {

dropdown.isOpen = true;

expect( staticColorsGrid.selectedColor ).to.be.null;
expect( staticColorsGrid.selectedColor ).to.equal( 'red' );
expect( documentColorsGrid.selectedColor ).to.equal( 'red' );

const redStaticColorTile = staticColorsGrid.items.find( tile => tile.color === 'red' );
const redDocumentColorTile = documentColorsGrid.items.get( 0 );

expect( redStaticColorTile.isOn ).to.be.false;
expect( redStaticColorTile.isOn ).to.be.true;
expect( redDocumentColorTile.isOn ).to.be.true;
} );

it( 'checkmark is present in static colors', () => {
it( 'should have color tile turned on in static colors section when document colors are full', () => {
const command = editor.commands.get( 'testColorCommand' );

setModelData( model,
Expand All @@ -469,17 +469,40 @@ describe( 'ColorTableView', () => {
dropdown.isOpen = true;

expect( staticColorsGrid.selectedColor ).to.equal( '#00FF00' );
expect( documentColorsGrid.selectedColor ).to.be.null;
expect( documentColorsGrid.selectedColor ).to.equal( '#00FF00' );

const redStaticColorTile = staticColorsGrid.items.find( tile => tile.color === '#00FF00' );
const redDocumentColorTile = documentColorsGrid.items.get( 0 );
const activeDocumentColorTile = documentColorsGrid.items.find( tile => tile.isOn );

expect( redStaticColorTile.isOn ).to.be.true;
expect( redDocumentColorTile.isOn ).to.be.false;
expect( activeDocumentColorTile ).to.be.undefined;
} );

it( 'should not have a selection for unknown colors exceeding document colors limit', () => {
const command = editor.commands.get( 'testColorCommand' );

setModelData( model,
'<paragraph><$text testColor="gold">Bar</$text></paragraph>' +
'<paragraph><$text testColor="rgb(10,20,30)">Foo</$text></paragraph>' +
'<paragraph><$text testColor="#FFAACC">Baz</$text></paragraph>' +
'<paragraph><$text testColor="pink">Test</$text></paragraph>'
);
command.value = 'pink';

dropdown.isOpen = true;

expect( staticColorsGrid.selectedColor ).to.equal( 'pink' );
expect( documentColorsGrid.selectedColor ).to.equal( 'pink' );

const activeStaticDocumentTile = staticColorsGrid.items.find( tile => tile.isOn );
const activeDocumentColorTile = documentColorsGrid.items.find( tile => tile.isOn );

expect( activeStaticDocumentTile ).to.be.undefined;
expect( activeDocumentColorTile ).to.be.undefined;
} );
} );

describe( 'empty document colors', () => {
describe( 'disabled document colors section', () => {
let editor, element, dropdown, model;

beforeEach( () => {
Expand Down Expand Up @@ -511,7 +534,7 @@ describe( 'ColorTableView', () => {
return editor.destroy();
} );

it( 'document colors are not created', () => {
it( 'should not create document colors section', () => {
const colorTableView = dropdown.colorTableView;

setModelData( model,
Expand Down