Unwind the host-mode stack when navigating to a page you are already inside - #5688
Merged
Merged
Conversation
Contributor
Preview deploymentsHost Test Results 1 files 1 suites 2h 39m 27s ⏱️ Results for commit 5d16f9d. Realm Server Test Results 1 files ±0 1 suites ±0 14m 29s ⏱️ +44s Results for commit 5d16f9d. ± Comparison against earlier commit 645d5cd. |
The published site does not go through addToHostModeStack: templates/index.gts calls HostModeStateService#pushCard, a separate stack with the same push-only behaviour. Fixing only the operator-mode service left boxel.ai — the case that matters — still stacking a duplicate of the page you came from. Tests use the existing ViewCardDemo fixture, which is already a cycle (index → secondary → tertiary → index), so tertiary's button is a real "up" link: one test walks down two levels and clicks through to the root, the other seeds a trail and unwinds to its middle. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The stack closed its top card on any click outside itself. Combined with the unwind, that undid the navigation it was reacting to: opening a card already in the trail unwinds to it, leaving that card on top, and the same click then read as an outside-click and closed it. The trail emptied, serialize() returned undefined, the hostModeStack param dropped out of the URL and the whole stack unmounted. Dismiss is now scoped to the scrim itself and ignores clicks landing inside a stack item, so the gesture is defined by where it applies rather than by exclusion. The breadcrumbs exception drops out — breadcrumbs render outside the scrim and never reached the handler. Only host mode was affected. Host submode drives the trail from stack items and breadcrumbs, so no click outside the stack existed there; a real visitor could not hit it in either mode, since the scrim covers the primary card and only a synthetic click reaches through it. Tests: the unwind by the route a visitor can take (a stack item targeting a card below it) in both modes, and dismiss-by-scrim in both. Host submode had no unwind coverage at all, so addToHostModeStack was untested. The existing query-param assertion read window.location, which is the test runner's URL and never carries app params; the new one reads currentURL. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
burieberry
force-pushed
the
cs-12434-host-mode-stack-unwind-on-ancestor
branch
from
August 5, 2026 00:57
a5a4d31 to
645d5cd
Compare
Both stacks decided push-vs-unwind with their own copy of the same branch table, so a change to one could silently diverge from the other. Move it to `unwindOrPush`, which mutates the stack in place so each service can pass its TrackedArray straight in, and cover the branch table with unit tests instead of only through two acceptance suites. Use lastIndexOf rather than indexOf to find the target: pushing can no longer duplicate a card, but the `hostModeStack` query param is accepted verbatim, so a stale or hand-written one can carry the same card twice. Going back means the nearest occurrence, not the first. Match a stack item by `data-host-mode-stack-item` instead of its class name, so the dismiss hook is a thing of its own rather than a style hook the next rename takes away. Not a `data-test-` attribute, which production builds strip. Initialize `hostModeStack` to a TrackedArray like the ones restoreState installs, so a mutation before the first restore is reactive too. Read the query param under test from currentURL(): window.location in an acceptance test is the test runner's own URL and carries no app params, so asserting null against it passed whether or not the param was cleared. Click `.inner` in the scrim test, since that fills the scrim and is what a background click lands on, and drop the older single-card backdrop test the two-deep one now covers. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The unwind path resolves a repeated card to its topmost occurrence, but close still removed the first one. Since `hostModeStack` is taken from the URL verbatim, a trail can hold the same card twice — and on ["a","b","a"], closing the top card spliced index 0 instead: the visible card stayed put while a buried one disappeared. Every close path funnels into the same `removeCardFromStack`, so the scrim, the top card's close button and the breadcrumbs all shared it. Give the two services a `removeTopmost` helper alongside `unwindOrPush`, so both trails agree that a card id resolves to the copy on screen. Sequential closes from a breadcrumb click land on the same trail either way, since they remove a whole run from the top down; a single close is where the position mattered. Drop `data-host-mode-breadcrumbs`, whose only consumer was the click-outside exceptSelector that no longer exists. Drop the acceptance test that clicked the primary card's button with a stack open: the scrim covers the primary card, and test-helpers' click does no hit-testing, so it asserted a path no visitor can take. The unit tests cover that branch directly and the test below it covers the reachable route.
burieberry
marked this pull request as ready for review
August 5, 2026 17:42
backspace
approved these changes
Aug 7, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
CS-12434
Why
Host mode only ever pushes onto its stack, so following an in-page link back to a page you are already inside stacks a second copy instead of unwinding to it.
Found on the boxel.ai blog: opening a post from the index pushes it correctly, but the post's "← Back to Boxel Blog" link then pushes the index on top of the post — index → post → index. It applies to any host-mode realm with an "up" link, and cards are only ever handed
viewCard, so realm content cannot work around it.What
Opening a card already behind you in the trail is a navigation back to it, so unwind rather than push:
Both stacks need this and were separate implementations, so the branch table now lives in
app/utils/host-mode-stack.ts, called byHostModeStateService#pushCard(the published site) andOperatorModeStateService#addToHostModeStack(host submode).Duplicate trails.
hostModeStackcomes off the URL verbatim, so?hostModeStack=["a","b","a"]is a trail the app has to handle. Every lookup resolves to the topmost occurrence — the copy the visitor can see. That also fixes close, which removed the first copy: the visible card stayed put while a buried one silently disappeared, on every close path (scrim, close button, breadcrumbs).Dismiss had to move with it. The stack closed its top card on any click outside itself, which undid the unwind — the same click that unwound to a card also read as an outside-click and closed it, emptying the trail and unmounting the stack. Dismiss is now scoped to the scrim and ignores clicks inside a stack item, matched by a
data-host-mode-stack-itemattribute that exists for that purpose (not adata-test-one, which production builds strip). The breadcrumbsexceptSelectorand itsdata-host-mode-breadcrumbshook drop out, since breadcrumbs render outside the scrim.One intentional behaviour change: the old
onClickOutsidewas document-wide, so in submode clicking the workbench chrome closed the top card. It no longer does — clicking the surrounding app furniture isn't a request to close a card, and the close button and breadcrumbs remain.Testing
Local dev stack: host mode 25/25, host submode 40/40, unit 11/11.
lint:hbs,lint:types, eslint, prettier clean.New
tests/unit/host-mode-stack-test.tscovers the branch table directly instead of only through two acceptance suites. Acceptance tests cover unwinding to the root, unwinding by the route a visitor can take, scrim dismissal, and closing a card that appears twice — that last one checked against a reverted fix, where it fails. The submode side had no unwind coverage at all, soaddToHostModeStackwas shipping untested.Two query-param assertions read
window.location, which in an acceptance test is the test runner's URL and never carries app params — so assertingnullpassed regardless of behaviour. One arrived with this PR's own new test; the other predates it onmain. Both now readcurrentURL().Two tests came out: one duplicated the new scrim test, and one clicked the primary card's button with a stack open, which the scrim covers —
@ember/test-helpers'clickdoes no hit-testing, so it asserted a path no visitor can take.Notes
Unrelated oddity left alone:
stack.gtsiterates withkey='cardId'over an array of strings, so the key resolves toundefinedfor every item. Changing it shifts DOM reuse, which the stack animations depend on, so it wants its own change with Percy on it. --> Created ticket: CS-12452 — Host-mode stack's {{#each}} key resolves to undefined for every item🤖 Generated with Claude Code