Skip to content

fix(site-plan): share identical responsive variant menus - #2249

Closed
chubes4 wants to merge 8 commits into
trunkfrom
fix/shared-variant-navigation
Closed

chubes4 wants to merge 8 commits into
trunkfrom
fix/shared-variant-navigation

Conversation

@chubes4

@chubes4 chubes4 commented Sep 26, 2026

Copy link
Copy Markdown
Contributor

Problem

When a capture has desktop and mobile document variants, each viewport's menu is a separate inline core/navigation. Renaming an item in the Site Editor updates only that block. The other viewport keeps the old label.

Depends on #2245 (fix/desktop-menu-navigation) and must merge after it. This branch is rebased onto that branch; the commits here are only the shared-entity change.

Root cause

Variant menus are intentionally kept as separate core/navigation blocks so each viewport can keep its own presentation. Nothing then bound those blocks to one wp_navigation post. Identical ordered (label, destination) sets stayed inline, so an edit could not propagate.

Responsive document classes often live on layout-shell wrapper attributes, not only className, so a className-only scope check misses the pair. A sentinel ref also cannot be left in stored page content: the navigation renderer reads attributes before a render filter can replace them, and an unresolved ref drops the inline items.

Fix

After shell extraction, group core/navigation blocks that sit in different responsive variant scopes and share an ordered (label, destination) set (fragments ignored, matching shell identity). Both blocks get the same ref. Divergent item sets stay inline and independent. Variant attributes (overlayMenu, classes) stay on each block.

Theme bootstrap creates one wp_navigation post for that item set and, on init, writes the real post ID into stored page/template content so render and the Site Editor both bind to it. A later rename is not overwritten.

Regression test

php php-transformer/tests/unit/shared-variant-navigation.php

Fails before (git stash of the fix):

FAIL: identical variant menus share one navigation ref - []
FAIL: identical variant menus emit one wp_navigation entity - 0
FAIL: the shared entity carries the identical item set
FAIL: in-document identical variant menus share one navigation ref - []
FAIL: in-document identical variant menus emit one wp_navigation entity
Shared variant navigation: 5 failed, 8 passed

Passes after:

Shared variant navigation passed: 13 assertions

Also passed: php php-transformer/tests/contract/shared-shell-plan.php, php php-transformer/tests/contract/wordpress-site-plan.php, php php-transformer/tests/contract/header-link-cluster-navigation.php, php php-transformer/tests/unit/paragraph-label-desktop-menu.php.

Verification

Fresh import with SSI origin/main (9c83decd) paired to this implementation via studio create --static-site-importer-path.

  • 11 content pages have two core/navigation blocks sharing one ref. A third one-item menu on some routes stays unreferenced.
  • One rename of the blog-route item to Journal in the wp_navigation post (the Navigation screen write): /, /blog, /shop, and /services-1 HTML each contain Journal twice (both variant navs). >Blog< is gone.
  • At 1440 on those routes: 1 visible header, Journal visible, first menu link y=45. x=580 versus the fix(php-transformer): hoist viewport-partitioned chrome into shared parts #2243 baseline x=733 (y=45); that offset is the parent fix(php-transformer): recognize paragraph-labelled menus inside custom-element hosts #2245 residual, not this change.
  • At 390: 1 visible header. Both nav elements contain the renamed item (5 items each). At rest both navs have 0 layout size, matching the baseline which also does not show an open mobile menu until a toggle.

Fixes #2246

AI disclosure: implemented by xAI Grok 4.7 via OpenCode (opencode run), orchestrated and reviewed by Claude (Anthropic).

…m-element hosts

A presentation-only custom element wrapping a nav was captured as companion
HTML before NavigationPattern could see the landmark, so paragraph-wrapped
menu labels never became editable navigation links.
…nition

A custom-element nav that also carries a display:none accessibility hint
must stay on the path that preserves that hint, instead of being replaced
by a navigation block that drops it.
A block list's text-align and a row flex list's justify-content were
dropped when the list became a flex core/navigation, so items started
at the container edge instead of the source position.
layout.justifyContent packs a wrapping column on the cross axis, so a
right-aligned list still started at the container edge. Restate the
source packing as a row on the generated list.
A global row rule also right-aligned other menus. Hang the packing
declarations off the source list's own classes.
When desktop and mobile menus have the same ordered items, both core/navigation blocks reference one wp_navigation entity so a single rename updates every viewport.
Responsive document classes can live on layout-shell wrapper attributes, so identical variant menus still share one navigation entity.
Page content is parsed before render filters can replace a sentinel ref, so init writes the created wp_navigation id into stored blocks.
@chubes4

chubes4 commented Sep 26, 2026

Copy link
Copy Markdown
Contributor Author

Closing as superseded by #2248 (plan-level shared navigation entity + SSI #1865 persistence), which covers #2246 without a runtime content rewrite. #2248 will be rebased onto trunk (now including #2245).

AI disclosure: Claude (Anthropic) via OpenCode.

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.

Desktop and mobile variant menus are separate inline navigations; one menu edit should update both

1 participant