Skip to content

feat: add support for slotted list-box to context-menu - #12172

Merged
web-padawan merged 11 commits into
mainfrom
feat/context-menu-list-box
Jul 20, 2026
Merged

feat: add support for slotted list-box to context-menu#12172
web-padawan merged 11 commits into
mainfrom
feat/context-menu-list-box

Conversation

@web-padawan

Copy link
Copy Markdown
Member

Description

Same as #12131 but for vaadin-context-menu.

Type of change

  • Feature

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@web-padawan
web-padawan requested a review from vursen July 16, 2026 12:27

@vaadin-review-bot vaadin-review-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Reviewed the changes — left 4 comments.

Finding
⚠️ A slotted list-box menu is left unclosable when items is set before slotchange runs.
⚠️ Removing a slotted list-box throws an uncaught error when both items and renderer are set.
⚠️ __preserveMenuState reads listBox.items.indexOf without guarding against undefined items.
🧹 New _menuListBox getter duplicates the list-box resolution already in __getListBox().

Comment thread packages/context-menu/src/vaadin-context-menu-mixin.js Outdated
Comment thread packages/context-menu/src/vaadin-context-menu-mixin.js Outdated
Comment thread packages/context-menu/src/vaadin-context-menu-mixin.js
Comment thread packages/context-menu/src/vaadin-menu-overlay-mixin.js Outdated
- Reject the items + renderer combination synchronously in
  _contentSourceChanged so removing a slotted list-box no longer surfaces
  a deferred throw from the async slotchange handler.
- Restore the default closeOn when the items branch cleared it before the
  slotted list-box was detected, so clicking an item closes the menu.
- Guard __preserveMenuState against a slotted list-box whose items are not
  yet initialized.
- Consolidate list-box resolution into a single _menuListBox getter on
  ItemsMixin (replacing __getListBox); the first-child fallback for custom
  renderer content is inlined at the two call sites that need it.

Part of #11422

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@web-padawan
web-padawan requested a review from sissbruecker July 16, 2026 13:25
Comment thread dev/playground/context-menu.html Outdated
Comment thread packages/context-menu/src/vaadin-context-menu-mixin.js Outdated
Combining a slotted <vaadin-context-menu-list-box> with the items or
renderer API is not a supported use case, so it now throws instead of
warning. With the three content sources mutually exclusive, the closeOn
handling collapses to a single default-clearing branch.

Part of #11422

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Comment thread packages/context-menu/src/vaadin-context-menu-mixin.js Outdated
@web-padawan
web-padawan requested a review from vursen July 20, 2026 11:14
web-padawan and others added 4 commits July 20, 2026 14:16
Move the mutual-exclusion check for items, renderer, and a slotted
list-box out of a complex observer into the updated() lifecycle,
matching the approach used in vaadin-select.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Comment thread packages/context-menu/src/vaadin-contextmenu-items-mixin.js Outdated
Comment thread packages/context-menu/src/vaadin-contextmenu-items-mixin.js Outdated
Comment thread packages/context-menu/src/vaadin-menu-overlay-mixin.js Outdated
Co-authored-by: Sergey Vinogradov <mr.vursen@gmail.com>
@web-padawan
web-padawan removed the request for review from sissbruecker July 20, 2026 14:31
@sonarqubecloud

Copy link
Copy Markdown

@web-padawan
web-padawan merged commit 0421006 into main Jul 20, 2026
10 checks passed
@web-padawan
web-padawan deleted the feat/context-menu-list-box branch July 20, 2026 14:39
web-padawan added a commit that referenced this pull request Jul 20, 2026
Add public vaadin-context-menu-item and vaadin-context-menu-list-box
entrypoints with proper JSDoc including Styling sections. Add the
ContextMenuItemData TS type and deprecate ContextMenuItem in favor of it.

Fixes #11422
Related to #12172

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
web-padawan added a commit that referenced this pull request Jul 20, 2026
Add public vaadin-context-menu-item and vaadin-context-menu-list-box
entrypoints with proper JSDoc including Styling sections. Add the
ContextMenuItemData TS type and deprecate ContextMenuItem in favor of it.

Fixes #11422
Related to #12172

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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.

3 participants