Skip to content

Commit

Permalink
Merge pull request #15658 from ckeditor/ck/15580-font-color-and-font-…
Browse files Browse the repository at this point in the history 
…background-color-throw-error

Fix (font): Font color and font background should work in both the main toolbar and the balloon toolbar. Closes #15580.

MINOR BREAKING CHANGE (font): The `colorSelectorView` property will no longer be accessible from the `ColorUI` plugin in the `@ckeditor/ckeditor5-font/src/ui/colorui.ts`.
  • Loading branch information
@niegowski
niegowski committed Jan 19, 2024
2 parents 6b8372e + a97f3c8 commit 63aaa3e
Show file tree
Hide file tree
Showing 5 changed files with 161 additions and 18 deletions.
23 changes: 8 additions & 15 deletions packages/ckeditor5-font/src/ui/colorui.ts
View file
Expand Up @@ -14,7 +14,6 @@ import {
normalizeColorOptions,
getLocalizedColorOptions,
focusChildOnDropdownOpen,
type ColorSelectorView,
type ColorSelectorExecuteEvent,
type ColorSelectorColorPickerCancelEvent,
type ColorSelectorColorPickerShowEvent
Expand Down Expand Up @@ -63,11 +62,6 @@ export default class ColorUI extends Plugin {
*/
public columns: number;

/**
* Keeps a reference to {@link module:ui/colorselector/colorselectorview~ColorSelectorView}.
*/
public colorSelectorView: ColorSelectorView | undefined;

/**
* Keeps all changes in color picker in one batch while dropdown is open.
*/
Expand Down Expand Up @@ -100,7 +94,6 @@ export default class ColorUI extends Plugin {
this.icon = icon;
this.dropdownLabel = dropdownLabel;
this.columns = editor.config.get( `${ this.componentName }.columns` )!;
this.colorSelectorView = undefined;
}

/**
Expand All @@ -123,7 +116,7 @@ export default class ColorUI extends Plugin {
// Font color dropdown rendering is deferred once it gets open to improve performance (#6192).
let dropdownContentRendered = false;

this.colorSelectorView = addColorSelectorToDropdown( {
const colorSelectorView = addColorSelectorToDropdown( {
dropdownView,
colors: localizedColors.map( option => ( {
label: option.label,
Expand All @@ -140,7 +133,7 @@ export default class ColorUI extends Plugin {
colorPickerViewConfig: hasColorPicker ? ( componentConfig.colorPicker || {} ) : false
} );

this.colorSelectorView.bind( 'selectedColor' ).to( command, 'value' );
colorSelectorView.bind( 'selectedColor' ).to( command, 'value' );

dropdownView.buttonView.set( {
label: this.dropdownLabel,
Expand All @@ -156,7 +149,7 @@ export default class ColorUI extends Plugin {

dropdownView.bind( 'isEnabled' ).to( command );

this.colorSelectorView.on<ColorSelectorExecuteEvent>( 'execute', ( evt, data ) => {
colorSelectorView.on<ColorSelectorExecuteEvent>( 'execute', ( evt, data ) => {
if ( dropdownView.isOpen ) {
editor.execute( this.commandName, {
value: data.value,
Expand All @@ -173,11 +166,11 @@ export default class ColorUI extends Plugin {
}
} );

this.colorSelectorView.on<ColorSelectorColorPickerShowEvent>( 'colorPicker:show', () => {
colorSelectorView.on<ColorSelectorColorPickerShowEvent>( 'colorPicker:show', () => {
this._undoStepBatch = editor.model.createBatch();
} );

this.colorSelectorView.on<ColorSelectorColorPickerCancelEvent>( 'colorPicker:cancel', () => {
colorSelectorView.on<ColorSelectorColorPickerCancelEvent>( 'colorPicker:cancel', () => {
if ( this._undoStepBatch!.operations.length ) {
// We need to close the dropdown before the undo batch.
// Otherwise, ColorUI treats undo as a selected color change,
Expand All @@ -199,11 +192,11 @@ export default class ColorUI extends Plugin {

if ( isVisible ) {
if ( documentColorsCount !== 0 ) {
this.colorSelectorView!.updateDocumentColors( editor.model, this.componentName );
colorSelectorView!.updateDocumentColors( editor.model, this.componentName );
}

this.colorSelectorView!.updateSelectedColors();
this.colorSelectorView!.showColorGridsFragment();
colorSelectorView!.updateSelectedColors();
colorSelectorView!.showColorGridsFragment();
}
} );

Expand Down
25 changes: 25 additions & 0 deletions packages/ckeditor5-font/tests/manual/tickets/15580/1.html
View file
@@ -0,0 +1,25 @@
<div id="editor">
<p><span>01. no-color; no-color</span></p>
<p><span style="color:hsl(0, 0%, 100%);background-color:hsl(0, 0%, 0%);">02. color:hsl(0, 0%, 100%);background-color:hsl(0, 0%, 0%);</span></p>
<p><span style="color:hsl(0, 75%, 60%);">03. color:hsl(0, 75%, 60%); no-color;</span></p>
<p><span style="background-color:hsl(90, 75%, 60%);">04. no-color; background-color:hsl(90, 75%, 60%);</span></p>
<p><span style="color:hsl(30, 75%, 60%);background-color:hsl(0, 0%, 30%);">05. color:hsl(30, 75%, 60%); background-color:hsl(0, 0%, 30%);</span></p>
<p><span style="color:#00FFFF;background-color:rgb(255, 0, 0);">06. color:#00FFFF;background-color:rgb(255, 0, 0);</span></p>
<p><span style="color:hsla( 0, 0%, 0%, .7);background-color:gold;">07. color:hsla( 0, 0%, 0%, .7); background-color:gold;</span></p>
<p><span style="color:rgba( 0, 120, 250, 0.8);background-color:hsla(270, 100%, 50%, 0.3);">08. color:;background-color:hsla(270, 100%, 50%, 0.3);</span></p>
<p><span style="color:#ddd;background-color:hsl(150, 75%, 60%);">09. color:#ddd;background-color:hsl(150, 75%, 60%);</span></p>
<p><span style="color:hsl(270, 75%, 60%);background-color:#d82;">10. color:hsl(270, 75%, 60%);background-color:#d82;</span></p>
</div>

<div id="color-box" style="margin-top: 10px; border: 1px solid black; padding: 5px">
<p>You can use this helper to generate text with a particular color / background applied. Change inputs and accept it with enter key.</p>
<label>
Color:
<input type="text" name="color" id="color" placeholder="#3d3d3d">
</label>
<label>
Background Color:
<input type="text" name="bgcolor" id="bgcolor" placeholder="pink">
</label>
<p contenteditable="true"><span>Text for copying.</span></p>
</div>
59 changes: 59 additions & 0 deletions packages/ckeditor5-font/tests/manual/tickets/15580/1.js
View file
@@ -0,0 +1,59 @@
/**
* @license Copyright (c) 2003-2024, CKSource Holding sp. z o.o. All rights reserved.
* For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license
*/

/* globals console, window, document */

import ClassicEditor from '@ckeditor/ckeditor5-editor-classic/src/classiceditor.js';
import ArticlePluginSet from '@ckeditor/ckeditor5-core/tests/_utils/articlepluginset.js';
import FontColor from '../../../../src/fontcolor.js';
import FontBackgroundColor from '../../../../src/fontbackgroundcolor.js';
import { BalloonToolbar } from '@ckeditor/ckeditor5-ui';

ClassicEditor
.create( document.querySelector( '#editor' ), {
image: { toolbar: [ 'toggleImageCaption', 'imageTextAlternative' ] },
plugins: [
ArticlePluginSet,
FontColor,
FontBackgroundColor,
BalloonToolbar
],
toolbar: [
'heading',
'|',
'fontColor',
'fontBackgroundColor',
'bold',
'italic',
'link',
'bulletedList',
'numberedList',
'blockQuote',
'undo',
'redo'
],
fontColor: {
columns: 3
},
balloonToolbar: [ 'bold', 'italic', 'fontColor' ]
} )
.then( editor => {
window.editor = editor;
} )
.catch( err => {
console.error( err.stack );
} );

function updateText( styleName ) {
return evt => {
const el = document.querySelector( '#color-box > p > span' );
if ( el ) {
el.style[ styleName ] = evt.target.value;
}
};
}

document.getElementById( 'color' ).addEventListener( 'change', updateText( 'color' ) );
document.getElementById( 'bgcolor' ).addEventListener( 'change', updateText( 'backgroundColor' ) );
5 changes: 5 additions & 0 deletions packages/ckeditor5-font/tests/manual/tickets/15580/1.md
View file
@@ -0,0 +1,5 @@
## Font color and font background color throw error while having buttons in the main toolbar and the balloon toolbar [#8233](https://github.com/ckeditor/ckeditor5/pull/15658)

1. Select text, should appear balloon with font color button.
2. Click in the font color button and change color.
3. There should be no errors in the console.
67 changes: 64 additions & 3 deletions packages/ckeditor5-font/tests/ui/colorui.js
View file
Expand Up @@ -232,7 +232,7 @@ describe( 'ColorUI', () => {
const spyUndo = sinon.spy( editor.commands.get( 'undo' ), 'execute' );

dropdown.isOpen = true;
testColorPlugin.colorSelectorView.fire( 'colorPicker:show' );
dropdown.colorSelectorView.fire( 'colorPicker:show' );

dropdown.colorSelectorView.selectedColor = 'hsl( 0, 0%, 100% )';

Expand All @@ -250,7 +250,7 @@ describe( 'ColorUI', () => {

it( 'should create new batch when color picker is showed', () => {
dropdown.isOpen = true;
testColorPlugin.colorSelectorView.colorGridsFragmentView.colorPickerButtonView.fire( 'execute' );
dropdown.colorSelectorView.colorGridsFragmentView.colorPickerButtonView.fire( 'execute' );

dropdown.colorSelectorView.selectedColor = '#000000';

Expand All @@ -270,7 +270,7 @@ describe( 'ColorUI', () => {
} );

dropdown.isOpen = true;
testColorPlugin.colorSelectorView.colorGridsFragmentView.colorPickerButtonView.fire( 'execute' );
dropdown.colorSelectorView.colorGridsFragmentView.colorPickerButtonView.fire( 'execute' );

expect( testColorPlugin._undoStepBatch.operations.length,
'should have 0 changes in batch' ).to.equal( 0 );
Expand Down Expand Up @@ -535,4 +535,65 @@ describe( 'ColorUI', () => {
expect( dropdown.colorSelectorView.colorPickerView ).to.be.undefined;
} );
} );

// Issue: https://github.com/ckeditor/ckeditor5/issues/15580
// For simplicity we create editor with two same buttons in toolbar
// instead overcomplicating stuff with ballon toolbar.
describe( 'toolabar with two same instance of testColor', () => {
let editor, element;

beforeEach( () => {
element = document.createElement( 'div' );
document.body.appendChild( element );

return ClassicTestEditor
.create( element, {
plugins: [ Paragraph, TestColorPlugin, Undo ],
testColor: testColorConfig,
toolbar: [ 'testColor' ]
} )
.then( newEditor => {
editor = newEditor;
model = editor.model;
testColorPlugin = newEditor.plugins.get( 'TestColorPlugin' );
} );
} );

afterEach( () => {
element.remove();

return editor.destroy();
} );

describe( 'testColor Dropdown', () => {
let dropdown, dropdown2;

beforeEach( () => {
command = editor.commands.get( 'testColorCommand' );
dropdown = editor.ui.componentFactory.create( 'testColor' );
dropdown2 = editor.ui.componentFactory.create( 'testColor' );
} );

afterEach( () => {
dropdown.destroy();
dropdown2.destroy();
} );

it( 'should execute command if in toolbar there are more than one dropdowns', () => {
const spy = sinon.spy( editor, 'execute' );

dropdown.isOpen = true;

dropdown.colorSelectorView.colorPickerFragmentView.colorPickerView.fire( 'colorSelected', { color: '#a37474' } );

sinon.assert.calledWithExactly( spy, 'testColorCommand', sinon.match( { value: '#a37474' } ) );

dropdown2.isOpen = true;

dropdown2.colorSelectorView.colorPickerFragmentView.colorPickerView.fire( 'colorSelected', { color: '#ffffff' } );

sinon.assert.calledWithExactly( spy, 'testColorCommand', sinon.match( { value: '#ffffff' } ) );
} );
} );
} );
} );

0 comments on commit 63aaa3e

Please sign in to comment.