Skip to content

fix(Menu): add title tooltip on truncated option labels and responsiv… - #1180

Open
nomeshwer wants to merge 2 commits into
frappe:mainfrom
nomeshwer:fix/dropdown-title-tooltip-and-max-width
Open

nomeshwer wants to merge 2 commits into
frappe:mainfrom
nomeshwer:fix/dropdown-title-tooltip-and-max-width

Conversation

@nomeshwer

@nomeshwer nomeshwer commented Sep 17, 2026

Copy link
Copy Markdown

Summary

When dropdown menus contain items with long labels or descriptions (e.g. site/subdomain switchers in Frappe Cloud, multi-account selectors, or workflow actions), the text is truncated with .truncate.

Currently, hovering over a truncated item does not reveal the underlying full string, forcing users to click blindly or guess between items with similar prefixes. Additionally, unconstrained dropdowns can overflow narrow viewport widths.

This PR introduces two focused enhancements:

  1. Hover Tooltip Accessibility: Binds :title="item.label" and :title="item.description" on the label and description containers in MenuItemContent.vue.
  2. Responsive Max-Width Containment: Applies max-w-[min(26rem,calc(100vw-2rem))] to unconstrained menus, while preserving trigger matching when matchTriggerWidth is enabled.

Before & After

image
  • Before: Truncated item labels (e.g. frappe-enterprise-sandbox-migrat...) cannot be inspected on hover, leaving critical subdomain information hidden.
  • After: Hovering immediately reveals the full domain string (frappe-enterprise-sandbox-migration-test.frappe.cloud) via the browser's native accessible tooltip.

Verification

  • Automated Unit Tests: Added unit test suite in src/components/Menu/MenuItemContent.test.ts.
  • Vitest Suite: 134/134 test files pass (2,058 tests).
  • Typecheck: yarn type-check passes with 0 errors.

Changelog

  • MenuItemContent: Bind :title to truncated item label and description for hover inspection.
  • Dropdown / Menu / ContextMenu: Add responsive max-w-[min(26rem,calc(100vw-2rem))] containment to prevent viewport overflow while preserving trigger width matching when matchTriggerWidth: true.

…e max-width

- Add native title attribute to truncated option label and description in MenuItemContent.vue
- Add responsive max-width containment to menuClasses.content
- Add unit test suite in MenuItemContent.test.ts
@nomeshwer
nomeshwer marked this pull request as ready for review September 17, 2026 13:21
@greptile-apps

greptile-apps Bot commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 4/5

The PR is not safe to merge until responsive containment preserves matchTriggerWidth and the required changelog section is added.

Summary

Adds native title tooltips to default menu labels/descriptions and a shared responsive maximum width.

Reviews (1) · Last reviewed commit: "fix(Menu): add title tooltip on truncate..."

Comment thread src/components/Menu/utils.ts Outdated
export const menuClasses = {
content:
'menu-content z-[100] min-w-40 divide-y divide-outline-elevation-2 rounded-6 bg-surface-elevation-2 shadow-2xl ring-1 ring-black ring-opacity-5 focus:outline-none',
'menu-content z-[100] min-w-40 max-w-[min(26rem,calc(100vw-2rem))] divide-y divide-outline-elevation-2 rounded-6 bg-surface-elevation-2 shadow-2xl ring-1 ring-black ring-opacity-5 focus:outline-none',

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Trigger width gets capped
When matchTriggerWidth is enabled and the trigger is wider than 26rem, this shared maximum width clamps the menu instead of matching the trigger exactly. Apply the responsive cap only when trigger-width matching is disabled.

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/components/Menu/utils.ts
Line: 19

Comment:
**Trigger width gets capped**
When `matchTriggerWidth` is enabled and the trigger is wider than 26rem, this shared maximum width clamps the menu instead of matching the trigger exactly. Apply the responsive cap only when trigger-width matching is disabled.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in bcee21a: extracted menuClasses.contentMaxWidth and applied it to Dropdown.vue only when trigger-width matching is disabled (!matchTriggerWidth && menuClasses.contentMaxWidth). Also added the required ## Changelog section to the PR description.

- Isolate contentMaxWidth to avoid clamping menus when matchTriggerWidth is enabled
- Apply contentMaxWidth to Dropdown only when matchTriggerWidth is disabled
- Apply contentMaxWidth to ContextMenu and submenu containers
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants