ckeditor5 icon indicating copy to clipboard operation
ckeditor5 copied to clipboard

Create dropdown menu component

Open Mati365 opened this issue 1 year ago • 1 comments

Feature (ui): Create dropdown menu component.

Part of https://github.com/ckeditor/ckeditor5/issues/16311.


Other PRs

Focus change PR: https://github.com/ckeditor/ckeditor5/pull/16642 Search integration PR: https://github.com/ckeditor/ckeditor5/pull/16314 AI Integration PR: https://github.com/cksource/ckeditor5-commercial/pull/6213

Screens

⚠️ Search function is introduced in another PR.

Menu Scrolling RTL

Example usage

const view = new DropdownMenuRootListView( editor, [
  {
     menu: 'Menu 1',
     children: [
       // Initialize using definition object.
       {
          label: 'Menu Item 1.1',
          onExecute: () => { ... }
       },
       // Initialize using class instance.
       new DropdownMenuListItemButtonView( locale, 'Item' ),
       new ListSeparatorView( locale ),
       // Submenu initialization.
       {
         menu: 'Menu 1.1',
         children: [
           new DropdownMenuListItemButtonView( locale, 'Nested Item' ),
         ]
       }
     ] 
  } 
] );

// Somewhere later in the code.
view.factory.appendChild( {
  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' );
customInstance.factory.appendChild( {
  menu: 'Sub menu',
  children: [
    new DropdownMenuListItemButtonView( locale, 'Item 3' ),
  ]
} );

// Walk over dropdown content. 
customInstance.walk( {
    Menu: () => { ... },
    Default: () => { ... }
} );

view.factory.appendChild( 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' );
  1. 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. The one nice side effect of such approach is the ability to create dump() functionality, which allows to easily serialize whole menu to simple pseudo-HTML form, which is super easy to test in other integrations.
const view = new DropdownMenuRootListView( editor, [
  {
     menu: 'Menu 1',
     children: [
       // initialize using definition object
       {
          label: 'Menu Item 1.1',
          onExecute: () => { ... }
       },
       // initialize using object
       new DropdownMenuListItemButtonView( locale, 'Item' ),
       new ListSeparatorView( locale ),
       // submenu initialization
       {
         menu: 'Menu 1.1',
         children: [
           new DropdownMenuListItemButtonView( locale, 'Nested Item' ),
         ]
       }
     ] 
  } 
] );

// 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 );
  1. Horizontal menu bar and vertical positioning dropdown button positioning functions have been removed.
  2. DSL format of menu definition is simplified and groups menu attributes has been removed. Now it is possible to pass ListSeparatorView manually.
  3. Icons are optional, though it is possible to allocate space for missing if it is not present (by default it is disabled).
  4. Event delegation moved from root menu list to normal menu instances.
  5. 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. ~Scrolling in large sub-menus is missing.~
  3. It might look awful on mobile.

Useful links and discussions

  1. https://mdn.github.io/dom-examples/popover-api/nested-popovers/
  2. https://blog.logrocket.com/use-css-anchor-positioning/
  3. https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/popover
  4. https://github.com/ckeditor/ckeditor5/issues/5514#issuecomment-507230949

Mati365 avatar Jun 28 '24 06:06 Mati365

@scofalik

menu/definition directory - please put all things into one file, they are all mostly used only by the factory util, with I thing just one exception. Remove "definition" factory and move the singular file into menu/ directory.

👍🏻 done

Change the factory class name and file name to something like DropdownMenuFactory -- it does not create a definition, it creates a menu.

👍🏻 done

Same with getCurrentlyVisibleMenusElements().

At this moment, it's kinda crucial to get all DOM elements that belong to the menu. Without such helper, it's impossible to render dropdown menu inside dropdown. Check this place for additional PR that implements AI support: https://github.com/cksource/ckeditor5-commercial/pull/6213/files#diff-c3e2cfc5f2b64e23fc4bb25cdb3e76e1615a85a1fd0fac616a71d2271cb1456cR165

preloadAllMenus() seems not to be used anywhere besides tests (at least that what my IDE tells me).

As we discussed, it's used in search implementation right here. walk function cannot walk through a menu that has collapsed menu entries.

👍🏻 I moved this code to search PR.

To build .menus I am pretty sure we can do it with a simple tree-search algorithm that uses while and an array to store results. We won't need to use several utils, types and files.

While I agree that this solution is good enough in terms of making the tree flat, or simply iteration over such tree, it makes it difficult to perform search using such approach. We need to know when we enter to the node, when we leave, to construct a proper search tree. Please check this file. I used API that is quite similar to ast-types, so I think it's quite normal way to solve that kind of the problems.

⚠️ Partially done. I simplified the tree walker and moved more sophisticated logic to search PR.

and there is .walk() which isn't used anywhere

It's used especially in testing, and AI integration to enable or disable menu entries of specific type. It's used here.

For items entries, we may need a "path" to them, i.e. an array of menus in which an item is. But I think that will be needed for the search only, and I purposely don't mention any search related things when discussing what should be dropped from this PR. As I mentioned multiple times, this PR should be only about the nested menus functionality.

⚠️ So that's why the flatten tree method was added :)

tree is not necessary

❌ It is necessary, and it's not only my opinion. It was originally added in the menu bar (here) for testing purposes. It allows us to compare tree structures created by definition quickly. At the same time, I agree that there are a lot of other, simpler approaches, to do that but I believe that it's the only one that is the most elegant one because filtering, flattening, and other tree-things can be done using the most common algorithms that are basically copy & paste from other implementations.

Mati365 avatar Jul 02 '24 12:07 Mati365

Damn this would be good, currently looking for exactly this

Joshuajrodrigues avatar Apr 04 '25 10:04 Joshuajrodrigues

@Joshuajrodrigues It's already present on master and was deployed some time ago. In a little bit of different form (without search bar).

Mati365 avatar Apr 04 '25 10:04 Mati365