fix(02-02): restore page margins and slide transitions broken by KeepAlive restructure
All checks were successful
Demo images / Build & push demo images (push) Successful in 3m36s
All checks were successful
Demo images / Build & push demo images (push) Successful in 3m36s
The Task 3 checkpoint failed on the real preview: outer page margins broke and the up/down main-tab slide animations stopped playing. Root cause: the 02-02 restructure moved view-wrapper (absolute inset-0) onto each view's root inside the padded wrapper, and split navigation across two sibling Transitions behind a stable intermediate div — but dashboard-styles.css scopes every transition as a compound selector on .view-wrapper, which must be the keyed direct child of .perspective-container. - Restore the pre-02-02 rendered DOM exactly: single Transition whose child is a keyed div.view-wrapper containing the per-route wrapper shape - Re-integrate KeepAlive via statically-named per-route wrapper components (dashboardViewWrappers.ts) so the keyed div.view-wrapper is the cached component's own root; cache membership gated by :include on wrapper names derived from KEEP_ALIVE_PATHS - Drop the scroll-retention Map: scroll containers now live inside keyed/ cached wrappers, restoring the old reset-to-top behavior for non-kept routes - Pin the visual contract structurally in keepAliveTabs.test.ts (padded wrapper classes must render INSIDE div.view-wrapper) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
parent
ac6852893d
commit
2668705516
@ -1,126 +1,68 @@
|
||||
<template>
|
||||
<RouterView v-slot="{ Component, route }">
|
||||
<div
|
||||
ref="wrapperRef"
|
||||
class="mobile-safe-top"
|
||||
:class="wrapperClass(route)"
|
||||
:style="wrapperStyle"
|
||||
>
|
||||
<!-- KeepAlive itself is never conditionally removed — only its child
|
||||
is, via v-if on the <component> below. Toggling KeepAlive itself
|
||||
with v-if would destroy its whole instance cache on every
|
||||
navigation away from a kept-alive route, which defeats the point. -->
|
||||
<Transition :name="getTransitionName(route)">
|
||||
<KeepAlive :max="KEEP_ALIVE_MAX">
|
||||
<component
|
||||
v-if="shouldKeepAlive(route)"
|
||||
:is="Component"
|
||||
:key="route.path"
|
||||
:class="componentClass(route)"
|
||||
/>
|
||||
</KeepAlive>
|
||||
</Transition>
|
||||
<Transition :name="getTransitionName(route)">
|
||||
<Transition :name="getTransitionName(route)">
|
||||
<KeepAlive :max="KEEP_ALIVE_MAX" :include="keepAliveIncludes">
|
||||
<component
|
||||
v-if="!shouldKeepAlive(route)"
|
||||
:is="Component"
|
||||
:is="wrapperFor(route.path)"
|
||||
:key="route.path"
|
||||
:class="componentClass(route)"
|
||||
/>
|
||||
</Transition>
|
||||
<div v-if="!isFullBleedRoute(route)" class="shrink-0 h-6 md:h-12" aria-hidden="true"></div>
|
||||
</div>
|
||||
:mobile-tab-padding-top="mobileTabPaddingTop"
|
||||
:needs-mobile-back-button-space="needsMobileBackButtonSpace"
|
||||
>
|
||||
<component
|
||||
:is="Component"
|
||||
:class="isFullBleedPath(route.path) ? undefined : 'view-container flex-none'"
|
||||
/>
|
||||
</component>
|
||||
</KeepAlive>
|
||||
</Transition>
|
||||
</RouterView>
|
||||
</template>
|
||||
|
||||
<script setup lang="ts">
|
||||
// The KeepAlive host extracted from Dashboard.vue's nested RouterView (D-01).
|
||||
// The KeepAlive host extracted from Dashboard.vue's nested RouterView (D-01),
|
||||
// restructured after the Task 3 checkpoint failure (broken margins + missing
|
||||
// slide transitions) to restore the pre-02-02 rendered DOM exactly.
|
||||
//
|
||||
// Structural invariants (see 02-02-PLAN.md Task 1 for the full rationale —
|
||||
// the tests in __tests__/keepAliveTabs.test.ts pin these):
|
||||
// 1. Nothing between the RouterView slot and <KeepAlive> carries a binding
|
||||
// that changes identity per route — the outer wrapper div below is
|
||||
// stable (no :key); only its *classes* are route-derived. A keyed
|
||||
// ancestor would tear down KeepAlive's entire instance cache on every
|
||||
// navigation.
|
||||
// 2. Composition order is <Transition> outside <KeepAlive> outside
|
||||
// <component :is="Component">.
|
||||
// 3. :key="route.path" lives on <component :is> itself, never on an
|
||||
// ancestor of <KeepAlive> — putting it on a wrapping div would make
|
||||
// KeepAlive cache that div (keyed, torn down every navigation) instead
|
||||
// of the view.
|
||||
// 4. Both pre-existing wrapper shapes (full-bleed chat/mesh vs. the default
|
||||
// padded/scrollable shape) survive byte-for-byte via wrapperClass() /
|
||||
// wrapperStyle() / componentClass() below.
|
||||
import { computed, nextTick, onBeforeUnmount, ref, watch } from 'vue'
|
||||
import { RouterView, useRoute, type RouteLocationNormalizedLoaded } from 'vue-router'
|
||||
// Structural invariants — the tests in __tests__/keepAliveTabs.test.ts pin
|
||||
// these, and dashboard-styles.css is the contract they serve:
|
||||
//
|
||||
// 1. The <Transition> child is a keyed `div.view-wrapper` that sits directly
|
||||
// in the RouterView slot (so it is the direct child of
|
||||
// `.perspective-container` in Dashboard.vue). Every transition in
|
||||
// dashboard-styles.css is a compound selector
|
||||
// (`.slide-up-enter-active.view-wrapper`, `.depth-forward-enter-from.view-wrapper`,
|
||||
// …), so the transition classes and `view-wrapper` MUST share one element,
|
||||
// with no intermediate wrapper flattening the 3D perspective or clipping
|
||||
// slide movement.
|
||||
// 2. The per-route wrapper shapes (full-bleed chat/mesh vs. the default
|
||||
// padded/scrollable shape) live INSIDE `div.view-wrapper` — `view-wrapper`
|
||||
// is `absolute inset-0` and must never be applied to the view's own root
|
||||
// inside the padded wrapper (that pins the view over the page padding:
|
||||
// the broken-margins regression).
|
||||
// 3. KeepAlive caching is reconciled with (1) and (2) by making the keyed
|
||||
// `div.view-wrapper` the root of a statically-defined per-route wrapper
|
||||
// component (dashboardViewWrappers.ts). KeepAlive sits between
|
||||
// <Transition> and the keyed wrapper vnode — the canonical composition —
|
||||
// and `:include` (matching the wrappers' static names, derived from
|
||||
// KEEP_ALIVE_PATHS) decides which wrappers are instance-cached. Caching
|
||||
// the wrapper caches its whole subtree, including the routed view.
|
||||
// 4. KeepAlive itself is never keyed, toggled with v-if, or nested under a
|
||||
// per-route element — any of those tears down its instance cache.
|
||||
//
|
||||
// Scroll behavior: each route's scroll container lives inside its keyed
|
||||
// wrapper, so non-kept routes reset to top on entry (the pre-02-02 behavior)
|
||||
// and the kept-alive tab's scroll container is part of its cached subtree —
|
||||
// no manual scroll-retention bookkeeping needed.
|
||||
import { RouterView } from 'vue-router'
|
||||
import { useRouteTransitions } from './useRouteTransitions'
|
||||
import { KEEP_ALIVE_MAX, shouldKeepAlive } from './keepAliveRoutes'
|
||||
import { KEEP_ALIVE_MAX } from './keepAliveRoutes'
|
||||
import { isFullBleedPath, keepAliveIncludeNames, wrapperFor } from './dashboardViewWrappers'
|
||||
|
||||
const props = defineProps<{
|
||||
defineProps<{
|
||||
mobileTabPaddingTop: number | null
|
||||
needsMobileBackButtonSpace: boolean
|
||||
}>()
|
||||
|
||||
const route = useRoute()
|
||||
const { getTransitionName } = useRouteTransitions()
|
||||
const wrapperRef = ref<HTMLElement | null>(null)
|
||||
|
||||
type RouteLike = RouteLocationNormalizedLoaded | { path: string }
|
||||
|
||||
function isFullBleedRoute(r: RouteLike): boolean {
|
||||
return r.path === '/dashboard/chat' || r.path === '/dashboard/mesh'
|
||||
}
|
||||
|
||||
/** Classes for the stable outer wrapper — was previously split across two
|
||||
* v-if/v-else divs; unified here into one so it never needs a per-route key
|
||||
* (invariant 1) while still producing the exact same visual result. */
|
||||
function wrapperClass(r: RouteLike) {
|
||||
if (isFullBleedRoute(r)) {
|
||||
return [
|
||||
'h-full',
|
||||
r.path === '/dashboard/mesh' ? 'dashboard-scroll-panel mobile-scroll-pad mesh-dashboard-panel' : '',
|
||||
props.mobileTabPaddingTop ? 'overflow-y-auto' : '',
|
||||
]
|
||||
}
|
||||
return [
|
||||
'absolute inset-0 px-4 pt-4 md:pt-8 md:px-8 overflow-y-auto dashboard-scroll-panel',
|
||||
props.needsMobileBackButtonSpace ? 'mobile-scroll-pad-back' : 'mobile-scroll-pad',
|
||||
]
|
||||
}
|
||||
|
||||
const wrapperStyle = computed(() =>
|
||||
props.mobileTabPaddingTop ? { paddingTop: `${props.mobileTabPaddingTop + 16}px` } : undefined,
|
||||
)
|
||||
|
||||
/** Classes merged onto the transitioning component's own root element via
|
||||
* Vue's attribute fallthrough. `view-wrapper` carries the transition/
|
||||
* positioning CSS (dashboard-styles.css) that used to live on the outer,
|
||||
* now-stable wrapper div; `view-container flex-none` matches the default
|
||||
* shape's previous per-component class. */
|
||||
function componentClass(r: RouteLike): string {
|
||||
return isFullBleedRoute(r) ? 'view-wrapper' : 'view-wrapper view-container flex-none'
|
||||
}
|
||||
|
||||
// ── Per-route scroll retention ─────────────────────────────────────────────
|
||||
// The default shape's wrapper is the scroll container and is now stable
|
||||
// across navigations (no key), so without this a kept-alive tab would
|
||||
// inherit whatever scroll offset the previous tab left behind — worse than
|
||||
// the old reset-to-top. Bounded by the route table (open item, accepted per
|
||||
// 02-02-PLAN.md assumptions).
|
||||
const scrollPositions = new Map<string, number>()
|
||||
|
||||
watch(
|
||||
() => route.path,
|
||||
(newPath, oldPath) => {
|
||||
if (wrapperRef.value && oldPath) scrollPositions.set(oldPath, wrapperRef.value.scrollTop)
|
||||
nextTick(() => {
|
||||
if (wrapperRef.value) wrapperRef.value.scrollTop = scrollPositions.get(newPath) ?? 0
|
||||
})
|
||||
},
|
||||
)
|
||||
|
||||
onBeforeUnmount(() => {
|
||||
if (wrapperRef.value) scrollPositions.set(route.path, wrapperRef.value.scrollTop)
|
||||
})
|
||||
const keepAliveIncludes = keepAliveIncludeNames()
|
||||
</script>
|
||||
|
||||
@ -101,6 +101,56 @@ describe('DashboardRouterView keep-alive behavior', () => {
|
||||
expect(shouldKeepAlive({ path: '/dashboard/marketplace' })).toBe(true)
|
||||
expect(shouldKeepAlive({ path: '/dashboard/marketplace/abc' })).toBe(false)
|
||||
})
|
||||
|
||||
// Visual contract (Task 3 checkpoint regression): the padded default-shape
|
||||
// wrapper must render INSIDE `div.view-wrapper`, never the other way around.
|
||||
// dashboard-styles.css scopes every transition as a compound selector
|
||||
// (`.slide-up-enter-active.view-wrapper`, …) and `.view-wrapper` is
|
||||
// `absolute inset-0` — if `view-wrapper` ever moves onto the view root
|
||||
// inside the padded wrapper, page margins break and the slide/depth
|
||||
// transitions stop firing. This test pins the structure so that regression
|
||||
// cannot silently return.
|
||||
it('renders the padded default wrapper inside div.view-wrapper', async () => {
|
||||
const router = makeRouter()
|
||||
router.push('/dashboard/other')
|
||||
await router.isReady()
|
||||
|
||||
const wrapper = mount(DashboardRouterView, {
|
||||
props: { mobileTabPaddingTop: null, needsMobileBackButtonSpace: false },
|
||||
global: { plugins: [router] },
|
||||
})
|
||||
await flushPromises()
|
||||
|
||||
const viewWrapper = wrapper.find('.view-wrapper')
|
||||
expect(viewWrapper.exists()).toBe(true)
|
||||
|
||||
// view-wrapper is the bare transition surface — no padding classes on it.
|
||||
expect(viewWrapper.classes()).not.toContain('px-4')
|
||||
expect(viewWrapper.classes()).not.toContain('view-container')
|
||||
|
||||
// Its first child is the padded scroll container carrying the page margins.
|
||||
const padded = viewWrapper.element.firstElementChild as HTMLElement
|
||||
expect(padded).not.toBeNull()
|
||||
for (const cls of [
|
||||
'absolute', 'inset-0', 'px-4', 'pt-4', 'md:pt-8', 'md:px-8',
|
||||
'overflow-y-auto', 'mobile-safe-top', 'dashboard-scroll-panel', 'mobile-scroll-pad',
|
||||
]) {
|
||||
expect(padded.classList.contains(cls), `padded wrapper missing ${cls}`).toBe(true)
|
||||
}
|
||||
|
||||
// The routed view sits inside the padded wrapper with its per-component
|
||||
// classes, followed by the scroll-clearance spacer.
|
||||
const view = wrapper.find('.other-stub')
|
||||
expect(view.exists()).toBe(true)
|
||||
expect(view.classes()).toContain('view-container')
|
||||
expect(view.classes()).toContain('flex-none')
|
||||
expect(padded.contains(view.element)).toBe(true)
|
||||
const spacer = padded.querySelector('[aria-hidden="true"]')
|
||||
expect(spacer).not.toBeNull()
|
||||
expect(spacer!.classList.contains('shrink-0')).toBe(true)
|
||||
|
||||
wrapper.unmount()
|
||||
})
|
||||
})
|
||||
|
||||
describe('RefreshIndicator', () => {
|
||||
|
||||
BIN
neode-ui/src/views/dashboard/dashboardViewWrappers.ts
Normal file
BIN
neode-ui/src/views/dashboard/dashboardViewWrappers.ts
Normal file
Binary file not shown.
Loading…
x
Reference in New Issue
Block a user