Group changes into one batch

packages/ckeditor5-font/package.json

@@ -16,6 +16,7 @@
     "@ckeditor/ckeditor5-ui": "^37.1.0"
   },
   "devDependencies": {
+    "@ckeditor/ckeditor5-undo": "^37.1.0",
     "@ckeditor/ckeditor5-core": "^37.1.0",
     "@ckeditor/ckeditor5-dev-utils": "^37.0.0",
     "@ckeditor/ckeditor5-editor-classic": "^37.1.0",

packages/ckeditor5-font/src/fontcommand.ts

@@ -8,6 +8,7 @@
  */
 
 import { Command, type Editor } from 'ckeditor5/src/core';
+import { type Batch, type Writer } from 'ckeditor5/src/engine';
 
 /**
  * The base font command.
@@ -57,14 +58,15 @@ export default abstract class FontCommand extends Command {
 	 * @param options.value The value to apply.
 	 * @fires execute
 	 */
-	public override execute( options: { value?: string } = {} ): void {
+	public override execute( options: { value?: string; batch?: Batch } = {} ): void {
 		const model = this.editor.model;
 		const document = model.document;
 		const selection = document.selection;
 
 		const value = options.value;
+		const batch = options.batch;
 
-		model.change( writer => {
+		const updateAttribute = ( writer: Writer ) => {
 			if ( selection.isCollapsed ) {
 				if ( value ) {
 					writer.setSelectionAttribute( this.attributeKey, value );
@@ -82,6 +84,17 @@ export default abstract class FontCommand extends Command {
 					}
 				}
 			}
-		} );
+		};
+
+		// In some scenarios, you may want to use a single undo step for multiple changes (e.g. in color picker).
+		if ( batch ) {
+			model.enqueueChange( batch, writer => {
+				updateAttribute( writer );
+			} );
+		} else {
+			model.change( writer => {
+				updateAttribute( writer );
+			} );
+		}
 	}
 }

packages/ckeditor5-font/src/ui/colortableview.ts

@@ -223,6 +223,7 @@ export default class ColorTableView extends View {
 		}
 		this.items.add( this.colorGridsPageView );
 		this.colorGridsPageView.delegate( 'execute' ).to( this );
+		this.colorGridsPageView.delegate( 'showColorPicker' ).to( this );
 	}
 
 	/**
@@ -625,6 +626,10 @@ class ColorGridsPageView extends View {
 			icon: ColorPaletteIcon,
 			class: 'ck-color-table__color-picker'
 		} );
+
+		this.colorPickerButtonView.on( 'execute', () => {
+			this.fire<ColorTableShowColorPickerEvent>( 'showColorPicker' );
+		} );
 	}
 
 	/**
@@ -829,12 +834,12 @@ class ColorPickerPageView extends View {
 			keystrokes,
 			colorPickerConfig
 		}:
-		{
-			focusTracker: FocusTracker;
-			focusables: ViewCollection;
-			keystrokes: KeystrokeHandler;
-			colorPickerConfig: ColorPickerConfig | false;
-		}
+			{
+				focusTracker: FocusTracker;
+				focusables: ViewCollection;
+				keystrokes: KeystrokeHandler;
+				colorPickerConfig: ColorPickerConfig | false;
+			}
 	) {
 		super( locale );
 
@@ -1060,3 +1065,13 @@ export type ColorTableCancelEvent = {
 	name: 'cancel';
 	args: [];
 };
+
+/**
+ * Fired whenever color picker will be shown.
+ *
+ * @eventName ~ColorTableView#showColorPicker
+ */
+export type ColorTableShowColorPickerEvent = {
+	name: 'showColorPicker';
+	args: [];
+};

packages/ckeditor5-font/src/ui/colorui.ts

@@ -8,6 +8,7 @@
  */
 
 import { Plugin, type Editor } from 'ckeditor5/src/core';
+import type { Batch } from 'ckeditor5/src/engine';
 import { createDropdown, normalizeColorOptions, getLocalizedColorOptions, focusChildOnDropdownOpen } from 'ckeditor5/src/ui';
 
 import {
@@ -19,7 +20,8 @@ import {
 import {
 	type default as ColorTableView,
 	type ColorTableExecuteEvent,
-	type ColorTableCancelEvent
+	type ColorTableCancelEvent,
+	type ColorTableShowColorPickerEvent
 } from './colortableview';
 import type FontColorCommand from '../fontcolor/fontcolorcommand';
 import type FontBackgroundColorCommand from '../fontbackgroundcolor/fontbackgroundcolorcommand';
@@ -63,6 +65,11 @@ export default class ColorUI extends Plugin {
 	 */
 	public colorTableView: ColorTableView | undefined;
 
+	/**
+	 * Keeps all changes in color picker in one batch while dropdown is open.
+	 */
+	declare private _undoStepBatch: Batch;
+
 	/**
 	 * Creates a plugin which introduces a dropdown with a pre–configured {@link module:font/ui/colortableview~ColorTableView}.
 	 *
@@ -111,7 +118,6 @@ export default class ColorUI extends Plugin {
 			const dropdownView: ColorTableDropdownView = createDropdown( locale );
 			// Font color dropdown rendering is deferred once it gets open to improve performance (#6192).
 			let dropdownContentRendered = false;
-			let colorSavedUponDropdownOpen: string;
 
 			this.colorTableView = addColorTableToDropdown( {
 				dropdownView,
@@ -148,19 +154,25 @@ export default class ColorUI extends Plugin {
 
 			this.colorTableView.on<ColorTableExecuteEvent>( 'execute', ( evt, data ) => {
 				if ( dropdownView.isOpen ) {
-					editor.execute( this.commandName, data );
+					editor.execute( this.commandName, {
+						value: data.value,
+						batch: this._undoStepBatch
+					} );
 				}
 
 				if ( data.source !== 'colorPicker' ) {
 					editor.editing.view.focus();
 				}
 			} );
 
+			this.colorTableView.on<ColorTableShowColorPickerEvent>( 'showColorPicker', () => {
+				this._undoStepBatch = editor.model.createBatch();
+			} );
+
 			this.colorTableView.on<ColorTableCancelEvent>( 'cancel', () => {
-				editor.execute( this.commandName, {
-					value: colorSavedUponDropdownOpen
-				} );
-				this.colorTableView!.selectedColor = colorSavedUponDropdownOpen;
+				if ( this._undoStepBatch!.operations.length ) {
+					editor.execute( 'undo', this._undoStepBatch );
+				}
 
 				editor.editing.view.focus();
 			} );
@@ -173,10 +185,6 @@ export default class ColorUI extends Plugin {
 				}
 
 				if ( isVisible ) {
-					if ( hasColorPicker ) {
-						colorSavedUponDropdownOpen = dropdownView.colorTableView!.selectedColor!;
-					}
-
 					if ( documentColorsCount !== 0 ) {
 						this.colorTableView!.updateDocumentColors( editor.model, this.componentName );
 					}

packages/ckeditor5-font/tests/fontcommand.js

@@ -236,6 +236,15 @@ describe( 'FontCommand', () => {
 			expect( getData( model ) ).to.equal( '<paragraph>a[<$text font="foo">bcfo]obar</$text>xyz</paragraph>' );
 		} );
 
+		it( 'should use provided batch', () => {
+			setData( model, '<paragraph>a[bc<$text font="foo">fo]obar</$text>xyz</paragraph>' );
+			const batch = model.createBatch();
+			const spy = sinon.spy( model, 'enqueueChange' );
+
+			command.execute( { value: '#f00', batch } );
+			sinon.assert.calledWith( spy, batch );
+		} );
+
 		describe( 'should cause firing model change event', () => {
 			let spy;
 

packages/ckeditor5-font/tests/integration.js

@@ -243,5 +243,23 @@ describe( 'Integration test Font', () => {
 
 			expect( getData( model ) ).to.equal( '<paragraph>[<$text fontColor="lab( 18% -17 7 )">foo</$text>]</paragraph>' );
 		} );
+
+		it( 'should undo all changes done in a batch with a single step', () => {
+			setModelData( model, '<paragraph>[foo]</paragraph>' );
+
+			const dropdown = editor.ui.componentFactory.create( 'fontColor' );
+
+			dropdown.isOpen = true;
+			dropdown.colorTableView.fire( 'showColorPicker' );
+
+			// Execute multiple color changes.
+			dropdown.colorTableView.colorPickerPageView.colorPickerView.color = '#113322';
+			dropdown.colorTableView.colorPickerPageView.colorPickerView.color = '#654321';
+			dropdown.colorTableView.colorPickerPageView.colorPickerView.color = '#123456';
+
+			editor.commands.get( 'undo' ).execute();
+
+			expect( getData( model ) ).to.equal( '<paragraph>[foo]</paragraph>' );
+		} );
 	} );
 } );

packages/ckeditor5-font/tests/ui/colorui.js

@@ -11,6 +11,7 @@ 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';
+import Undo from '@ckeditor/ckeditor5-undo/src/undo';
 import { add as addTranslations, _clear as clearTranslations } from '@ckeditor/ckeditor5-utils/src/translation-service';
 import { setData as setModelData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model';
 
@@ -72,7 +73,7 @@ describe( 'ColorUI', () => {
 
 		return ClassicTestEditor
 			.create( element, {
-				plugins: [ Paragraph, TestColorPlugin ],
+				plugins: [ Paragraph, TestColorPlugin, Undo ],
 				testColor: testColorConfig
 			} )
 			.then( newEditor => {
@@ -194,18 +195,52 @@ describe( 'ColorUI', () => {
 				sinon.assert.notCalled( spy );
 			} );
 
-			it( 'should cancel changes', () => {
-				dropdown.isOpen = false;
-
-				dropdown.colorTableView.selectedColor = '#ff0000';
+			it( 'should undo changes', () => {
+				const spyUndo = sinon.spy( editor.commands.get( 'undo' ), 'execute' );
 
 				dropdown.isOpen = true;
+				testColorPlugin.colorTableView.fire( 'showColorPicker' );
+
+				dropdown.colorTableView.selectedColor = 'hsl( 0, 0%, 100% )';
 
-				dropdown.colorTableView.selectedColor = '#123456';
+				editor.commands.get( 'testColorCommand' ).isEnabled = true;
+
+				dropdown.colorTableView.fire( 'execute', {
+					value: 'hsl( 210, 65%, 20% )',
+					source: 'colorPicker'
+				} );
 
 				dropdown.colorTableView.colorPickerPageView.cancelButtonView.fire( 'execute' );
 
-				expect( dropdown.colorTableView.selectedColor ).to.equal( '#ff0000' );
+				sinon.assert.calledOnce( spyUndo );
+			} );
+
+			it( 'should create new batch when color picker is showed', () => {
+				dropdown.isOpen = true;
+				testColorPlugin.colorTableView.colorGridsPageView.colorPickerButtonView.fire( 'execute' );
+
+				dropdown.colorTableView.selectedColor = '#000000';
+
+				editor.commands.get( 'testColorCommand' ).isEnabled = true;
+
+				dropdown.colorTableView.fire( 'execute', {
+					value: 'hsl( 210, 65%, 20% )',
+					source: 'colorPicker'
+				} );
+
+				expect( testColorPlugin._undoStepBatch.operations.length,
+					'should have 1 change in batch' ).to.equal( 1 );
+
+				dropdown.colorTableView.fire( 'execute', {
+					value: 'hsl( 110, 60%, 12% )',
+					source: 'saveButton'
+				} );
+
+				dropdown.isOpen = true;
+				testColorPlugin.colorTableView.colorGridsPageView.colorPickerButtonView.fire( 'execute' );
+
+				expect( testColorPlugin._undoStepBatch.operations.length,
+					'should have 0 changes in batch' ).to.equal( 0 );
 			} );
 
 			it( 'should avoid call the command multiple times', () => {