Nxt 4643 Create/rework container components by hriverahdez · Pull Request #436 · knime/knime-designsystem

hriverahdez · 2026-04-16T15:34:12Z

This PR introduces the following changes:

MenuContainer:

This now became more of a "base" component, which provides behaviors for many other components. It changed like so:

Doesn't directly reuse ListContainer anymore, which use to couple it to its implementation. Instead, it reuses behaviors via composables. This is also helpful later on to implement features like nested submenus, which involve using recursive components. Had this component kept using the ListContainer then all that complexity would been added to it unnecessarily
Another reason is because this component doesn't support the "empty state", as that wouldn't make sense for it
ATM also doesn't support the loading state (will discuss with UX if this is needed)
Add capability to support a large variant for the items. Though this applies at "container-level" meaning all items would have the same size.
Add capability to support adding a badge. This is not added to the accessory object because:
(1) the badge is meant to be displayed at the right end, unlike the accessories
(2) you can have items with both an accessory (e.g avatar) and a badge. So making the badge an accessory wouldn't allow this.
Add capability to support item-level handlers. This means the each object in the items prop can receive a handler function which runs when the item is interacted with, whether that is via clicks or keyboard events. Although, the older general event (itemClicked) was kept for backwards compatibility since that's used by the SplitButton

ListContainer:

No API changes
Uses the same composables that were extracted to support the shared behaviors (e.g keyboard nav, unique ids, etc)

ListItem:

Added supporting badges to the content, as a trailing item.
However, introduced a new restriction for trailing items. The prev impl allowed supplying both a trailingIcon and a shortcut, both of which were trailing items but that doesn't make sense. This PR makes it so that the component checks for: (first) shortcut, (second) icon (third) badge, and bails on first-match. It keeps the same behavior for preserving the space

Menu:

New component. Allows rendering a menu using a custom toggler, as opposed to MenuButton which only works with a KdsButton. This component is needed for cases where we want to implement togglers that do not necessarily should live in KDS. For example, clicking an account avatar can open a menu, but having a MenuButtonAvatar doesn't make sense, specially if there's very little cases like this.

⚠️ This component requires a slot to be used, and in the slot all the necessary utilities are exposed to control the menu

ContextMenu:

New component. Unlike the Menu this satisfies a different use-case; one where you don't have a specific element to anchor your menu to. This is the case for context menus, which open on right-click interactions which may or may not have an anchor (e.g blank canvas). So far this seems like the only case for opening a menu w/o having an anchor, therefore the component was directly named ContextMenu. But it's usage is rather generic so it can serve also other purposes

changeset-bot · 2026-04-16T15:34:15Z

⚠️ No Changeset found

Latest commit: fd34c34

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copilot

Pull request overview

This PR expands the menu/container infrastructure in the KNIME Design System by extracting shared list/menu behaviors into composables, introducing new menu APIs (custom toggler + context menu), and extending popover/list-item capabilities to support these use cases.

Changes:

Added KdsMenu (custom toggler via slot) and KdsContextMenu (custom-positioned menu) components.
Refactored keyboard navigation + stable DOM id behavior into shared composables and reused them in KdsListContainer and KdsMenuContainer.
Extended KdsPopover with a "custom" placement mode and extended KdsListItem with trailing badges.

Reviewed changes

Copilot reviewed 24 out of 24 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
packages/components/src/overlays/Popover/types.ts	Adds `customPlacementPosition` prop documentation/type for custom popover placement.
packages/components/src/overlays/Popover/enums.ts	Adds `"custom"` to popover placement options.
packages/components/src/overlays/Popover/KdsPopover.vue	Implements custom placement behavior and runtime warnings.
packages/components/src/index.ts	Re-exports containers from the package root.
packages/components/src/containers/index.ts	Updates container public exports (Menu added; ContextMenu currently not exported).
packages/components/src/containers/_helpers/useStableItemIds.ts	New shared helper for deduplication + stable prefixed DOM ids.
packages/components/src/containers/_helpers/useListItemKeyboardNav.ts	New shared helper for list/menu keyboard navigation + active descendant behavior.
packages/components/src/containers/MenuContainer/types.ts	Extends menu item model (badge, shortcut, handler) and adds a container-wide size variant.
packages/components/src/containers/MenuContainer/KdsMenuItem.vue	New internal renderer component for menu items (headline vs selectable item).
packages/components/src/containers/MenuContainer/KdsMenuContainer.vue	Refactors MenuContainer to use new composables and adds variant support.
packages/components/src/containers/MenuContainer/KdsMenuContainer.stories.ts	Adds Storybook coverage for MenuContainer behaviors and variants.
packages/components/src/containers/Menu/types.ts	Defines `KdsMenuProps` (currently aliased to `KdsMenuContainerProps`).
packages/components/src/containers/Menu/index.ts	Exports the new `KdsMenu` component.
packages/components/src/containers/Menu/KdsMenu.vue	New Menu component wrapping Popover + MenuContainer with a slot-based toggler API.
packages/components/src/containers/Menu/KdsMenu.stories.ts	Adds Storybook story + play test for `KdsMenu`.
packages/components/src/containers/ListItem/KdsListItem/types.ts	Adds badge prop and clarifies trailing-item mutual exclusivity.
packages/components/src/containers/ListItem/KdsListItem/KdsListItem.vue	Implements badge rendering and trailing-item precedence logic.
packages/components/src/containers/ListContainer/tests/KdsListContainer.test.ts	Adjusts duplicate-id warning assertion to be less brittle.
packages/components/src/containers/ListContainer/KdsListContainer.vue	Refactors to use the new shared composables for ids + keyboard navigation.
packages/components/src/containers/ListContainer/KdsListContainer.stories.ts	Updates story expectations for allow-no-selection Enter behavior.
packages/components/src/containers/ContextMenu/types.ts	Adds props for context menu positioning + popover mode.
packages/components/src/containers/ContextMenu/index.ts	Exports the new `KdsContextMenu` component (local barrel).
packages/components/src/containers/ContextMenu/KdsContextMenu.vue	New ContextMenu component using `KdsPopover` custom placement + `KdsMenuContainer`.
packages/components/src/containers/ContextMenu/KdsContextMenu.stories.ts	Adds Storybook story scaffolding for context menu usage.

Copilot · 2026-04-16T15:44:54Z

+const anchorStyle = {
+  "anchor-name": placement === kdsPopoverPlacement.CUSTOM ? "" : anchorName,
+};


For placement === "custom", anchorStyle sets anchor-name to an empty string. This still applies a CSS property with an invalid/meaningless value on the anchor element. Consider returning an empty style object for the custom case (or omitting anchor-name) so consumers don’t apply a broken anchor-name declaration.

Suggested change

const anchorStyle = {

"anchor-name": placement === kdsPopoverPlacement.CUSTOM ? "" : anchorName,

};

const anchorStyle =

placement === kdsPopoverPlacement.CUSTOM

? {}

: { "anchor-name": anchorName };

Copilot · 2026-04-16T15:44:55Z

+  /**
+   * This is only valid and accounted for when the `placement` is of type `custom`. Supplying this will
+   * position the popover statically on the position stated in this value.
+   */
+  customPlacementPosition?: { x: number; y: number };


customPlacementPosition is only meaningful when placement is "custom", but the current prop typing allows invalid combinations and relies on runtime warnings. Consider making KdsPopoverProps a discriminated union so TS enforces that customPlacementPosition is required for placement: "custom" and disallowed otherwise.

@ChristianAlbrecht I wanted to try this even before copilot suggested it but I decided against it because I remember you mentioned that discriminated unions don't work that well for props. Would you be ok with the current approach?

Copilot · 2026-04-16T15:44:55Z

+    <KdsMenuContainer
+      :id="menuId"
+      ref="menuContainer"
+      :items="props.items"
+      :menu-max-height="menuMaxHeight"


KdsMenu accepts ...props from KdsMenuProps but doesn’t forward ariaLabel (and ignores any provided id) when rendering KdsMenuContainer, so the rendered menu will always use KdsMenuContainer’s default aria-label (currently "Actions"). Forward ariaLabel/id (or remove them from the public props type) so consumers can set accessible naming and stable IDs.

I'll remove id and ariaLabel from the props as those should only be internally controlled in the case for this component

Copilot

Pull request overview

Copilot reviewed 24 out of 24 changed files in this pull request and generated 5 comments.

Copilot · 2026-04-17T08:42:01Z

+const open = defineModel<boolean>({ default: false });
+
+const onItemClick = (itemId: string) => {
+  open.value = false;
+  emit("itemClick", itemId);
+};


KdsContextMenu opens/closes the popover but never moves focus into the menu when open becomes true. Other menu-like components (e.g. KdsMenu, KdsMenuButton, KdsSplitButton) focus the menu container on open so keyboard navigation works immediately. Add a watch(open, ...) + nextTick() to focus the inner KdsMenuContainer when the context menu opens (and ensure the container ref is available).

I intentionally left this out. Moving the focus automatically for the MenuContainer (used by the ContextMenu) also means focusing the first item, which makes it visually stand out (as if you'd hover), which is obviously necessary when doing keyboard nav. However, opening the menu with the mouse makes this feel rather weird because you get an item that's automatically highlighted for no apparent reason.

cc @ChristianAlbrecht

But if you do a focus via code and its visually highlighted then the impl is wrong. It should use :focus-visible istead of :focus to style the stuff.

@carlos22 the thing is I took all of that logic from an existing component (ListContainer) and extracted it into a composable. I didn't want to modify it too much to not break the other component and not go further beyond scope. I can try to see if it's an easy fix, let's see...

Ok got that, but I would still do the focus and then fix it with the different pseudo class if it is annoying (which it might be).

@carlos22 @ChristianAlbrecht I've been playing around a bit with the keyboard nav code and I think it's best to have its own separate PR for this

Copilot · 2026-04-17T08:42:02Z

+const anchorStyle = {
+  "anchor-name": placement === kdsPopoverPlacement.CUSTOM ? "" : anchorName,
+};

 const onToggle = (event: ToggleEvent) => {
  open.value = event.newState === "open";
 };

 defineExpose<KdsPopoverExpose>({ anchorStyle, popoverId });
+
+const wrapperStyles = computed(() => {
+  if (
+    placement === kdsPopoverPlacement.CUSTOM &&
+    props.customPlacementPosition
+  ) {
+    return {
+      "max-inline-size": maxInlineSize,
+      left: `${props.customPlacementPosition.x}px`,
+      top: `${props.customPlacementPosition.y}px`,
+    };
+  }
+
+  return {
+    "position-anchor": anchorName,
+    "max-inline-size": maxInlineSize,
+  };
+});


KdsPopover adds a new placement="custom" + customPlacementPosition code path, but the Popover story file doesn’t include a story (or play assertions) that renders the custom placement and verifies that the popover positions correctly and that anchorStyle behaves as expected (e.g. not setting an anchor-name). Since Storybook play tests contribute to coverage here, please add a dedicated story for the custom placement path.

- make separation between ListContainer and MenuContainer more explicit - add badge support for MenuContainer items - change from event to item handler NXT-4643 ()

NXT-4643 ()

NXT-4643 (Use KDS menu button for team selector in App Home Exp.)

- stop context menu happening over items in the KdsContextMenu - improve visuals of KdsMenu and KdsContextMenu stories NXT-4643 ()

hriverahdez · 2026-04-17T09:12:22Z

There are no code changes in this composable, other than using the options arguments and the callback functions in the triggers as opposed to emitting directly as it used to when it was component code, and one small change (see comment below)

Other logic is the same as before

hriverahdez · 2026-04-17T09:14:45Z

+        event.preventDefault();
+        break;
+      case "Enter":
+        if (activeId.value && targetItem) {


I changed this approach from the prev impl. Before, it used to emit an event with an undefined id when hitting enter but nothing was focused, which was weird and didn't seem to enable any use-case I could think of

sonarqubecloud · 2026-04-17T09:16:37Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
95.8% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

Copilot

Pull request overview

Copilot reviewed 24 out of 24 changed files in this pull request and generated 7 comments.

Copilot · 2026-04-17T09:20:18Z

+const wrapperStyles = computed(() => {
+  if (
+    placement === kdsPopoverPlacement.CUSTOM &&
+    props.customPlacementPosition
+  ) {


placement: "custom" / customPlacementPosition is a new behavior path in KdsPopover, but the existing KdsPopover.stories.ts doesn’t appear to include any story that sets placement="custom" (and validates positioning/light-dismiss). Please add Storybook coverage for the custom placement mode so this code path is exercised by the Storybook test suite and visual snapshots.

Copilot · 2026-04-17T09:20:19Z

+    return {
+      "max-inline-size": maxInlineSize,
+      left: `${props.customPlacementPosition.x}px`,
+      top: `${props.customPlacementPosition.y}px`,
+    };


For placement: "custom", the inline styles only set left/top (and max size). Native popovers typically have default inset values; if right/bottom remain set (e.g. via inset: 0), the popover can stretch across the viewport, which breaks light-dismiss (“click outside”) and can lead to an unexpectedly large click-interception area. Consider explicitly clearing inset (e.g. inset: auto or right/bottom: auto) and setting the intended positioning mode (typically position: fixed) in the custom branch.

Copilot · 2026-04-17T09:20:19Z

+// Export container components
+export * from "./containers";
+export type * from "./containers";


This PR introduces new public exports (containers) and new/extended public APIs (e.g. Popover placement: "custom", KdsMenu, KdsContextMenu). There doesn’t appear to be any Changeset added (the .changeset/ folder only contains config.json), so the release tooling won’t capture these user-facing changes. Please add an appropriate changeset (likely minor if any breaking API surface changed, otherwise patch).

Copilot · 2026-04-17T09:20:19Z

+export const Default: Story = {
+  render: (args) => ({
+    components: { KdsMenu, KdsAvatar },
+    setup() {
+      return { args };


This story file only defines a Default story. To align with the repo’s Storybook testing/coverage approach, please add the standard TextOverflow, DesignComparator, and AllCombinations stories (or explicitly document why one of them doesn’t apply for this slot-based wrapper component). Without them, important prop/state combinations (e.g. variant, menuMaxHeight, open/closed rendering) won’t be systematically covered by Storybook tests/snapshots.

Copilot · 2026-04-17T09:20:20Z

+export const Default: Story = {
+  render: (args) => ({
+    components: { KdsContextMenu, KdsButton },
+    setup() {
+      const open = ref(false);
+      const position = ref<{ x: number; y: number }>();
+      const openOnPosition = (event: MouseEvent) => {
+        const { clientX, clientY } = event;
+        position.value = { x: clientX, y: clientY };


This story file only defines a Default story. Please add the standard TextOverflow, DesignComparator, and AllCombinations stories (or explicitly note why any of them don’t apply) so the component’s rendering and prop combinations are covered by Storybook tests/snapshots (e.g. popoverMode, variant, menu content sizes).

Copilot · 2026-04-17T09:20:20Z

+  argTypes: {
+    id: {
+      control: "text",
+      table: { category: "props" },
+    },


variant and menuMaxHeight are props of KdsMenuContainer, but they’re not included in argTypes / args here. That makes the Controls panel and autodocs incomplete and can hide these supported options from users. Consider adding both props to argTypes (category props) and providing explicit defaults in args.

Copilot · 2026-04-17T09:20:20Z

+  /**
+   * Badge to display on the item. This can only be displayed if both `shortcut` and `trailingIcon` are **NOT** supplied
+   */
+  badge?: {
+    label: string;


badge adds a new rendering branch in KdsListItem, but there are currently no Storybook stories exercising it (searching the existing SmallListItem.stories.ts / LargeListItem.stories.ts shows no badge usage). Please add at least one story (and/or include it in AllCombinations) to cover the badge case so the new UI and the shortcut/icon precedence logic are covered by Storybook tests and visual snapshots.

hriverahdez requested review from a team and RainerSchmoeger as code owners April 16, 2026 15:34

hriverahdez requested review from Copilot and knime-ghub-bot April 16, 2026 15:34

Copilot started reviewing on behalf of hriverahdez April 16, 2026 15:34 View session

Copilot AI reviewed Apr 16, 2026

View reviewed changes

hriverahdez changed the title ~~Nxt 4643 use menu button for team selector~~ Nxt 4643 Create/rework container components Apr 16, 2026

Copilot AI review requested due to automatic review settings April 17, 2026 08:34

Copilot started reviewing on behalf of hejtful April 17, 2026 08:34 View session

Copilot AI reviewed Apr 17, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings April 17, 2026 09:09

hriverahdez force-pushed the NXT-4643-use-menu-button-for-team-selector branch from 7482155 to 98e9afb Compare April 17, 2026 09:09

Copilot started reviewing on behalf of hriverahdez April 17, 2026 09:09 View session

hriverahdez and others added 6 commits April 17, 2026 11:09

NXT-4643: Improve MenuContainer

401f246

- make separation between ListContainer and MenuContainer more explicit - add badge support for MenuContainer items - change from event to item handler NXT-4643 ()

NXT-4643: introduce Menu comp with custom toggle

cb9ba68

NXT-4643 ()

NXT-4643: create component for context menu

6ad8321

NXT-4643 ()

NXT-4643: fixes

35e3049

NXT-4643 ()

NXT-4643: Fixes

ad55b99

NXT-4643 (Use KDS menu button for team selector in App Home Exp.)

NXT-4643: improvements

fd34c34

- stop context menu happening over items in the KdsContextMenu - improve visuals of KdsMenu and KdsContextMenu stories NXT-4643 ()

hriverahdez force-pushed the NXT-4643-use-menu-button-for-team-selector branch from 98e9afb to fd34c34 Compare April 17, 2026 09:10

hriverahdez commented Apr 17, 2026

View reviewed changes

Copilot AI reviewed Apr 17, 2026

View reviewed changes

Conversation

hriverahdez commented Apr 16, 2026

Uh oh!

changeset-bot bot commented Apr 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⚠️ No Changeset found

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

Uh oh!

hriverahdez Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

Uh oh!

hriverahdez Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

hriverahdez Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

carlos22 Apr 20, 2026

Choose a reason for hiding this comment

Uh oh!

hriverahdez Apr 20, 2026

Choose a reason for hiding this comment

Uh oh!

carlos22 Apr 20, 2026

Choose a reason for hiding this comment

Uh oh!

hriverahdez Apr 20, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

hriverahdez Apr 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hriverahdez Apr 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Apr 17, 2026

Quality Gate passed

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 17, 2026

changeset-bot bot commented Apr 16, 2026 •

edited

Loading

hriverahdez Apr 17, 2026 •

edited

Loading

hriverahdez Apr 17, 2026 •

edited

Loading