[kernel 797] add disconnected screen #121
Conversation
![tembo[bot]](https://avatars.githubusercontent.com/in/1223475?s=60&v=4){kind=small}
There was a problem hiding this comment.
A few small things to consider from the diff:
images/chromium-headful/client/src/app.vue
- There are now two
@Watch('connected', { immediate: true })handlers (onConnectedandonConnectedChange). Might be simpler/less error-prone to fold thewasConnectedbehavior into the existingonConnectedwatcher so there’s only one place reacting to connection transitions.
@Watch('connected', { immediate: true })
onConnected(connected: boolean) {
if (connected) {
this.wasConnected = true
this.applyQueryResolution()
}
}
images/chromium-headful/client/src/components/connect.vue
- Removing the non-connecting form means the overlay can render empty if we’re not
connectingbut also notconnected(e.g. transient failures, bad creds, server down). Even a minimal fallback text + retry/reload affordance would help avoid a “blank screen” state.
images/chromium-headful/client/src/components/disconnected.vue
- Since this is user-facing UI, consider using existing theme vars + i18n rather than hard-coded strings/colors so it stays consistent with the rest of the client.
images/chromium-headful/client/src/neko/base.ts
- Nice fix on the handler binding. Since you already log
code/reasonon close, it could be useful to include that detail in theErroryou pass intoonDisconnectedso the UI/toasts can surface it.
this.onDisconnected(
new Error(
`websocket closed: code=${event.code}${event.reason ? `, reason=${event.reason}` : ''}`,
),
)
images/chromium-headful/client/src/neko/index.ts
EVENT.RECONNECTINGnow callscleanup()and the comment says “Don’t … attempt reconnection”. If the intent is to fully stop reconnect attempts, it might be worth ensuring any underlying reconnect timers/sockets are also stopped, or updating the comment to match what actually happens.
There was a problem hiding this comment.
Overall direction makes sense: distinguish first-connect vs post-connect disconnect, and fixing the websocket handler bindings is definitely the right move.
A few small follow-ups:
- Consider including the websocket close
code/reasonin theErrorpassed into the disconnected path (right now it’s only in debug logs). - The new disconnected overlay likely wants an explicit
z-indexso it always sits above the video/menus. connect.vuecan render a blank overlay whenconnectingis false; making the loader unconditional avoids that.- Minor copy tweak: “shut down” reads more natural than “shutdown”.
wasConnectednever resets; if there’s ever an explicit logout/new-session flow, it can trap users on the disconnected overlay.
| this._ws.onclose = (event) => { | ||
| this.emit('debug', `websocket closed: code=${event.code}, reason=${event.reason}`) | ||
| this.onDisconnected(new Error('websocket closed')) | ||
| } |
There was a problem hiding this comment.
Nice fix on the event handler binding.
Right now we log event.code/event.reason but the user-facing Error('websocket closed') drops that context (it ends up in the disconnected notification text). Might be worth carrying the details into the Error message.
| this._ws.onclose = (event) => { | |
| this.emit('debug', `websocket closed: code=${event.code}, reason=${event.reason}`) | |
| this.onDisconnected(new Error('websocket closed')) | |
| } | |
| this._ws.onclose = (event) => { | |
| const message = `websocket closed: code=${event.code}, reason=${event.reason || 'unknown'}` | |
| this.emit('debug', message) | |
| this.onDisconnected(new Error(message)) | |
| } |
|
|
||
| <style lang="scss" scoped> | ||
| .disconnected-overlay { | ||
| position: fixed; |
There was a problem hiding this comment.
Minor UI nit: since this is a full-screen overlay, adding an explicit z-index helps ensure it always renders above the video/menus.
| position: fixed; | |
| position: fixed; | |
| z-index: 9999; |
| {{ $t('connect.connect') }} | ||
| </button> | ||
| </form> | ||
| <div class="loader" v-if="connecting"> |
There was a problem hiding this comment.
Since the login form is gone, v-if="connecting" can leave an empty overlay if connecting briefly flips false/undefined during init. Making the loader unconditional avoids a blank screen.
| <div class="loader" v-if="connecting"> | |
| <div class="loader"> |
| } | ||
|
|
||
| export const disconnected = { | ||
| message: 'Browser has been shutdown and is no longer available', |
There was a problem hiding this comment.
Tiny copy tweak: "shut down" reads a bit more natural here.
| message: 'Browser has been shutdown and is no longer available', | |
| message: 'Browser has been shut down and is no longer available', |
| <neko-side v-if="!videoOnly && side" /> | ||
| <neko-connect v-if="!connected" /> | ||
| <neko-connect v-if="!connected && !wasConnected" /> | ||
| <neko-disconnected v-if="!connected && wasConnected" /> |
There was a problem hiding this comment.
One edge case with wasConnected: if there’s an explicit logout flow (or any future “start a new session” flow), this flag never resets so you can get stuck on the disconnected overlay forever. Might be worth either (a) renaming it to something like hasEverConnected to make the semantics explicit, and/or (b) resetting it when initiating a new login attempt (or when the store indicates a new session).
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| <div class="window"> | ||
| <form class="message" v-if="!connecting" @submit.stop.prevent="connect"> | ||
| <span v-if="!autoPassword">{{ $t('connect.login_title') }}</span> | ||
| <span v-else>{{ $t('connect.invitation_title') }}</span> |
There was a problem hiding this comment.
Connect screen shows empty overlay after connection failure
Medium Severity
After removing the login form, the neko-connect component only displays content when connecting is true (showing the loader). If the initial connection attempt fails, setConnected(false) in the store sets connecting=false, resulting in neko-connect being shown (because !connected && !wasConnected) but with an empty window. Users see a blocking semi-transparent overlay with no visible content and no way to retry or interact, leaving them stuck.
Additional Locations (1)
| background: #000; | ||
| display: flex; | ||
| justify-content: center; | ||
| align-items: center; |
There was a problem hiding this comment.
Disconnected overlay missing position fixed for proper layering
High Severity
The .disconnected-overlay CSS uses width: 100vw; height: 100vh but lacks position: fixed (or position: absolute with positioning offsets). Since the parent #neko container uses display: flex; flex-direction: row, this overlay becomes a flex item laid out alongside main.neko-main rather than overlaying the content. Other overlay components like neko-connect and neko-about correctly use position: fixed/absolute with top/left/right/bottom: 0 to achieve proper overlay behavior.
rgarcia
left a comment
rgarcia
left a comment
There was a problem hiding this comment.
agree with the one comment about copy but otherwise lgtm
| } | ||
|
|
||
| export const disconnected = { | ||
| message: 'Browser has been shutdown and is no longer available', |
Note
Adds a post-connection disconnected UX and tightens connection lifecycle handling.
neko-disconnectedcomponent anddisconnectedlocale string;app.vuenow gatesneko-connectvsneko-disconnectedviawasConnectedandconnectedwasConnectedand set it upon connection to distinguish initial login from later disconnectionsconnect.vuesimplified to a loader-only view with auto-login behavior retainedonerrorbinding and detailedoncloselogging inbase.tsneko/index.ts: if WebSocket is closed during ICEdisconnected, perform cleanup (showing overlay); otherwise let WebRTC recoverWritten by Cursor Bugbot for commit 0b98dc4. This will update automatically on new commits. Configure here.