fix(ui): teleported nav must not outlive the screen that raised it
Demo images / Build & push demo images (push) Successful in 3m30s
Demo images / Build & push demo images (push) Successful in 3m30s
Reported: the nav above the bottom bar — back buttons, the mesh tabs —
stayed stuck across other screens.
Cause is the KeepAlive work from phase 2, and specifically the half of it
that is invisible from the view's own file. Main tabs are KeepAlive'd, so
navigating DEACTIVATES a view instead of unmounting it. Content the view
Teleports to <body> is not in the view's DOM subtree, so deactivation
does not remove it and it keeps rendering over the destination screen.
Two offenders, matching the report exactly:
- Mesh.vue teleports its mobile TAB BAR and its chat BACK BUTTON to
<body>, gated only on `mobileShowChat` — never on whether Mesh was the
screen you were looking at.
- components/BackButton.vue teleports the shared mobile back button with
NO gate at all, so it leaked out of every view that uses it. Fixing the
shared component fixes every caller at once: Vue propagates
activated/deactivated from the KeepAlive boundary down through the
subtree, so a child can guard itself.
BaseModal already solved the transient-dialog half of this class in
204d4523 by closing on route change. That is the right fix for a dialog
and the wrong one for chrome: a tab bar has no "closed" state to fall
back to, and forcing one would lose the user's place. New
useViewActive() composable instead — chrome is simply not rendered while
its owner is off screen, and returns exactly as it was.
THE PERFORMANCE IS NOT SACRIFICED, which was the explicit constraint.
The Teleport is gated, not the view, so the instance stays cached and
revisiting a tab is still instant. A test pins this: setup() must run
exactly ONCE across a navigate-away-and-back round trip. If someone
later "fixes" this by dropping KeepAlive, that test fails.
Deliberately untouched: AppSession.vue, whose teleport is load-bearing —
its own comment records that moving the iframe node reloads the app, and
app-session is excluded from KeepAlive anyway so it cannot leak. Toasts,
the app launcher and the connection banner are app-level rather than
view-owned; gating those would be wrong.
Verified: 3 new tests; full suite 105 files / 848 tests green.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
b945738d62
commit
a5b923fa0b
@@ -12,8 +12,14 @@
|
||||
{{ label }}
|
||||
</button>
|
||||
|
||||
<!-- Mobile: floating transparent button pinned 8px above the tab bar -->
|
||||
<Teleport to="body">
|
||||
<!-- Mobile: floating transparent button pinned 8px above the tab bar.
|
||||
Gated on the owning view being active: this is Teleported to <body>, so
|
||||
it lives outside the view's own subtree and KeepAlive deactivating the
|
||||
owner does NOT remove it — the button stayed pinned above the tab bar on
|
||||
every other screen. activated/deactivated propagate from the KeepAlive
|
||||
boundary down to this child, so the shared component can guard itself and
|
||||
every caller is fixed at once. -->
|
||||
<Teleport v-if="isViewActive" to="body">
|
||||
<button
|
||||
type="button"
|
||||
@click="$emit('click')"
|
||||
@@ -28,6 +34,7 @@
|
||||
</template>
|
||||
|
||||
<script setup lang="ts">
|
||||
import { useViewActive } from '@/composables/useViewActive'
|
||||
/**
|
||||
* Standard back button. Renders a transparent text link at the top on desktop
|
||||
* and a floating transparent "glass" pill pinned above the tab bar on mobile —
|
||||
@@ -36,6 +43,8 @@
|
||||
* Presentational only: it emits `click`; the parent keeps its own navigation
|
||||
* logic (router.push / router.back / conditional goBack).
|
||||
*/
|
||||
const isViewActive = useViewActive()
|
||||
|
||||
withDefaults(
|
||||
defineProps<{
|
||||
label?: string
|
||||
|
||||
@@ -0,0 +1,103 @@
|
||||
import { describe, it, expect } from 'vitest'
|
||||
import { mount } from '@vue/test-utils'
|
||||
import { defineComponent, h, KeepAlive, ref, Teleport } from 'vue'
|
||||
import { useViewActive } from '../useViewActive'
|
||||
|
||||
/**
|
||||
* Teleported chrome must not outlive the screen that raised it.
|
||||
*
|
||||
* Main tabs are KeepAlive'd, so navigating away deactivates a view instead of
|
||||
* unmounting it. Anything Teleported to <body> is outside the view's subtree
|
||||
* and therefore survives that deactivation — Mesh's mobile tab bar and the
|
||||
* shared BackButton stayed pinned above the bottom bar on every other screen.
|
||||
*/
|
||||
const ViewWithTeleportedChrome = defineComponent({
|
||||
name: 'ViewWithTeleportedChrome',
|
||||
setup() {
|
||||
const isViewActive = useViewActive()
|
||||
return () =>
|
||||
h('div', [
|
||||
isViewActive.value
|
||||
? h(Teleport, { to: 'body' }, [h('button', { class: 'leaky-chrome' }, 'Back')])
|
||||
: null,
|
||||
])
|
||||
},
|
||||
})
|
||||
|
||||
const Other = defineComponent({ name: 'Other', setup: () => () => h('div', 'other screen') })
|
||||
|
||||
function chromeCount() {
|
||||
return document.body.querySelectorAll('.leaky-chrome').length
|
||||
}
|
||||
|
||||
describe('useViewActive', () => {
|
||||
it('removes teleported chrome when the view is deactivated, and restores it on return', async () => {
|
||||
const showFirst = ref(true)
|
||||
const host = mount(
|
||||
defineComponent({
|
||||
setup: () => () =>
|
||||
h(KeepAlive, null, {
|
||||
default: () => (showFirst.value ? h(ViewWithTeleportedChrome) : h(Other)),
|
||||
}),
|
||||
}),
|
||||
{ attachTo: document.body },
|
||||
)
|
||||
|
||||
expect(chromeCount()).toBe(1)
|
||||
|
||||
// Navigate away: KeepAlive DEACTIVATES rather than unmounts.
|
||||
showFirst.value = false
|
||||
await host.vm.$nextTick()
|
||||
expect(chromeCount()).toBe(0)
|
||||
|
||||
// Returning must bring it back — the whole point of KeepAlive is that the
|
||||
// instance survived, so the chrome has to come back with it.
|
||||
showFirst.value = true
|
||||
await host.vm.$nextTick()
|
||||
expect(chromeCount()).toBe(1)
|
||||
|
||||
host.unmount()
|
||||
})
|
||||
|
||||
it('keeps the instance alive across the round trip (performance is not sacrificed)', async () => {
|
||||
const showFirst = ref(true)
|
||||
const seen: number[] = []
|
||||
const Counting = defineComponent({
|
||||
name: 'Counting',
|
||||
setup() {
|
||||
const isViewActive = useViewActive()
|
||||
const uid = Math.random()
|
||||
seen.push(uid)
|
||||
return () => h('div', [isViewActive.value ? h(Teleport, { to: 'body' }, [h('i', { class: 'leaky-chrome' })]) : null])
|
||||
},
|
||||
})
|
||||
|
||||
const host = mount(
|
||||
defineComponent({
|
||||
setup: () => () =>
|
||||
h(KeepAlive, null, { default: () => (showFirst.value ? h(Counting) : h(Other)) }),
|
||||
}),
|
||||
{ attachTo: document.body },
|
||||
)
|
||||
|
||||
showFirst.value = false
|
||||
await host.vm.$nextTick()
|
||||
showFirst.value = true
|
||||
await host.vm.$nextTick()
|
||||
|
||||
// setup() ran once: the view was cached, not re-created. If this ever
|
||||
// becomes 2, the fix has been "solved" by throwing away the perf work.
|
||||
expect(seen.length).toBe(1)
|
||||
expect(chromeCount()).toBe(1)
|
||||
|
||||
host.unmount()
|
||||
})
|
||||
|
||||
it('defaults to active outside a KeepAlive boundary', () => {
|
||||
// Neither hook fires here. A component used both ways — or mounted bare in
|
||||
// a test — must render normally rather than stay invisible forever.
|
||||
const host = mount(ViewWithTeleportedChrome, { attachTo: document.body })
|
||||
expect(chromeCount()).toBe(1)
|
||||
host.unmount()
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,41 @@
|
||||
import { ref, onActivated, onDeactivated, type Ref } from 'vue'
|
||||
|
||||
/**
|
||||
* Is this view the one currently on screen?
|
||||
*
|
||||
* Main tab views are `KeepAlive`'d (see `keepAliveRoutes.ts`), which is what
|
||||
* makes revisiting a tab instant — navigating away DEACTIVATES the view rather
|
||||
* than unmounting it. That is exactly the behaviour we want for performance, and
|
||||
* exactly the trap for anything the view `Teleport`s to `<body>`: teleported
|
||||
* content is not inside the view's own DOM subtree, so it keeps rendering over
|
||||
* whatever screen the user went to next. Mesh's mobile tab bar and its chat
|
||||
* back button did this — they stayed pinned above the bottom bar on every other
|
||||
* screen.
|
||||
*
|
||||
* `BaseModal` solves the transient-dialog half of this by closing itself on any
|
||||
* route change. That is the right fix for a dialog and the wrong one for
|
||||
* persistent chrome: a tab bar has no "closed" state to fall back to, and
|
||||
* forcing one would lose the user's place. Chrome should simply not be rendered
|
||||
* while its owner is off screen, and should come back exactly as it was.
|
||||
*
|
||||
* Usage — gate the `Teleport` itself, so nothing reaches `<body>` at all:
|
||||
*
|
||||
* const isViewActive = useViewActive()
|
||||
* <Teleport v-if="isViewActive" to="body"> … </Teleport>
|
||||
*
|
||||
* Defaults to `true`: outside a `KeepAlive` boundary neither hook ever fires, so
|
||||
* a view used both ways (or a component mounted bare in a test) must render
|
||||
* normally rather than stay invisible forever. Inside `KeepAlive`, Vue fires
|
||||
* `onActivated` immediately after `onMounted`, so the initial `true` is correct
|
||||
* there too and there is no first-paint gap.
|
||||
*/
|
||||
export function useViewActive(): Ref<boolean> {
|
||||
const isViewActive = ref(true)
|
||||
onActivated(() => {
|
||||
isViewActive.value = true
|
||||
})
|
||||
onDeactivated(() => {
|
||||
isViewActive.value = false
|
||||
})
|
||||
return isViewActive
|
||||
}
|
||||
@@ -1,6 +1,7 @@
|
||||
<script setup lang="ts">
|
||||
import { ref, computed, watch, nextTick, onActivated, onDeactivated, onMounted, onUnmounted } from 'vue'
|
||||
import { useRoute } from 'vue-router'
|
||||
import { useViewActive } from '@/composables/useViewActive'
|
||||
import { useMeshStore } from '@/stores/mesh'
|
||||
import { useTransportStore } from '@/stores/transport'
|
||||
import type { MeshMessage, MeshPeer, SessionStatus } from '@/stores/mesh'
|
||||
@@ -24,6 +25,15 @@ const mesh = useMeshStore()
|
||||
const transport = useTransportStore()
|
||||
const route = useRoute()
|
||||
|
||||
// Everything this view Teleports to <body> — the mobile tab bar, the chat back
|
||||
// button, and the two transport choosers — lives OUTSIDE the view's own DOM
|
||||
// subtree, so KeepAlive deactivating this view does not remove it. Without this
|
||||
// gate the tab bar and back button stayed pinned above the bottom bar on every
|
||||
// other screen. Gating the Teleport (not its child) means nothing reaches
|
||||
// <body> at all while Mesh is off screen; the view itself stays alive, so
|
||||
// revisiting it is still instant.
|
||||
const isViewActive = useViewActive()
|
||||
|
||||
// "Flash LoRa" header button target: the connected radio wins, else any
|
||||
// detected-but-unconnected stick. Opens the global setup modal at its
|
||||
// flash step (backend stops the listener and frees the port itself).
|
||||
@@ -2316,7 +2326,7 @@ async function downloadAttachment(payload: MeshAttachmentPayload) {
|
||||
<!-- Mobile: floating back button (shared glass pill style), pinned
|
||||
above the tab bar — replaces the in-header arrow so the back
|
||||
control is no longer crammed inside the chat container. -->
|
||||
<Teleport to="body">
|
||||
<Teleport v-if="isViewActive" to="body">
|
||||
<button
|
||||
type="button"
|
||||
class="mesh-chat-mobile-back mobile-back-btn back-button-glass px-6 py-3 rounded-xl font-medium items-center justify-center gap-2"
|
||||
@@ -2707,7 +2717,7 @@ async function downloadAttachment(payload: MeshAttachmentPayload) {
|
||||
placement as the mobile back button). Switches the whole pane between
|
||||
the chat and each tool. Hidden while an individual conversation is open
|
||||
(the back button takes over there). -->
|
||||
<Teleport to="body">
|
||||
<Teleport v-if="isViewActive" to="body">
|
||||
<div v-show="!mobileShowChat" class="mesh-mobile-tabbar">
|
||||
<button class="mesh-mtab" :class="{ active: mobileTab === 'chat' }" @click="selectMobileTab('chat')">Chat</button>
|
||||
<button class="mesh-mtab" :class="{ active: mobileTab === 'bitcoin' }" @click="selectMobileTab('bitcoin')">BTC</button>
|
||||
@@ -2726,7 +2736,7 @@ async function downloadAttachment(payload: MeshAttachmentPayload) {
|
||||
Teleported to body so the fixed backdrop covers the FULL viewport —
|
||||
rendered in place it sits inside a transformed/filtered glass panel,
|
||||
which traps position:fixed to just the right chat panel. -->
|
||||
<Teleport to="body">
|
||||
<Teleport v-if="isViewActive" to="body">
|
||||
<div v-if="transportChoice" class="mesh-transport-modal-backdrop" @click.self="pickTransport('cancel')">
|
||||
<div class="glass-card mesh-transport-modal">
|
||||
<h3 class="mesh-transport-title">📎 How should I send this?</h3>
|
||||
@@ -2761,7 +2771,7 @@ async function downloadAttachment(payload: MeshAttachmentPayload) {
|
||||
Each preset shows its nominal size target + a transfer-time estimate
|
||||
from the same mesh.transport-advice RPC the file-attach flow uses.
|
||||
Teleported to body — see the transport chooser above. -->
|
||||
<Teleport to="body">
|
||||
<Teleport v-if="isViewActive" to="body">
|
||||
<div v-if="imageQualityChoice" class="mesh-transport-modal-backdrop" @click.self="pickImageQuality(null)">
|
||||
<div class="glass-card mesh-transport-modal">
|
||||
<h3 class="mesh-transport-title">🖼️ Choose Image Quality</h3>
|
||||
|
||||
Reference in New Issue
Block a user