Create dropdown menu component

[Mati365]

Feature (ui): Create dropdown menu component. Closes #16311.

---

AI Integration

Commercial PR: https://github.com/cksource/ckeditor5-commercial/pull/6213

Screens

Example usage

const view = new DropdownMenuRootListView( editor, [
  {
     menu: 'Menu 1',
     children: [
       new DropdownMenuListItemButtonView( locale, 'Item' ),
       new ListSeparatorView( locale ),
       {
         menu: 'Menu 1.1',
         children: [
           new DropdownMenuListItemButtonView( locale, 'Nested Item' ),
         ]
       }
     ] 
  } 
] );

// Somewhere later in the code.
view.appendTopLevelChild( {
  menu: 'Menu 2',
  children: [
    new DropdownMenuListItemButtonView( locale, 'Item 2' )
  ]
} );

// It is also possible to register custom menu using it's instance and modify it.
const customInstance = new DropdownMenuView( editor, 'Menu' );

view.appendTopLevelChild( customInstance );
view.appendMenuChildren( 
  [ new DropdownMenuListItemButtonView( locale, 'Item 3' ) ],
  customInstance
] );

view.render(); 

Changes performed on menubar implementation

1. Generate menus property dynamically instead of storing it as array class member. It is possible to register already created DropdownMenuView instance that was created outside of menu. Such instance might have items added or deleted depending on external integration logic (for example, adding button menu entry).

Example:

const customMenuInstance = new DropdownMenuView( editor, 'My custom menu' );
const view = new DropdownMenuRootListView( locale, [
  {
     menu: 'Menu 1',
     children: [
       // ... other menu items
       customMenuInstance
     ] 
  } 
] );

// ... later in the code ...
const newSubMenu =  new DropdownMenuView( editor, 'My submenu menu' );

newSubMenu.menuItems.addMany( [   
    new DropdownMenuListItemButtonView( locale, 'Your Searchable Item' ),
    new ListSeparatorView( locale ),
] );

// It's optional to use `appendMenuChildren` exported by root menu class. 
// New items can be added manually, without need to access root menu instance.
customMenuInstance.menuItems.add(
   new DropdownMenuListItemView( locale, menuInstance, newSubMenu)
);

// ... later in filter implementation ... 
// It should return 1 item that was added to `newSubMenu` because
// `DropdownMenuListItemButtonView` is automatically discovered while `tree` construction.
filterView.menuView.search( 'Your Searchable Item' );

3. Added tree getter that allows to traverse through whole menu that allows for lookup for specific menu entries in tests, filtering menu or creating another instance of dropdown menu with mapped definition.

const view = new DropdownMenuRootListView( editor, [
  {
     menu: 'Menu 1',
     children: [
       new DropdownMenuListItemButtonView( locale, 'Item' ),
       new ListSeparatorView( locale ),
       {
         menu: 'Menu 1.1',
         children: [
           new DropdownMenuListItemButtonView( locale, 'Nested Item' ),
         ]
       }
     ] 
  } 
] );

// Dump whole tree for debug / testing purposes. 
console.info( view.dump() ); 

// Walk through menu and do some stuff on specific items.
view.walk( {
    Item: ( { node: { search } } ) => {
      if ( search.raw.includes( 'cats' ) ) {
         console.info( 'I see cat here...' ); 
      }
    }
} );

// Let's access tree and menu directly.
console.info( view.tree, view.menus );

5. Horizontal menu bar and vertical positioning dropdown button positioning functions have been removed.
6. DSL format of menu definition is simplified and groups menu attributes has been removed. Now it is possible to pass ListSeparatorView manually.
7. Icons are optional, though it is possible to allocate space for missing if it is not present (by default it is disabled).
8. Event delegation moved from root menu list to normal menu instances.
9. Menu panel views are now mounted to ck-body-wrapper.

Drawbacks

1. tree / menus getter calls might be expensive for huge menus (> 500 entries). It's not real case and at this moment there are no actions required to be performed in this scope. It is fixed by adding optional lazy initialization of menus.
2. Mobile view is not implemented nor tested. It's better to do that in further iterations.
3. Scrolling in large sub-menus is missing.

[coderabbitai]

Actionable comments posted: 1

Out of diff range and nitpick comments (6)

packages/ckeditor5-ui/src/dropdown/menu/dropdownmenurootlistview.ts (6)

50-66: Add documentation to the ofConfig method to explain its parameters and behavior.

---

71-95: Add documentation to the _createMenu method to clarify its purpose and the parameters it accepts.

---

98-139: Add documentation to the _createMenuItems method to describe its functionality and parameters.

---

145-181: Add documentation to the _createMenuItemContentFromFactory method to explain its behavior and parameters.

---

187-213: Add documentation to the _registerMenuTree method to detail its functionality and parameters.

---

218-229: Add documentation to the registerMenu method to explain its behavior and parameters.

[coderabbitai]

Actionable comments posted: 3

[coderabbitai]

Actionable comments posted: 2

[coderabbitai]

Actionable comments posted: 3

[coderabbitai]

Actionable comments posted: 0

Out of diff range and nitpick comments (1)

packages/ckeditor5-ui/src/bindings/clickoutsidehandler.ts (1)

[!TIP]
Codebase Verification

Several files in the codebase still do not utilize the new options parameter in clickOutsideHandler. It's important to review whether this parameter should be used in these instances or if its current usage is intentional.

• packages/ckeditor5-ui/src/toolbar/block/blocktoolbar.ts
• packages/ckeditor5-ui/src/menubar/utils.ts
• packages/ckeditor5-ui/src/dropdown/menu/utils.ts
• packages/ckeditor5-ui/src/dropdown/utils.ts
• packages/ckeditor5-table/src/tableproperties/tablepropertiesui.ts
• packages/ckeditor5-table/src/tablecellproperties/tablecellpropertiesui.ts
• packages/ckeditor5-mention/src/mentionui.ts
• packages/ckeditor5-link/src/linkui.ts
• packages/ckeditor5-image/src/imagetextalternative/imagetextalternativeui.ts
• packages/ckeditor5-image/src/imageresize/imagecustomresizeui.ts

Analysis chain

Line range hint 29-55: LGTM! Ensure the new options parameter integrates well with other parts of the system.

Scripts Executed

The following scripts were executed for the analysis:

---

Script:

#!/bin/bash
# Description: Verify integration of the `options` parameter in `clickOutsideHandler` with other parts of the system.

# Test: Search for usages of `clickOutsideHandler` and check if `options` is being passed correctly.
rg --type typescript $'clickOutsideHandler'

Length of output: 77

---

---

Script:

#!/bin/bash
# Description: Verify integration of the `options` parameter in `clickOutsideHandler` with other parts of the system.

# Test: Search for usages of `clickOutsideHandler` and check if `options` is being passed correctly.
rg --type ts 'clickOutsideHandler'

Length of output: 2782

---

---

Script:

#!/bin/bash
# Description: Extract the lines of code where `clickOutsideHandler` is called to check if the `options` parameter is being passed correctly.

# Extract the relevant lines from each file where `clickOutsideHandler` is called.
rg --type ts --context 2 'clickOutsideHandler\('

Length of output: 4937

[coderabbitai]

Actionable comments posted: 4

Out of diff range and nitpick comments (5)

packages/ckeditor5-ui/src/dropdown/menu/dropdownmenufilteredlistview.ts (3)

17-17: Add a description for the TODO comment to clarify pending tasks or remove if not applicable.

---

21-23: Add a description for the TODO comment to clarify pending tasks or remove if not applicable.

---

31-50: Ensure proper documentation for the constructor and its parameters for better maintainability.

packages/ckeditor5-ui/src/dropdown/menu/dropdownmenurootlistview.ts (2)

25-27: Add a description for the TODO comment to clarify pending tasks or remove if not applicable.

---

29-48: Ensure proper documentation for the constructor and its parameters for better maintainability.

[coderabbitai]

Actionable comments posted: 1

Out of diff range and nitpick comments (2)

packages/ckeditor5-ui/src/dropdown/menu/dropdownmenulistfilteredview.ts (1)

17-23: Complete the TODO comments with appropriate descriptions or remove them if they are no longer needed.

packages/ckeditor5-ui/src/dropdown/menu/utils.ts (1)

45-64: The implementation of toggleMenusAndFocusItemsOnHover correctly handles menu toggling on hover. However, consider resolving the TODO comment regarding whether to focus on hover, as this can affect user experience.

[coderabbitai]

Actionable comments posted: 1

[coderabbitai]

Actionable comments posted: 0

Out of diff range and nitpick comments (1)

packages/ckeditor5-ui/src/dropdown/menu/search/createtreefromflattenmenuviews.ts (1)

18-74: The function createTreeFromFlattenMenuViews is well-implemented with clear logic for constructing a tree from a flat list of menus. However, it heavily relies on non-null assertions (e.g., menu.buttonView.label! and menu.panelView.children!.first). It would be beneficial to add a comment clarifying that these values are expected to be non-null as per upstream guarantees.

[coderabbitai]

Actionable comments posted: 3

[coderabbitai]

Actionable comments posted: 1

[coderabbitai]

Actionable comments posted: 2

[coderabbitai]

Actionable comments posted: 12

[coderabbitai]

Actionable comments posted: 11

[coderabbitai]

Actionable comments posted: 0

Out of diff range and nitpick comments (1)

packages/ckeditor5-ui/tests/dropdown/menu/search/filterdropdownmenutreebyregexp.js (1)

1-4: Ensure the license header is up-to-date.

Verify that the license header is current and matches the project's standard format.

[coderabbitai]

Actionable comments posted: 1

[scofalik]

Thanks for providing an overview of changes introduced by the PR. I am in the middle of the review but I have some questions about what you wrote.

// Somewhere later in the code.
view.appendTopLevelChild( {
  menu: 'Menu 2',
  children: [
    new DropdownMenuListItemButtonView( locale, 'Item 2' )
  ]
} );

Do we need this for any specific functionality right now? The two features that for sure will use the new dropdowns are Merge fields and AI assistant. Both features have a static list of categories and items.

Much of what you wrote and explained is related to adding items dynamically. It wasn't in the scope of this component and maybe it can be simplified. One, because it's easier to review and close a PR when there's less code. Two, because we don't want to provide a public API too hastily. Three, because there's a high chance this code will be unified and/or refactored anyway in a few quarters. And four, because it might be beneficial in another area:

1. tree / menus getter calls might be expensive for huge menus (> 500 entries). It's not real case and at this moment there are no actions required to be performed in this scope.

We have a real case for this, as we have reports from some users that they are looking to use as many as hundreds of merge fields. So, I am concerned about performance here.

[Mati365]

@scofalik

Do we need this for any specific functionality right now? The two features that for sure will use the new dropdowns are Merge fields and AI assistant. Both features have a static list of categories and items.

Nope. These methods simplify mocking of the data in the tests and basically are tiny in terms of implementation (it's rather an alias for internal method). What do you think about marking them as internal?

We have a real case for this, as we have reports from some users that they are looking to use as many as hundreds of merge fields. So, I am concerned about performance here

:o afaik menu is not designed to hold that amount of data, we have to discuss that

[Mati365]

@scofalik Lazy initialization, menu cache and found items limit added. It means that we can handle ~800 items without drastically reducing startup / navigation performance.

There are few downsides:

1. The search engine needs to perform full initialization of all lazy menus during the first search (it's required to walk through all button or other "flat" non-menus entries).
2. I kept it behind the flag. Besides that, the performance is much better, it makes menus equally harder to unit test in other integrations because menu is not fully initialized. That's also why dump() exists, there is no need to perform "advanced" tests in integrations where only shallow menu structure must be checked.
3. The most crucial one - lazy initialization must be disabled in submenus where their buttons isEnabled state is controlled by external observables. The primary example might be AI Commands view with enabledCommandsIds. Each command button might be dynamically disabled, and search engine detects that situations.
4. CSS issues with large amount of items in submenus. It's not a trivial issue and as we discussed will be fixed later, in this particular PR.