Application menu bar #15893

oleq · 2024-02-21T10:29:11Z

Suggested merge commit message (convention)

Feature (ui,editor-classic,editor-decoupled): Implemented the application menu bar that contains various options and commands for controlling and navigating the editor. Closes #15894.

MINOR BREAKING CHANGE (upload): The FileDialogButtonView class has been moved from ckeditor5-upload to ckeditor5-ui. Please update your import paths accordingly (was: import { FileDialogButtonView } from 'ckeditor5/src/upload.js';, is: import { FileDialogButtonView } from 'ckeditor5/src/ui.js';).

MINOR BREAKING CHANGE (theme-lark): The default vertical spacing around ButtonView in ListItemView (--ck-list-button-padding) has been reduced for better readability. This affects the presentation of various editor features that use this type of UI (headings, font size, font family, etc.). You can restore the previous value by setting --ck-list-button-padding: calc(.2 * var(--ck-line-height-base) * var(--ck-font-size-base)) calc(.4 * var(--ck-line-height-base) * var(--ck-font-size-base)); in your custom styles sheet.

Additional information

Requires https://github.com/cksource/ckeditor5-commercial/pull/6072

…n the DropdownView ecosystem to prevent breaking changes.

… obsolete check button view class.

…elow the menu bar.

Internal (ui): Implemented the MenuBarView component (and dependencies). Used the prototype in the ClassicEditor along with several example integrations.

Feature (block-quote): Added menu bar integration to the block quote package.

Internal (horizontal-line): Added menu bar integration. Related to #15894.

Internal (page-break): Added menu bar integration. Related to #15894.

Internal (remove-format): Added menu bar integration. Related to #15894.

…enu bar config normalization.

Internal (ui,editor-classic): Streamlined the menu bar config normalization. Made the feature an opt-in in the `ClassicEditor`. Related to #15894.

Internal (theme-lark): Fixed the spacing in buttons without icons.

Internal (core): Added menu bar entry in the a11y dialog.

…ermine correct behaviors of the menu bar in a localized editor.

Internal (ui): RTL support for the menu bar.

Internal (document-outline): Added Table of contents button in default menu bar structure. Internal (restricted-editing): Refactored the restricted editing buttons in the menu bar.

Internal (editor-decoupled): Add menu bar to decoupled editor. Related to #15894.

…s. (#16126) Internal (ui): Defined the final structure of the menu bar. Localized sub-menu labels.

Co-authored-by: Michał Remiszewski <[email protected]> Co-authored-by: Aleksander Nowodzinski <[email protected]> Co-authored-by: Michał Remiszewski <[email protected]>

…-core.

scofalik

I went through the PR and left some documentation-related comments and other loose thoughts, that we don't need to improve unless you strongly agree.

But, there is one thing that I don't like and it is the number of MenuBar* components that exist and need to be used to craft a UI component for menu bar. E.g. MenuBarMenuListItemView seems like an unnecessary complication for a plugin developer. If the component is really needed, maybe we could at least hide it from the developer.

Instead, the way I see it is that we have a small, defined set of items that can end up in the menu bar lists. Contrary to the toolbar, we don't want flexibility here. We agree that not everything makes sense in the menu bar. So, as I said, we have small lists of components to choose from.

Instead of crafting the components from the building blocks, it would be better (and safer!) to just give the developers the components. AFAICS we have four components now:

button -- a "leaf" in the menus structure, you click it and it makes an action and closes the menu bar, some of them are "toggles" (can be enabled, and have aria-checked attribute), this could be a parameter in the constructor,
file button -- probably it must be a separate component from the regular button,
sub-menu -- a button that opens a new menu, all I should care about is providing components for the menu, not creating the menu bar list view by hand and link it with other components (I mean things like event delegations),
dropdown -- a button that opens an arbitrary view in a dropdown, for things like color picker or table insertion grid, all I should care about is to provide the view.

So, we should have four "top-level" classes to be used by a plugin developer, with precise parameters in constructors and standarized behaviors.

I know that it sounds easy but then may get complicated in the details, especially I believe focus handling and keystrokes handling may be difficult for the dropdown component. But we should strive for the above, we should make it easy for plugin developers to provide the components. Right now, if I were to provide such component, I would be all in fear that I forgot to set some property, bind some observable, delegate an event, etc. Basically it is impossible to create a correct component without copy pasting existing, similar one.

(PS. this would not be a breaking change - we can build it over the existing components and update integrations as we go, depending on how much time we have.)

scofalik · 2024-04-02T12:43:39Z

packages/ckeditor5-alignment/src/alignmentui.ts

@@ -120,35 +173,59 @@ export default class AlignmentUI extends Plugin {
 	}

 	/**
-	 * Helper method for initializing the button and linking it with an appropriate command.
+	 * Creates a menu for all alignment options to use either in menu bar.


Something is wrong with this line. "either" is unnecessary or you wanted to write something more here.

scofalik · 2024-04-02T13:02:49Z

packages/ckeditor5-alignment/src/alignmentui.ts

+				const listItemView = new MenuBarMenuListItemView( locale, menuView );
+				const buttonView = new MenuBarMenuListItemButtonView( locale );


Do these two must be separate classes? Couldn't the button extend the list item view? I would lean towards limiting the number of different classes and wrapper-components. It seems like there's a lot of boilerplate to create when creating menu items, which are quite simple and limited by design (in contrary to toolbar components where we want to allow including anything).

scofalik · 2024-04-02T15:01:35Z

packages/ckeditor5-core/src/editor/editorconfig.ts

+	 * By default, a {@link module:ui/menubar/utils#DefaultMenuBarItems default set of items} is used that already includes
+	 * **all core editor features**. For you convenience, there are `config.menuBar.addItems` and


Suggested change

* By default, a {@link module:ui/menubar/utils#DefaultMenuBarItems default set of items} is used that already includes

* **all core editor features**. For you convenience, there are `config.menuBar.addItems` and

* By default, a {@link module:ui/menubar/utils#DefaultMenuBarItems default set of items} is used. It includes

* **all loaded official editor features**. For you convenience, there are `config.menuBar.addItems` and

scofalik · 2024-04-02T15:22:52Z

packages/ckeditor5-core/src/editor/editorconfig.ts

+	 * * `menu` &ndash; A {@link module:ui/menubar/menubarview#MenuBarMenuDefinition definition of a menu} that should be added to
+	 * the menu bar,
+	 * * `group` &ndash; A {@link module:ui/menubar/menubarview#MenuBarMenuGroupDefinition definition of a button group} that should be


Let's change the order here so we will go from smallest to biggest component (or the other way). Right now it is just mixed.

scofalik · 2024-04-02T15:23:53Z

packages/ckeditor5-core/src/editor/editorconfig.ts

+	 * * `'end'` &ndash; Adds a top-level menu (e.g. "Format", "Insert", etc.) at the end of the menu bar,
+	 * * `'end:GROUP_OR_MENU'` &ndash; Adds a button/group at the end of the specific group/menu,
+	 * * `'after:BUTTON_OR_GROUP_OR_MENU'` &ndash; Adds a button/group/menu right after the specific button/group/menu,
+	 * * `'before:BUTTON_OR_GROUP_OR_MENU'` &ndash; Adds a button/group/menu right after the specific button/group/menu.


Suggested change

* * `'before:BUTTON_OR_GROUP_OR_MENU'` – Adds a button/group/menu right after the specific button/group/menu.

* * `'before:BUTTON_OR_GROUP_OR_MENU'` – Adds a button/group/menu right before the specific button/group/menu.

scofalik · 2024-04-10T11:51:20Z

packages/ckeditor5-ui/src/menubar/menubarmenubuttonview.ts

+ * A menu {@link module:ui/menubar/menubarmenuview~MenuBarMenuView#buttonView} class. Buttons like this one
+ * open both top-level bar menus as well as sub-menus.


* A menu {@link module:ui/menubar/menubarmenuview~MenuBarMenuView#buttonView} class. These buttons * are used both for top-level bar menus as well as in sub-menus.

scofalik · 2024-04-10T13:42:35Z

packages/ckeditor5-ui/src/menubar/menubarview.ts

+			/**
+			 * Adding unsupported components to the {@link module:ui/menubar/menubarview~MenuBarView} is not possible.
+			 *
+			 * A component should be either a {@link module:ui/menubar/menubarmenuview~MenuBarMenuView} (sub-menu) or a
+			 * {@link module:ui/menubar/menubarmenulistitembuttonview~MenuBarMenuListItemButtonView} (button).


This doesn't mention file dialog button view -- is this on purpose?

scofalik · 2024-04-10T13:57:29Z

packages/ckeditor5-ui/src/menubar/menubarmenuview.ts

+	/**
+	 * The name of the position of the {@link #panelView}, relative to the menu.
+	 *
+	 * **Note**: The value is updated each time the panel gets {@link #isOpen open}.


* **Note**: The value is updated each time the panel gets {@link #isOpen opened}.

scofalik · 2024-04-10T16:02:19Z

packages/ckeditor5-ui/src/menubar/menubarmenupanelview.ts

+
+/**
+ * The names of the positions of the {@link module:ui/menubar/menubarmenupanelview~MenuBarMenuPanelView}.
+ *
+ * They are reflected as CSS class suffixes on the panel view element.
+ */
+export type MenuBarMenuPanelPosition = 'se' | 'sw' | 'ne' | 'nw' | 'w' | 'e';


Didn't we have these registered in a different place?

scofalik · 2024-04-10T16:17:00Z

packages/ckeditor5-alignment/src/alignmentui.ts

-		return buttonView;
+				buttonView.extendTemplate( {
+					attributes: {
+						'aria-checked': buttonView.bindTemplate.to( 'isOn' )


I wonder if this could be just default behavior of a button. But then isOn would need to be able to take undefined state.

oleq and others added 30 commits January 29, 2024 16:25

WiP.

40c5f7a

WIP.

03be1d5

Added headings to the menu bar.

adbbe3f

Basic a11y for the menu bar.

98c1941

Merge branch 'master' into ck/app-menu-bar-prototype

68eca5e

Decoupled MenuBarView from DropdownView ecosystem. Reverted changes i…

09c3c4e

…n the DropdownView ecosystem to prevent breaking changes.

Simplified the active state of items in the menu bar. Removed the now…

fdd875b

… obsolete check button view class.

Adjustment to menu CSS for improved :hover styles.

d538db7

Streamlined event bus in the MenuBarView.

4fa1899

Code refactoring and docs.

0ffcda9

Defer MenuBarView initialization until it gets open.

83970d2

Adjusted rounded corners in the menu bar.

2b3df85

Code refactoring.

f0381a5

Added extra menu bar menu positions for tight layouts with no space b…

384cd86

…elow the menu bar.

Merge branch 'master' into ck/app-menu-bar-prototype

b27b5b6

Merge pull request #15806 from ckeditor/ck/app-menu-bar-prototype

138e0e7

Internal (ui): Implemented the MenuBarView component (and dependencies). Used the prototype in the ClassicEditor along with several example integrations.

Add block quote integration.

e71c4c0

Code clean-up.

4b1559e

Merge pull request #15904 from ckeditor/ck/app-menu-bar-block-quote

2a06f3b

Feature (block-quote): Added menu bar integration to the block quote package.

Add code integration.

1a5d98d

Update code integration.

d7fac84

Add subscript integration.

bce4378

Add superscript integration.

420c6dd

Fix docs.

56988ad

Remove tooltip from menu buttons.

7fbd653

Merge branch 'master' into ck/app-menu-bar

e3c8598

Add list integration.

3e907b7

Add menu bar integration to horizontal line. (#15910)

6586704

Internal (horizontal-line): Added menu bar integration. Related to #15894.

Add page break integration. (#15934)

2f95bb3

Internal (page-break): Added menu bar integration. Related to #15894.

Add remove format integration. (#15937)

db6a257

Internal (remove-format): Added menu bar integration. Related to #15894.

oleq and others added 24 commits March 28, 2024 11:01

Merge branch 'ck/app-menu-bar' into ck/app-menu-bar-opt-in

3c96750

Fixed the spacing in buttons without the icons.

31ea9a1

Streamlined menu bar opt-in configuration in the Classic editor and m…

214ad2b

…enu bar config normalization.

Merge pull request #16105 from ckeditor/ck/app-menu-bar-opt-in

9d197b2

Internal (ui,editor-classic): Streamlined the menu bar config normalization. Made the feature an opt-in in the `ClassicEditor`. Related to #15894.

Merge branch 'ck/app-menu-bar' into ck/app-menu-bar-button-margins

1c89898

Tests: Enabled the menu bar in critical manual tests.

eb8e64e

Merge pull request #16122 from ckeditor/ck/app-menu-bar-button-margins

0b6685b

Internal (theme-lark): Fixed the spacing in buttons without icons.

Merge branch 'ck/app-menu-bar' into ck/app-menu-bar-a11y-entry

5d70f84

Merge pull request #16118 from ckeditor/ck/app-menu-bar-a11y-entry

247cbef

Internal (core): Added menu bar entry in the a11y dialog.

Merge branch 'ck/app-menu-bar' into ck/app-menu-bar-rtl

86dd7c0

Used #uiLanguageDirection instead of #contentLanguageDirection to det…

f9fe05a

…ermine correct behaviors of the menu bar in a localized editor.

Merge pull request #16115 from ckeditor/ck/app-menu-bar-rtl

d73d3e3

Internal (ui): RTL support for the menu bar.

Merge branch 'ck/app-menu-bar' into ck/app-menu-bar-toc

6687684

Tests: Enabled the menu bar in the restricted editing manual test.

8cff45e

Merge pull request #16112 from ckeditor/ck/app-menu-bar-toc

0994381

Internal (document-outline): Added Table of contents button in default menu bar structure. Internal (restricted-editing): Refactored the restricted editing buttons in the menu bar.

Merge branch 'ck/app-menu-bar' into ck/app-menu-bar-decoupled-editor

0ff9925

Aligned the implementation to the latest API.

b8a27b8

Docs.

2ea9c6a

Merge pull request #16116 from ckeditor/ck/app-menu-bar-decoupled-editor

298efe0

Internal (editor-decoupled): Add menu bar to decoupled editor. Related to #15894.

CSS fixes.

291a56a

Defined the final structure of the menu bar. Localized sub-menu label…

68225ff

…s. (#16126) Internal (ui): Defined the final structure of the menu bar. Localized sub-menu labels.

Removed an obsolete group from the default config.

bf550c0

Merge branch 'master' into ck/app-menu-bar

92116d1

Tests: Unit tests for the menu bar

b0afc68

Co-authored-by: Michał Remiszewski <[email protected]> Co-authored-by: Aleksander Nowodzinski <[email protected]> Co-authored-by: Michał Remiszewski <[email protected]>

DawidKossowski marked this pull request as ready for review April 2, 2024 17:20

oleq added 2 commits April 2, 2024 19:49

Merge branch 'master' into ck/app-menu-bar

b80077a

Moved the "Accept" contexts.json entry from ckeditor5-ui to ckeditor5…

805a4c7

…-core.

oleq merged commit f072048 into master Apr 2, 2024
5 of 6 checks passed

oleq deleted the ck/app-menu-bar branch April 2, 2024 18:30

scofalik reviewed Apr 10, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Application menu bar #15893

Application menu bar #15893

oleq commented Feb 21, 2024 •

edited

Loading

scofalik left a comment •

edited

Loading

scofalik Apr 2, 2024

scofalik Apr 2, 2024

scofalik Apr 2, 2024

scofalik Apr 2, 2024

scofalik Apr 2, 2024

scofalik Apr 10, 2024

scofalik Apr 10, 2024

scofalik Apr 10, 2024

scofalik Apr 10, 2024

scofalik Apr 10, 2024

		const listItemView = new MenuBarMenuListItemView( locale, menuView );
		const buttonView = new MenuBarMenuListItemButtonView( locale );

		* By default, a {@link module:ui/menubar/utils#DefaultMenuBarItems default set of items} is used that already includes
		* all core editor features. For you convenience, there are `config.menuBar.addItems` and

	* * `'before:BUTTON_OR_GROUP_OR_MENU'` – Adds a button/group/menu right after the specific button/group/menu.
	* * `'before:BUTTON_OR_GROUP_OR_MENU'` – Adds a button/group/menu right before the specific button/group/menu.

		* A menu {@link module:ui/menubar/menubarmenuview~MenuBarMenuView#buttonView} class. Buttons like this one
		* open both top-level bar menus as well as sub-menus.

Application menu bar #15893

Application menu bar #15893

Conversation

oleq commented Feb 21, 2024 • edited Loading

Suggested merge commit message (convention)

Additional information

scofalik left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oleq commented Feb 21, 2024 •

edited

Loading

scofalik left a comment •

edited

Loading