Merge pull request #14215 from ckeditor/ck/3306-fix-keys-behaviour-in…

…-color-picker Fix (ui): Fixed the arrow keys behavior in the color picker component. Closes #14218.
ckeditor · May 29, 2023 · 29be557 · 29be557
2 parents 1f5c8d0 + f4f8151
commit 29be557
Show file tree

Hide file tree

Showing 3 changed files with 52 additions and 2 deletions.
diff --git a/packages/ckeditor5-font/tests/ui/colortableview.js b/packages/ckeditor5-font/tests/ui/colortableview.js
@@ -275,7 +275,7 @@ describe( 'ColorTableView', () => {
 			expect( colorPicker._hexColor, '`_hexColor` property value is incorrect' ).to.equal( '#660000' );
 		} );
 
-		it( 'should propagate the selected color to color picker if it changes', () => {
+		it( 'should propagate the selected color to color picker component if it changes', () => {
 			colorTableView.selectedColor = '#660000';
 			colorTableView._appendColorPicker();
 			colorTableView.selectedColor = '#660055';
@@ -286,6 +286,16 @@ describe( 'ColorTableView', () => {
 			expect( colorPicker._hexColor, '`_hexColor` property value is incorrect' ).to.equal( '#660055' );
 		} );
 
+		it( 'should propagate the selected color to color picker HTML if it changes', () => {
+			colorTableView.selectedColor = '#660000';
+			colorTableView._appendColorPicker();
+			colorTableView.selectedColor = '#660055';
+
+			const colorPicker = colorTableView.colorPickerPageView.colorPickerView;
+
+			expect( colorPicker.picker.getAttribute( 'color' ) ).to.equal( '#660055' );
+		} );
+
 		it( 'should navigate forwards using the Tab key', () => {
 			const keyEvtData = {
 				keyCode: keyCodes.tab,

diff --git a/packages/ckeditor5-ui/src/colorpicker/colorpickerview.ts b/packages/ckeditor5-ui/src/colorpicker/colorpickerview.ts
@@ -114,7 +114,10 @@ export default class ColorPickerView extends View {
 		} );
 
 		this.on( 'change:_hexColor', () => {
-			this.picker.setAttribute( 'color', this._hexColor );
+			// Should update color in color picker when its not focused
+			if ( document.activeElement !== this.picker ) {
+				this.picker.setAttribute( 'color', this._hexColor );
+			}
 
 			// There has to be two way binding between properties.
 			// Extra precaution has to be taken to trigger change back only when the color really changes.

diff --git a/packages/ckeditor5-ui/tests/colorpicker/colorpickerview.js b/packages/ckeditor5-ui/tests/colorpicker/colorpickerview.js
@@ -176,6 +176,43 @@ describe( 'ColorPickerView', () => {
 					} );
 				} );
 			} );
+
+			it( 'should not set any color directly to color picker HTML when its focused', () => {
+				document.body.appendChild( view.picker );
+
+				const spy = sinon.spy( view.picker, 'setAttribute' );
+
+				view.focus();
+
+				const event = new CustomEvent( 'color-changed', {
+					detail: {
+						value: '#733232'
+					}
+				} );
+
+				view.picker.dispatchEvent( event );
+
+				clock.tick( 200 );
+
+				sinon.assert.notCalled( spy );
+
+				// Cleanup DOM
+				view.picker.remove();
+			} );
+
+			it( 'should set color in color picker HTML when change was caused by input', () => {
+				const spy = sinon.spy( view.picker, 'setAttribute' );
+
+				const fieldView = view.hexInputRow.children.get( 1 ).fieldView;
+
+				fieldView.isFocused = true;
+				fieldView.value = '#ffffff';
+				fieldView.fire( 'input' );
+
+				clock.tick( 200 );
+
+				sinon.assert.calledOnce( spy );
+			} );
 		} );
 
 		describe( 'should not update color property', () => {