[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}
| 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', |
archandatta
force-pushed
the
archand/kernel-797/add-disconnected-screen
branch
from
0b98dc4 to
9227a23
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
|
|
||
| export const disconnected = { | ||
| message: 'Browser has been shut down and is no longer available', | ||
| } |
There was a problem hiding this comment.
Missing translation for disconnected message in non-English locales
Medium Severity
The new disconnected translation object was only added to en-us.ts, but the application supports 12 other locales (de-de, es-sp, sk-sk, sv-se, nb-no, fr-fr, ko-kr, fi-fi, ru-ru, zh-cn, zh-tw, ja-jp). When users with non-English locales view the disconnected screen, $t('disconnected.message') will fail to find the translation, causing either the raw key string to display or unexpected fallback behavior instead of a proper localized message.
Note
Introduces a post-disconnect overlay and refines connection lifecycle handling.
neko-disconnectedoverlay with i18n (disconnected.message) to inform users when the browser session is no longer availableapp.vue: showneko-connectonly before first successful connection (!connected && !wasConnected); showneko-disconnectedafter a dropped session (!connected && wasConnected); track via newwasConnectedwith watchersneko/index.ts): treat ICE hiccups vs. permanent shutdown differently—only cleanup when WebSocket is closed; remove reconnecting toastneko/base.ts): correctonerrorbinding and add detailed close logging before invoking disconnectionconnect.vue: remove manual login form; keep loader/auto-login behaviorWritten by Cursor Bugbot for commit 9227a23. This will update automatically on new commits. Configure here.