Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add menu bar integration to all basic styles. #15909

Merged
merged 8 commits into from
Mar 1, 2024

Conversation

mremiszewski
Copy link
Contributor

Suggested merge commit message (convention)

Feature (basic-styles): Add menu bar integration for code, strikethrough, subscript and superscript. Related to #15894.


Additional information

For example – encountered issues, assumptions you had to make, other affected tickets, etc.

@mremiszewski mremiszewski changed the base branch from master to ck/app-menu-bar February 22, 2024 18:52
@mremiszewski mremiszewski requested a review from oleq February 22, 2024 19:12
Copy link
Member

@oleq oleq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is fine but what do you think about the following code? We're repeating tons of code and whenever something needs to be changed/readjusted we'll have to remember about all features. I'm not talking about a project-wide helper (because this is too much thinking) but a package-wide?

diff --git a/packages/ckeditor5-basic-styles/src/bold/boldui.ts b/packages/ckeditor5-basic-styles/src/bold/boldui.ts
index ded6a64fc8..0499314dfd 100644
--- a/packages/ckeditor5-basic-styles/src/bold/boldui.ts
+++ b/packages/ckeditor5-basic-styles/src/bold/boldui.ts
@@ -9,7 +9,7 @@
 
 import { Plugin, icons } from 'ckeditor5/src/core.js';
 import { ButtonView, MenuBarMenuListItemButtonView } from 'ckeditor5/src/ui.js';
-import type AttributeCommand from '../attributecommand.js';
+import { getButtonCreator } from '../utils.js';
 
 const BOLD = 'bold';
 
@@ -29,11 +29,20 @@ export default class BoldUI extends Plugin {
 	 */
 	public init(): void {
 		const editor = this.editor;
-		const command: AttributeCommand = editor.commands.get( BOLD )!;
+		const t = editor.locale.t;
+		const command = editor.commands.get( BOLD )!;
+		const createButton = getButtonCreator( {
+			editor,
+			commandName: BOLD,
+			plugin: this,
+			icon: icons.bold,
+			label: t( 'Bold' ),
+			keystroke: 'CTRL+B'
+		} );
 
 		// Add bold button to feature components.
 		editor.ui.componentFactory.add( BOLD, () => {
-			const buttonView = this._createButton( ButtonView );
+			const buttonView = createButton( ButtonView );
 
 			buttonView.set( {
 				tooltip: true
@@ -45,35 +54,7 @@ export default class BoldUI extends Plugin {
 		} );
 
 		editor.ui.componentFactory.add( 'menuBar:' + BOLD, () => {
-			return this._createButton( MenuBarMenuListItemButtonView );
-		} );
-	}
-
-	/**
-	 * TODO
-	 */
-	private _createButton<T extends typeof ButtonView | typeof MenuBarMenuListItemButtonView>( ButtonClass: T ): InstanceType<T> {
-		const editor = this.editor;
-		const locale = editor.locale;
-		const command: AttributeCommand = editor.commands.get( BOLD )!;
-		const view = new ButtonClass( editor.locale ) as InstanceType<T>;
-		const t = locale.t;
-
-		view.set( {
-			label: t( 'Bold' ),
-			icon: icons.bold,
-			keystroke: 'CTRL+B',
-			isToggleable: true
+			return createButton( MenuBarMenuListItemButtonView );
 		} );
-
-		view.bind( 'isEnabled' ).to( command, 'isEnabled' );
-
-		// Execute the command.
-		this.listenTo( view, 'execute', () => {
-			editor.execute( BOLD );
-			editor.editing.view.focus();
-		} );
-
-		return view;
 	}
 }
diff --git a/packages/ckeditor5-basic-styles/src/subscript/subscriptui.ts b/packages/ckeditor5-basic-styles/src/subscript/subscriptui.ts
index b077d9e7ba..7ba2e5930b 100644
--- a/packages/ckeditor5-basic-styles/src/subscript/subscriptui.ts
+++ b/packages/ckeditor5-basic-styles/src/subscript/subscriptui.ts
@@ -9,9 +9,9 @@
 
 import { Plugin } from 'ckeditor5/src/core.js';
 import { ButtonView, MenuBarMenuListItemButtonView } from 'ckeditor5/src/ui.js';
-import type AttributeCommand from '../attributecommand.js';
 
 import subscriptIcon from '../../theme/icons/subscript.svg';
+import { getButtonCreator } from '../utils.js';
 
 const SUBSCRIPT = 'subscript';
 
@@ -31,10 +31,18 @@ export default class SubscriptUI extends Plugin {
 	 */
 	public init(): void {
 		const editor = this.editor;
+		const t = editor.locale.t;
+		const createButton = getButtonCreator( {
+			editor,
+			commandName: SUBSCRIPT,
+			plugin: this,
+			icon: subscriptIcon,
+			label: t( 'Subscript' )
+		} );
 
 		// Add subscript button to feature components.
 		editor.ui.componentFactory.add( SUBSCRIPT, () => {
-			const buttonView = this._createButton( ButtonView );
+			const buttonView = createButton( ButtonView );
 			const command = editor.commands.get( SUBSCRIPT )!;
 
 			buttonView.set( {
@@ -48,34 +56,7 @@ export default class SubscriptUI extends Plugin {
 		} );
 
 		editor.ui.componentFactory.add( 'menuBar:' + SUBSCRIPT, () => {
-			return this._createButton( MenuBarMenuListItemButtonView );
-		} );
-	}
-
-	/**
-	 * Creates a button for subscript command to use either in toolbar or in menu bar.
-	 */
-	private _createButton<T extends typeof ButtonView | typeof MenuBarMenuListItemButtonView>( ButtonClass: T ): InstanceType<T> {
-		const editor = this.editor;
-		const locale = editor.locale;
-		const command: AttributeCommand = editor.commands.get( SUBSCRIPT )!;
-		const view = new ButtonClass( editor.locale ) as InstanceType<T>;
-		const t = locale.t;
-
-		view.set( {
-			label: t( 'Subscript' ),
-			icon: subscriptIcon,
-			isToggleable: true
+			return createButton( MenuBarMenuListItemButtonView );
 		} );
-
-		view.bind( 'isEnabled' ).to( command, 'isEnabled' );
-
-		// Execute the command.
-		this.listenTo( view, 'execute', () => {
-			editor.execute( SUBSCRIPT );
-			editor.editing.view.focus();
-		} );
-
-		return view;
 	}
 }
diff --git a/packages/ckeditor5-basic-styles/src/utils.ts b/packages/ckeditor5-basic-styles/src/utils.ts
new file mode 100644
index 0000000000..dee4864491
--- /dev/null
+++ b/packages/ckeditor5-basic-styles/src/utils.ts
@@ -0,0 +1,48 @@
+/**
+ * @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
+ */
+
+/**
+ * @module basic-styles/utils
+ */
+
+import type { Editor, Plugin } from 'ckeditor5/src/core.js';
+import type AttributeCommand from './attributecommand.js';
+import type { ButtonView, MenuBarMenuListItemButtonView } from 'ckeditor5/src/ui.js';
+
+/**
+ * TODO
+ */
+export function getButtonCreator( {
+	editor, commandName, plugin, icon, label, keystroke
+}: {
+	editor: Editor;
+	commandName: string;
+	icon: string;
+	label: string;
+	plugin: Plugin;
+	keystroke?: string;
+} ) {
+	return function<T extends typeof ButtonView | typeof MenuBarMenuListItemButtonView>( ButtonClass: T ): InstanceType<T> {
+		const command = editor.commands.get( commandName )! as AttributeCommand;
+		const view = new ButtonClass( editor.locale ) as InstanceType<T>;
+
+		view.set( {
+			label,
+			icon,
+			keystroke,
+			isToggleable: true
+		} );
+
+		view.bind( 'isEnabled' ).to( command, 'isEnabled' );
+
+		// Execute the command.
+		plugin.listenTo( view, 'execute', () => {
+			editor.execute( commandName );
+			editor.editing.view.focus();
+		} );
+
+		return view;
+	};
+}

@oleq oleq merged commit cce0db3 into ck/app-menu-bar Mar 1, 2024
1 check was pending
@oleq oleq deleted the ck/app-menu-bar-basic-styles branch March 1, 2024 13:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants