Skip to content

feat(evo-menu): replace @item separator with @group attribute tag - #878

Open
LuLaValva wants to merge 2 commits into
mainfrom
llavalva-M451495HWX-evo-menu-groups-xaishq
Open

feat(evo-menu): replace @item separator with @group attribute tag#878
LuLaValva wants to merge 2 commits into
mainfrom
llavalva-M451495HWX-evo-menu-groups-xaishq

Conversation

@LuLaValva

Copy link
Copy Markdown
Member

Adds a <@group> attribute tag to evo-menu (and passes it through evo-menu-button), replacing the separator attribute on <@item>. Each group takes its own selected/selectedChange and nested <@item> tags; separators render automatically between groups, and roving tabindex, typeahead, and arrow navigation treat the whole menu as one list. Top-level <@item> tags may be mixed with groups and always render first, sharing the menu-level selected.

selected (menu-level or per-group) now also accepts null, meaning single-select with nothing selected yet — distinct from omitting selected, which renders plain non-selectable menu items. Default item values are now unique across the whole menu so grouped items keep distinct data-values.

Includes new Groups and Controlled stories/examples, updated tests, and a changeset (BREAKING: separator removed from <@item>).

🤖 Generated with Claude Code

Adds <@group> to evo-menu and evo-menu-button: each group has its own
selected/selectedChange and nested <@item> tags, with separators
rendered automatically between groups. Top-level items render first as
an implicit group sharing the menu-level selected. selected now also
accepts null (single-select with nothing selected), and default item
values are unique across the whole menu so roving focus works across
groups.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings August 12, 2026 16:00
@changeset-bot

changeset-bot Bot commented Aug 12, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: f715d1b

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@evo-web/marko Minor

Not sure what this means? Click here to learn what changesets are.

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

Copilot AI left a comment

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.

Pull request overview

This PR updates @evo-web/marko’s evo-menu API to support grouped menu items via a new <@group> attribute tag, removing the old per-<@item> separator attribute. It also propagates the grouping API through evo-menu-button, and updates stories/tests/snapshots plus a breaking-change changeset.

Changes:

  • Add <@group> to evo-menu and pass it through evo-menu-button, with automatic separators between groups.
  • Expand selection semantics so selected can be null (single-select with nothing selected yet) and ensure default item values are unique across the whole menu.
  • Add/adjust Storybook examples and update browser/server tests and snapshots to cover groups + controlled selection.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
packages/evo-marko/src/tags/evo-menu/index.marko Implements grouped rendering, selection handling, and automatic separators.
packages/evo-marko/src/tags/evo-menu/menu.stories.ts Updates docs/argTypes and adds Groups/Controlled stories.
packages/evo-marko/src/tags/evo-menu/test/test.server.ts Adds SSR snapshot coverage for selected=null, groups, and controlled selection.
packages/evo-marko/src/tags/evo-menu/test/test.browser.ts Adds interaction tests for selected=null, grouped behavior, and controlled selection.
packages/evo-marko/src/tags/evo-menu/test/snapshots/test.server.ts.snap Updates snapshots for new stories/behaviors.
packages/evo-marko/src/tags/evo-menu/examples/separator.marko Removes the old separator example.
packages/evo-marko/src/tags/evo-menu/examples/groups.marko Adds a grouped menu example.
packages/evo-marko/src/tags/evo-menu/examples/controlled.marko Adds a controlled-selection-across-groups example.
packages/evo-marko/src/tags/evo-menu-button/index.marko Passes group through to evo-menu and adjusts filter selected detection for null.
packages/evo-marko/src/tags/evo-menu-button/menu-button.stories.ts Replaces separator story with groups story.
packages/evo-marko/src/tags/evo-menu-button/test/test.server.ts Updates SSR coverage from separators to groups.
packages/evo-marko/src/tags/evo-menu-button/test/snapshots/test.server.ts.snap Updates snapshots to reflect new groups story and reordered snapshot entries.
packages/evo-marko/src/tags/evo-menu-button/examples/separator.marko Removes the old separator example.
packages/evo-marko/src/tags/evo-menu-button/examples/groups.marko Adds a grouped menu-button example.
.changeset/hungry-melons-repeat.md Documents the breaking API change (separator removed; <@group> added; selected supports null).
Suppressed comments (3)

packages/evo-marko/src/tags/evo-menu/index.marko:153

  • Clicking an item updates selected even when the menu/group is meant to be non-selectable (i.e., when selected was omitted). This can accidentally turn a plain menu into a selectable one after the first click.
                    onClick(e, target) {
                        rovTabindex.onClick(value);
                        selected = nextSelected(selected, value);
                        item.onClick && item.onClick(e, target);
                    }

packages/evo-marko/src/tags/evo-menu/index.marko:158

  • Keyboard activation (Enter/Space) updates selected even when the menu/group is non-selectable (when selected is omitted). This should be guarded the same way as click selection updates.
                    onKeyDown(e, target) {
                        if (e.key === "Enter" || e.key === " ") {
                            selected = nextSelected(selected, value);
                        }
                        rovTabindex.onKeyDown(e);

packages/evo-marko/src/tags/evo-menu/menu.stories.ts:76

  • The @group.selected doc type string doesn’t include null, but the description documents null as valid for single-select with no selection. This is a docs/type mismatch in Storybook.
        selected: {
          type: "number | string | (number | string)[]",
          description:

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +118 to +119
<const/isCheckbox=Array.isArray(group.selected) || (isFilter && !isRadio)>
<const/isCheckable=isRadio || isCheckbox>
Comment on lines +41 to 42
"If present, indicates the selected item(s) among the top-level `@item`s and automatically updates them on click. Use a single value for single-select (`null` for single-select with nothing selected), or an array for multi-select. Compares with `value` if present on `@item`, otherwise the item's index in the menu.",
table: { type: { summary: "number | string | (number | string)[]" } },
Group<Index> defaults to the full selection union so existing usage is
unchanged, while consumers can pin a narrow type (e.g. Group<string>)
and get a matching selectedChange parameter.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

PR Preview Deployed

Websiteevo-markoevo-reactebayui-coreebayui-core-reactskin

commit f715d1b

Comment on lines +114 to +116
<if=groupIndex>
<hr class=`${baseClass}__separator` role="separator">
</if>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Smart 👌

<const/isRadio=(
typeof group.selected === "number" ||
typeof group.selected === "string" ||
group.selected === null

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should we use == null instead of === null.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Definitely not! null or string or number opts this group into radio behavior, while undefined means the items should be non-selectable buttons!

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