🐛 fix(menu): focus trigger element on hover

[mikij]

What was done? 📝

When we have more menus on page clicking on item gives focus to element. After we hover on another menu page wants to scroll to focused element. With this fix menu gets focus on hover so previous menu looses it and therefor page will not need to scroll at all.

Screenshots or GIFs 📸

|-----Figma-----|
|-----Implementation-----|

Link to Issue 🔗

closes #418

Type of change 🏗

• New feature (non-breaking change that adds functionality)
• Bug fix (non-breaking change that fixes an issue)
• Refactor (non-breaking change that improves the code or technical debt)
• Chore (none of the above, such as upgrading libraries)

Breaking change 🚨

Checklist 🧐

• Tested on Chrome
• Tested on Safari
• Tested on Firefox
• No errors in the console

Summary by CodeRabbit

• Bug Fixes

  • Improved menu hover behavior: hovering a menu trigger now reliably focuses the trigger without scrolling, prevents unexpected menu closes, and ensures the hover-open sequence consistently registers and opens the menu for smoother, more predictable interaction.

• Accessibility

  • Menu triggers are now keyboard-focusable, improving navigation and interaction via keyboard.

[coderabbitai]

📝 Walkthrough

Walkthrough

Removed standalone: true from ZardMenuDirective; added a host tabindex attribute and changed ElementRef injection to typed inject<ElementRef<HTMLElement>>. On hover enter the directive now calls element.focus({ preventScroll: true }) before canceling scheduled close, registering hover menu, and opening the CDK menu.

Changes

Cohort / File(s)	Summary
Menu Directive libs/zard/src/lib/shared/components/menu/menu.directive.ts	Removed standalone: true from @Directive; added host binding [attr.tabindex]="'0'"; changed injection to inject<ElementRef<HTMLElement>>(ElementRef); added element.focus({ preventScroll: true }) on mouseenter before canceling close, registering hover, and opening CDK menu.
Manifest / Package manifest_file, package.json	Small metadata/package changes (+3/−2 lines) — minor package/manifest edits accompanying the directive update.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• ✨ feat(context-menu): adding context menu directive #381 — Edits the same menu.directive.ts, updating directive decorator, host bindings, and hover/trigger behavior.

Suggested reviewers

• srizzon
• ribeiromatheuss

Poem

A focus lands without a scroll,
The hover tames the menu's role,
Element gently held in place,
Small code tweak, a quieter space. ✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: adding focus management to the menu trigger element on hover to fix unwanted page scrolling.
Description check	✅ Passed	The PR description covers the problem, solution, testing status, and follows the template structure with proper sections and checked items.
Linked Issues check	✅ Passed	The PR implements the core fix for issue #418 by adding preventScroll: true when focusing the menu trigger on hover, preventing unwanted page scrolling.
Out of Scope Changes check	✅ Passed	All changes are scoped to the menu directive's focus behavior. The removal of standalone: true and dependency injection updates are necessary supporting changes.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix/menu-focus

---

No actionable comments were generated in the recent review. 🎉

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@libs/zard/src/lib/shared/components/menu/menu.directive.ts`:
- Line 111: Replace the plain focus() call in the menu directive with a
non-scrolling focus invocation to match the PR intent: change the existing
element.focus() call (in the menu directive where the element is focused after
hover/open) to element.focus({ preventScroll: true }) so the focus action does
not cause page scrolling; update the call in the MenuDirective (where the focus
is currently performed) accordingly.

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)

libs/zard/src/lib/shared/components/menu/menu.directive.ts (2)

48-48: 🧹 Nitpick | 🔵 Trivial

Type the ElementRef to avoid implicit any.

inject(ElementRef) gives you ElementRef<any>. Since you're calling .focus() and .contains() on nativeElement, type it explicitly for proper type safety.

Proposed fix

-  private readonly elementRef = inject(ElementRef);
+  private readonly elementRef = inject<ElementRef<HTMLElement>>(ElementRef);

As per coding guidelines: "Use TypeScript for all project files with proper typing and avoid any type."

---

106-115: ⚠️ Potential issue | 🟠 Major

Add tabindex: '0' to the host metadata to enable focus functionality.

The element.focus({ preventScroll: true }) call on line 111 won't execute effectively without an explicit tabindex. The host metadata declares role: 'button' but lacks tabindex, meaning [z-menu] applied to non-natively-focusable elements (e.g., <div>, <span>) will silently ignore the focus() call. The sibling z-context-menu and z-menu-content directives already set tabindex: '0' in this codebase; align this directive for consistency:

Required fix

   host: {
     role: 'button',
+    '[attr.tabindex]': "'0'",
     '[attr.aria-haspopup]': "'menu'",