[IMP] add keyboard navigation to menus

src/actions/action.ts

@@ -1,6 +1,8 @@
 import { SpreadsheetChildEnv } from "@odoo/o-spreadsheet-engine/types/spreadsheet_env";
 import { Color } from "../types";
 
+export type MenuItemOrSeparator = Action | "separator";
+
 /*
  * An Action represent a menu item for the menus of the top bar
  * and the context menu in the grid. It can also represent a button
@@ -86,6 +88,14 @@ export interface Action {
   onStopHover?: (env: SpreadsheetChildEnv) => void;
 }
 
+export interface ComputedAction
+  extends Omit<Action, "name" | "description" | "icon" | "secondaryIcon" | "children"> {
+  name: string;
+  description: string;
+  icon: string;
+  secondaryIcon: string;
+}
+
 export type ActionBuilder = (env: SpreadsheetChildEnv) => ActionSpec[];
 type ActionChildren = (ActionSpec | ActionBuilder)[];
 
@@ -137,3 +147,50 @@ export function createAction(item: ActionSpec): Action {
     onStopHover: item.onStopHover,
   };
 }
+
+export function getMenuItemsAndSeparators(
+  env: SpreadsheetChildEnv,
+  actions: Action[]
+): MenuItemOrSeparator[] {
+  function isRoot(menu: Action) {
+    return !menu.execute;
+  }
+
+  function hasVisibleChildren(menu: Action) {
+    return menu.children(env).some((child) => child.isVisible(env));
+  }
+
+  const menuItemsAndSeparators: MenuItemOrSeparator[] = [];
+  for (let i = 0; i < actions.length; i++) {
+    const menuItem = actions[i];
+    if (menuItem.isVisible(env) && (!isRoot(menuItem) || hasVisibleChildren(menuItem))) {
+      menuItemsAndSeparators.push(menuItem);
+    }
+    if (
+      menuItem.separator &&
+      i !== actions.length - 1 && // no separator at the end
+      menuItemsAndSeparators[menuItemsAndSeparators.length - 1] !== "separator" // no double separator
+    ) {
+      menuItemsAndSeparators.push("separator");
+    }
+  }
+  if (menuItemsAndSeparators[menuItemsAndSeparators.length - 1] === "separator") {
+    menuItemsAndSeparators.pop();
+  }
+  if (menuItemsAndSeparators.length === 1 && menuItemsAndSeparators[0] === "separator") {
+    return [];
+  }
+  return menuItemsAndSeparators;
+}
+
+export function isMenuItemEnabled(env: SpreadsheetChildEnv, menu: Action): boolean {
+  const children = menu.children?.(env);
+  if (children.length) {
+    return children.some((child) => isMenuItemEnabled(env, child));
+  } else {
+    if (menu.isEnabled(env)) {
+      return env.model.getters.isReadonly() ? menu.isReadonlyAllowed : true;
+    }
+    return false;
+  }
+}

src/components/menu/menu.css

@@ -2,6 +2,7 @@
   .o-menu {
     user-select: none;
     .o-menu-item {
+      outline: none;
       cursor: pointer;
       user-select: none;
       height: var(--os-desktop-menu-item-height);
@@ -13,7 +14,6 @@
         );
       }
       & {
-        &:hover,
         &.o-menu-item-active {
           background-color: var(--os-button-active-bg);
           color: var(--os-button-active-text-color);

src/components/menu/menu.ts

@@ -1,25 +1,27 @@
+import { Rect } from "@odoo/o-spreadsheet-engine";
 import { cssPropertiesToCss } from "@odoo/o-spreadsheet-engine/components/helpers/css";
 import { SpreadsheetChildEnv } from "@odoo/o-spreadsheet-engine/types/spreadsheet_env";
-import { Component, onWillUnmount } from "@odoo/owl";
-import { Action } from "../../actions/action";
+import { Component, useEffect, useRef } from "@odoo/owl";
+import { Action, isMenuItemEnabled, MenuItemOrSeparator } from "../../actions/action";
 import { Pixel } from "../../types";
 
 //------------------------------------------------------------------------------
 // Context Menu Component
 //------------------------------------------------------------------------------
 
-type MenuItemOrSeparator = Action | "separator";
+export type MenuItemRects = { [menuItemId: string]: Rect };
 
 export interface MenuProps {
-  menuItems: Action[];
+  menuItems: MenuItemOrSeparator[];
   onClose: () => void;
   onScroll?: (ev: CustomEvent) => void;
   onClickMenu?: (menu: Action, ev: CustomEvent) => void;
   onMouseEnter?: (menu: Action, ev: PointerEvent) => void;
-  onMouseOver?: (menu: Action, ev: PointerEvent) => void;
   onMouseLeave?: (menu: Action, ev: PointerEvent) => void;
-  isActive?: (menu: Action) => boolean;
+  hoveredMenuId?: string;
+  isHoveredMenuFocused?: boolean;
   width?: number;
+  onKeyDown?: (ev: KeyboardEvent) => void;
 }
 
 export interface MenuState {
@@ -37,53 +39,33 @@ export class Menu extends Component<MenuProps, SpreadsheetChildEnv> {
     onClose: Function,
     onClickMenu: { type: Function, optional: true },
     onMouseEnter: { type: Function, optional: true },
-    onMouseOver: { type: Function, optional: true },
     onMouseLeave: { type: Function, optional: true },
     width: { type: Number, optional: true },
-    isActive: { type: Function, optional: true },
+    hoveredMenuId: { type: String, optional: true },
+    isHoveredMenuFocused: { type: Boolean, optional: true },
     onScroll: { type: Function, optional: true },
+    onKeyDown: { type: Function, optional: true },
   };
 
   static components = {};
   static defaultProps = {};
 
-  private hoveredMenu: Action | undefined = undefined;
+  private menuRef = useRef("menu");
 
-  setup() {
-    onWillUnmount(() => {
-      this.hoveredMenu?.onStopHover?.(this.env);
-    });
-  }
-
-  get menuItemsAndSeparators(): MenuItemOrSeparator[] {
-    const menuItemsAndSeparators: MenuItemOrSeparator[] = [];
-    for (let i = 0; i < this.props.menuItems.length; i++) {
-      const menuItem = this.props.menuItems[i];
-      if (
-        menuItem.isVisible(this.env) &&
-        (!this.isRoot(menuItem) || this.hasVisibleChildren(menuItem))
-      ) {
-        menuItemsAndSeparators.push(menuItem);
-      }
-      if (
-        menuItem.separator &&
-        i !== this.props.menuItems.length - 1 && // no separator at the end
-        menuItemsAndSeparators[menuItemsAndSeparators.length - 1] !== "separator" // no double separator
-      ) {
-        menuItemsAndSeparators.push("separator");
+  setup(): void {
+    useEffect(() => {
+      if (this.props.hoveredMenuId && this.props.isHoveredMenuFocused && this.menuRef.el) {
+        const selector = `[data-name='${this.props.hoveredMenuId}']`;
+        const menuItemElement = this.menuRef.el.querySelector(selector) as HTMLElement;
+        menuItemElement?.focus();
       }
-    }
-    if (menuItemsAndSeparators[menuItemsAndSeparators.length - 1] === "separator") {
-      menuItemsAndSeparators.pop();
-    }
-    if (menuItemsAndSeparators.length === 1 && menuItemsAndSeparators[0] === "separator") {
-      return [];
-    }
-    return menuItemsAndSeparators;
+    });
   }
 
   get childrenHaveIcon(): boolean {
-    return this.props.menuItems.some((menuItem) => !!this.getIconName(menuItem));
+    return this.props.menuItems.some(
+      (menuItem) => menuItem !== "separator" && !!this.getIconName(menuItem)
+    );
   }
 
   getIconName(menu: Action) {
@@ -113,38 +95,14 @@ export class Menu extends Component<MenuProps, SpreadsheetChildEnv> {
     return !menu.execute;
   }
 
-  private hasVisibleChildren(menu: Action) {
-    return menu.children(this.env).some((child) => child.isVisible(this.env));
-  }
-
   isEnabled(menu: Action) {
-    const children = menu.children?.(this.env);
-    if (children.length) {
-      return children.some((child) => this.isEnabled(child));
-    } else {
-      if (menu.isEnabled(this.env)) {
-        return this.env.model.getters.isReadonly() ? menu.isReadonlyAllowed : true;
-      }
-      return false;
-    }
+    return isMenuItemEnabled(this.env, menu);
   }
 
   get menuStyle() {
     return this.props.width ? cssPropertiesToCss({ width: this.props.width + "px" }) : "";
   }
 
-  onMouseEnter(menu: Action, ev: PointerEvent) {
-    this.hoveredMenu = menu;
-    menu.onStartHover?.(this.env);
-    this.props.onMouseEnter?.(menu, ev);
-  }
-
-  onMouseLeave(menu: Action, ev: PointerEvent) {
-    this.hoveredMenu = undefined;
-    menu.onStopHover?.(this.env);
-    this.props.onMouseLeave?.(menu, ev);
-  }
-
   onClickMenu(menu: Action, ev: CustomEvent) {
     if (!this.isEnabled(menu)) {
       return;

src/components/menu/menu.xml

@@ -9,21 +9,22 @@
       t-on-pointerdown.prevent=""
       t-on-click.stop=""
       t-on-contextmenu.prevent="">
-      <t t-foreach="menuItemsAndSeparators" t-as="menuItem" t-key="menuItem_index">
+      <t t-foreach="props.menuItems" t-as="menuItem" t-key="menuItem_index">
         <div t-if="menuItem === 'separator'" class="o-separator border-bottom"/>
         <t t-else="">
           <t t-set="isMenuRoot" t-value="isRoot(menuItem)"/>
           <t t-set="isMenuEnabled" t-value="isEnabled(menuItem)"/>
           <div
+            tabIndex="-1"
             t-att-title="getName(menuItem)"
             t-att-data-name="menuItem.id"
             t-on-click="(ev) => this.onClickMenu(menuItem, ev)"
             t-on-auxclick="(ev) => this.onClickMenu(menuItem, ev)"
-            t-on-mouseover="(ev) => this.props.onMouseOver?.(menuItem, ev)"
-            t-on-mouseenter="(ev) => this.onMouseEnter?.(menuItem, ev)"
-            t-on-mouseleave="(ev) => this.onMouseLeave?.(menuItem)"
+            t-on-mouseenter="(ev) => this.props.onMouseEnter?.(menuItem, ev)"
+            t-on-mouseleave="(ev) => this.props.onMouseLeave?.(menuItem)"
+            t-on-keydown="(ev) => props.onKeyDown?.(ev)"
             class="o-menu-item d-flex justify-content-between align-items-center"
-            t-att-class="{'disabled': !isMenuEnabled, 'o-menu-item-active': props.isActive?.(menuItem)}"
+            t-att-class="{'disabled': !isMenuEnabled, 'o-menu-item-active': menuItem.id === props.hoveredMenuId}"
             t-att-style="getColor(menuItem)">
             <div class="d-flex w-100">
               <div

src/components/menu_popover/menu_popover.css

@@ -1,5 +1,6 @@
 .o-spreadsheet {
   .o-menu-wrapper {
     padding: var(--os-menu-vertical-padding) 0px;
+    outline: none;
   }
 }