feat(menu-separator): phases 2 to 7 migration by blunteshwar · Pull Request #6893 · adobe/spectrum-web-components

blunteshwar · 2026-11-02T06:43:04Z

Description

Phases 2 to 7 of the swc-menu-separator migration (Epic SWC-2668), combined into one PR. Targets the menu-separator-migration feature branch.

Setup (2): MenuSeparatorBase in core, the swc element, and the core package exports.
API (3): host role="separator" set in connectedCallback; no properties, slots or events. The 1st-gen deprecations for B1 to B3 are deferred (Q4).
Accessibility (4): plan and a11y analysis updated.
Styling (5): the line thickness comes from the parent menu. swc-menu sets --swc-menu-separator-thickness to divider-thickness-medium on ::slotted(swc-menu-separator). The separator centers the line in the 12px menu-item-section-divider-height container, uses base-padding-horizontal-medium for inline margin, and supports forced colors. A separator outside a menu renders a 0px line.
Testing (6): Storybook play-function tests and a Playwright toMatchAriaSnapshot spec.
Docs (7): menu-separator.mdx, migration-guide.mdx, an Accessibility story and a gen2 changeset.

Alignment with the merged plan

Re-checked against the plan merged in #6895:

swc-menu-separator is now in MENU_ALLOWED_CHILDREN, so swc-menu no longer warns about it. The Menu JSDoc and docs no longer refer to swc-divider as the separator (Q1).
The thickness property --swc-menu-separator-thickness, which swc-menu sets, is documented with @cssprop, as the plan expects for the one reviewed property.
Added an InMenu story, a docs example and a test that checks the 2px thickness comes from the menu and that the separator is not focusable.
Plan checklist ticks updated to match.

Open items

Q3: Figma shows a 7px gap (16px area) but published tokens give 5px (12px area). The test asserts the 5px margin and needs updating when the token ships.
margin-inline uses base-padding-horizontal-medium because menu-item-label-inline-edge-to-content is not in tokens.css. Needs design confirmation.
Contrast (1.4.11), VRT, and the tests and stories that need swc-menu-item and swc-menu-group are deferred.
The Playwright spec was not run locally.

Validation

yarn lint:gen2: passed
yarn workspace @spectrum-web-components/gen2 build: passed
npx vitest --run --project storybook menu-separator: passed (menu-separator and menu, 46 tests)

changeset-bot · 2026-11-02T06:43:08Z

⚠️ No Changeset found

Latest commit: 2a68285

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.

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

github-actions · 2026-11-02T06:47:32Z

📚 Branch Preview Links

🔍 Gen1 Visual Regression Test Results

When a visual regression test fails (or has previously failed while working on this branch), its results can be found in the following URLs:

Deployed to Azure Blob Storage: pr-6893

If the changes are expected, update the current_golden_images_cache hash in the circleci config to accept the new images. Instructions are included in that file.
If the changes are unexpected, you can investigate the cause of the differences and update the code accordingly.

Rajdeepc · 2026-11-03T07:27:50Z

+  block-size: var(--_swc-menu-separator-thickness);
+  margin-block: max(0px, calc((token("menu-item-section-divider-height") - var(--_swc-menu-separator-thickness)) / 2));
+  margin-inline: token("base-padding-horizontal-medium");
+  background-color: token("gray-200");


Revisit this as it might not meet the plan's contrast requirement. menu uses background-layer-2-color. Kindly use a design-approved token pairing that meets the requirement, or explicitly revise the grouping design and accessibility requirement.

Fixed in 2a68285. The line was gray-200, about 1.3:1 against the menu surface. It now uses gray-500, the lightest gray that reaches 3:1 against background-layer-2-color in both themes (about 3.2:1 light, 3.1:1 dark). This is heavier than the Figma line, so design still needs to confirm the token pairing.

Rajdeepc · 2026-11-03T07:28:49Z

+### 1. Rename the element
+
+```html
+<!-- Before -->
+<sp-menu-divider></sp-menu-divider>
+
+<!-- After -->
+<swc-menu-separator></swc-menu-separator>
+```


This is instructing the consumers to rename only the separator. An existing <sp-menu><sp-menu-divider>… becomes <sp-menu><swc-menu-separator>…, but only swc-menu supplies the new thickness property. The resulting line computes to 0px. The guide needs a parent-menu migration prerequisite and a complete composition example, including moving separators out of menu groups. Also reconcile the claim at line 78 that there are no public custom properties with the shipped @cssprop --swc-menu-separator-thickness.

Fixed in 2a68285. The guide now makes migrating the parent menu the first step and explains that a separator outside swc-menu computes to 0px. It adds a before/after composition that moves the separator out of a menu group. The "no public custom properties" line is replaced with a note that --swc-menu-separator-thickness is set by swc-menu and is not a consumer styling API. swc-menu-group hasn't shipped, so the example flattens the group.

Rajdeepc · 2026-11-03T07:34:25Z

+    const root = await gotoStory(
+      page,
+      'components-menu-separator--overview',
+      'swc-menu-separator'
+    );
+    await expect(root).toMatchAriaSnapshot(`
+      - separator
+    `);
+  });
+
+  test('should expose a single separator in anatomy', async ({ page }) => {
+    const root = await gotoStory(
+      page,
+      'components-menu-separator--anatomy',
+      'swc-menu-separator'
+    );
+    await expect(root).toMatchAriaSnapshot(`
+      - separator


This snapshots the entire Storybook root using only - separator. This just proves its presence but we are not actually testing its placement between menu items. Please assert the menu subtree and separator count explicitly.

Fixed in 2a68285. The spec now asserts the swc-menu subtree (menuitem Cut, menuitem Copy, separator, menuitem Delete) and exactly one separator for the overview, anatomy and accessibility stories. It also asserts the separator is a direct child of swc-menu. I haven't run it locally because it needs Storybook, so CI will be the first run.

…uide, and a11y spec

rubencarvalho · 2026-11-11T13:06:44Z

+// Placeholder rows: `swc-menu-item` has not shipped yet.
+const renderInMenu = (id: string) => html`
+  <swc-button id="${id}-trigger">Edit</swc-button>
+  <swc-menu open for="${id}-trigger">


every story renders swc-menu open, and since the menu is popover="auto" they close each other on the docs page. only the accessibility one stays open, so overview and anatomy just show the Edit button. could we set open only on overview like menu.stories.ts does and open the others from a play function?

rubencarvalho · 2026-11-11T13:06:45Z

+ * Block margin centers the line in the 12px section-divider container
+ * (Q3: container height pending a spacing token update). */
+.swc-MenuSeparator {
+  --_swc-menu-separator-thickness: var(--swc-menu-separator-thickness, 0px);


with the 0px fallback the line disappears anywhere swc-menu isn't the direct parent (a wrapper, or a future submenu or action menu). what if we fall back to token("divider-thickness-medium") here? the menu can still set it

rubencarvalho · 2026-11-11T13:06:45Z

+ * A menu separator separates groups of items inside a menu.
+ *
+ * @element swc-menu-separator
+ * @cssprop {Length} --swc-menu-separator-thickness - Line thickness. Set by the parent `swc-menu`; the separator does not define it.


@cssprop puts this in the API table, but the mdx says thickness isn't configurable and the migration guide says not to set it. could we pick one direction here and make the docs match?

rubencarvalho · 2026-11-11T13:06:45Z

+ *
+ * @element swc-menu-separator
+ * @cssprop {Length} --swc-menu-separator-thickness - Line thickness. Set by the parent `swc-menu`; the separator does not define it.
+ * @since 2.0.0-beta.1


nit: packages are at 2.0.0-beta.7 already, so i think this should be the next version rather than beta.1

rubencarvalho · 2026-11-11T13:06:45Z

+
+Place a menu separator between groups of menu items, not before the first item or after the last. The separator is not interactive and does not receive focus.
+
+Place it as a direct child of `swc-menu`, `swc-action-menu`, or a `swc-menu-item` submenu, never inside a `swc-menu-group`. The menu supplies the line thickness. `swc-menu-item` has not shipped yet, so the examples slot placeholder rows with `role="menuitem"`.


nit: I suspect these may come in soon, but swc-action-menu, swc-menu-item submenus and swc-menu-group haven't shipped yet, and the first two wouldn't set the thickness anyway since only swc-menu's ::slotted rule does. IDK if we should move this to an upcoming features section for now?

rubencarvalho · 2026-11-11T13:06:45Z

+    );
+
+    await step('exposes role="separator" on the host', async () => {
+      expect(separator.getAttribute('role')).toBe('separator');


small one, the sibling tests pass a message as the 2nd arg to expect() (the testing pr checklist asks for it too)

rubencarvalho · 2026-11-11T13:06:45Z

+
+| Area      | Spectrum 1                        | Spectrum 2                                          |
+| --------- | --------------------------------- | --------------------------------------------------- |
+| Role      | `role="separator"`, overridable   | `role="separator"`, set on the host and fixed       |


gen1 also overwrites the role in firstUpdated, so i don't think "overridable" is right. this row could probably go

rubencarvalho · 2026-11-11T13:06:45Z

+| Role      | `role="separator"`, overridable   | `role="separator"`, set on the host and fixed       |
+| Placement | Allowed anywhere inside `sp-menu` | Place it between groups of menu items inside a menu |
+
+## Migration steps


the guide template uses ## Update your code, ### Removed in Spectrum 2, a ## Styling section with the --mod-* and internals callouts, and a ## Checklist. could we line it up with the badge and divider guides? steps 1 and 4 also need a before/after snippet

rubencarvalho · 2026-11-11T13:06:45Z

 | --- | ----------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
 | Q1  | Standalone `swc-menu-separator`; do not reuse `swc-divider`. | Direction confirmed by blunteshwar (PR author) in this PR. nikkimk made the earlier call (Menu plan Q14, reuse `swc-divider`), so her review of the revised decision is still pending. Consistent with the menu-separator accessibility analysis. Supersedes Menu plan Q14. The Menu plan (Q14, Q15, Q21, Phase B) is updated to match. |
 | Q6  | Use the Medium divider thickness (2px). | Figma Divider frame (node 9233:4610): Small 1px, Medium 2px, Large 4px. The Menu frame draws 2px. 1st-gen also defaults to medium. The Spectrum CSS story passing `size: "s"` does not match Figma. |
+| Q2  | The core base extends `SpectrumElement`, not `DividerBase`. | Implemented in Phase 2 and 3 as the recommended option. `DividerBase` carries `size`, `vertical` and `staticColor`, which the separator must not expose. The duplicated line CSS is small (Q5). Pending reviewer confirmation. |


Q2 and Q8 both say pending confirmation, so maybe keep them in open questions until someone confirms? "User confirmation needed" (line 173) also still asks us to pick Q2

rubencarvalho · 2026-11-11T13:06:45Z

+::slotted(swc-menu-separator) {
+  --swc-menu-separator-thickness: token("divider-thickness-medium");
+
+  padding: 0;


nit: if the placeholder rule below targeted ::slotted(swc-menu-item) we wouldn't need this reset

blunteshwar added 8 commits November 2, 2026 08:58

feat(menu-separator): scaffold core and swc structure

8d515bd

feat(menu-separator): phase 3 api

48ffdf9

docs(menu-separator): phase 4 accessibility

a4ddd63

feat(menu-separator): phase 5 styling

e5ef093

fix(menu-separator): take line thickness from the menu

047dc10

test(menu-separator): phase 6 testing

00fde65

test(menu-separator): supply thickness from the menu in tests

38bf079

docs(menu-separator): phase 7 documentation

6b6e301

blunteshwar marked this pull request as ready for review November 3, 2026 06:28

blunteshwar requested a review from a team as a code owner November 3, 2026 06:28

blunteshwar added 5 commits November 3, 2026 11:58

Merge branch 'menu-separator-migration' into menu-separator-phases-2-7

69f1eed

feat(menu-separator): align phases 2-7 with the merged migration plan

96aa625

fix(menu-separator): render every story inside swc-menu

ff72b53

chore(menu-separator): update and add changesets

580de87

chore(menu-separator): merge changesets into one

e4faf92

Rajdeepc reviewed Nov 3, 2026

View reviewed changes

fix(menu-separator): address review feedback on contrast, migration g…

2a68285

…uide, and a11y spec

blunteshwar requested a review from Rajdeepc November 10, 2026 03:48

blunteshwar added the Status:Ready for review PR ready for review or re-review. label Nov 11, 2026

rubencarvalho added Component:Menu gen2 These issues or PRs map to our 2nd generation work to modernizing infrastructure. labels Nov 11, 2026

rubencarvalho reviewed Nov 11, 2026

View reviewed changes


		Place a menu separator between groups of menu items, not before the first item or after the last. The separator is not interactive and does not receive focus.

		Place it as a direct child of `swc-menu`, `swc-action-menu`, or a `swc-menu-item` submenu, never inside a `swc-menu-group`. The menu supplies the line thickness. `swc-menu-item` has not shipped yet, so the examples slot placeholder rows with `role="menuitem"`.

Conversation

blunteshwar commented Nov 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Alignment with the merged plan

Open items

Validation

Uh oh!

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

Uh oh!

⚠️ No Changeset found

Uh oh!

github-actions Bot commented Nov 2, 2026

📚 Branch Preview Links

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

blunteshwar commented Nov 2, 2026 •

edited

Loading

changeset-bot Bot commented Nov 2, 2026 •

edited

Loading