Skip to content

Commit 0645e5a

Browse files
sarg3ntclaude
andcommitted
address Copilot review findings on PR #141
- ARIA tab semantics: move role="tab" onto the label button so nested interactive elements (dot, close) are siblings of the tabbable element, not nested inside it. Wrapper is now role="presentation". - Per-tab status dot: add aria-label that reflects current state and reconnect action; aria-disabled when inert. - Toolbar buttons (clear, layout-drawer/dock/popout, close, +): add explicit aria-label; mark icon SVGs aria-hidden so screen readers don't try to announce path data. - Capabilities fetch failures and websocket errors now also write a red [console] line into the xterm buffer, so a failing session isn't a silent black void — matches the existing 'err' frame path. - Dock resize: switch from mouse events to Pointer Events with setPointerCapture/releasePointerCapture, plus blur and visibilitychange cleanups. Prevents stuck drag state when the mouse releases outside the window. - Fix CSS selector for .xterm height: actual DOM is .console-xterm-pad > #console-xterm > .console-session-host > .xterm — old selector skipped the #console-xterm wrapper, so the rule was inert and the terminal didn't always fill its container. - Popout templ doc comment was stale (mentioned overriding the close button); CSS now hides the entire header in popout mode, comment updated to match. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 94ce533 commit 0645e5a

4 files changed

Lines changed: 93 additions & 33 deletions

File tree

gearbox/internal/framework/templates/components/console.templ

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,9 @@ templ ConsoleDrawer() {
5151
<button type="button"
5252
id="console-btn-clear"
5353
class="console-tool-btn"
54+
aria-label="Clear terminal buffer"
5455
title="Clear (Ctrl-L sends to remote; this just clears the local buffer)">
55-
<svg class="w-4 h-4" fill="none" stroke="currentColor" viewBox="0 0 24 24">
56+
<svg class="w-4 h-4" aria-hidden="true" fill="none" stroke="currentColor" viewBox="0 0 24 24">
5657
<path stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M19 7l-.867 12.142A2 2 0 0116.138 21H7.862a2 2 0 01-1.995-1.858L5 7m5 4v6m4-6v6M1 7h22M9 7V4a1 1 0 011-1h4a1 1 0 011 1v3"></path>
5758
</svg>
5859
</button>
@@ -63,33 +64,37 @@ templ ConsoleDrawer() {
6364
<button type="button"
6465
id="console-btn-layout-drawer"
6566
class="console-tool-btn"
67+
aria-label="Switch to fullscreen layout"
6668
title="Fullscreen drawer">
67-
<svg class="w-4 h-4" fill="none" stroke="currentColor" viewBox="0 0 24 24">
69+
<svg class="w-4 h-4" aria-hidden="true" fill="none" stroke="currentColor" viewBox="0 0 24 24">
6870
<path stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M4 8V4m0 0h4M4 4l5 5m11-1V4m0 0h-4m4 0l-5 5M4 16v4m0 0h4m-4 0l5-5m11 5l-5-5m5 5v-4m0 4h-4"></path>
6971
</svg>
7072
</button>
7173
<button type="button"
7274
id="console-btn-layout-dock"
7375
class="console-tool-btn"
76+
aria-label="Switch to docked layout"
7477
title="Dock to bottom">
75-
<svg class="w-4 h-4" fill="none" stroke="currentColor" viewBox="0 0 24 24">
78+
<svg class="w-4 h-4" aria-hidden="true" fill="none" stroke="currentColor" viewBox="0 0 24 24">
7679
<rect x="3" y="4" width="18" height="16" rx="2" stroke-width="2"></rect>
7780
<line x1="3" y1="14" x2="21" y2="14" stroke-width="2"></line>
7881
</svg>
7982
</button>
8083
<button type="button"
8184
id="console-btn-layout-popout"
8285
class="console-tool-btn"
86+
aria-label="Open console in new window"
8387
title="Open in new window">
84-
<svg class="w-4 h-4" fill="none" stroke="currentColor" viewBox="0 0 24 24">
88+
<svg class="w-4 h-4" aria-hidden="true" fill="none" stroke="currentColor" viewBox="0 0 24 24">
8589
<path stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M10 6H6a2 2 0 00-2 2v10a2 2 0 002 2h10a2 2 0 002-2v-4M14 4h6m0 0v6m0-6L10 14"></path>
8690
</svg>
8791
</button>
8892
<button type="button"
8993
id="console-drawer-close"
9094
class="console-tool-btn"
95+
aria-label="Close console"
9196
title="Close console (Esc)">
92-
<svg class="w-4 h-4" fill="none" stroke="currentColor" viewBox="0 0 24 24">
97+
<svg class="w-4 h-4" aria-hidden="true" fill="none" stroke="currentColor" viewBox="0 0 24 24">
9398
<path stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M6 18L18 6M6 6l12 12"></path>
9499
</svg>
95100
</button>
@@ -106,7 +111,7 @@ templ ConsoleDrawer() {
106111
class="console-tab-new"
107112
aria-label="New tab"
108113
title="Open another tab to this box">
109-
<svg class="w-3 h-3" fill="none" stroke="currentColor" viewBox="0 0 24 24">
114+
<svg class="w-3 h-3" aria-hidden="true" fill="none" stroke="currentColor" viewBox="0 0 24 24">
110115
<path stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M12 4v16m8-8H4"></path>
111116
</svg>
112117
</button>

gearbox/internal/framework/templates/pages/console_popout.templ

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,14 @@ import (
1111
// — just the ConsoleDrawer in fullscreen mode plus the assets needed to
1212
// drive it.
1313
//
14-
// On load the inline boot script calls markPopout (which hides the
15-
// layout-switch buttons that don't apply here) and opens a session
16-
// against the given box. Closing the drawer here means closing the
17-
// browser tab — handled by overriding the close button to window.close().
14+
// On load the inline boot script calls markPopout which:
15+
// 1. Tags the drawer with .console-popout so CSS hides the entire
16+
// header (incl. the close button) and the tab bar — the popout
17+
// window is single-session, all chrome controls are meaningless.
18+
// 2. Forces the layout to drawer (fullscreen) and disables Esc-to-close.
19+
//
20+
// The only way to exit a popout is closing the OS window (Cmd-W /
21+
// red close button) — by design, since the chrome's gone anyway.
1822
templ ConsolePopoutPage(boxID string, boxName string) {
1923
<!DOCTYPE html>
2024
<html lang="en" class="h-full">

gearbox/static/css/components/console.css

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -247,7 +247,13 @@ body.console-dock-open {
247247
}
248248

249249
/* The .xterm viewport already handles its own scrollbar; suppress the
250-
* outer wrapper's so we don't get two stacked tracks during transitions. */
251-
.console-xterm-pad > .console-session-host > .xterm {
250+
* outer wrapper's so we don't get two stacked tracks during transitions.
251+
* DOM hierarchy is .console-xterm-pad > #console-xterm > .console-session-host > .xterm
252+
* — sessions append their host into #console-xterm. */
253+
.console-xterm-pad #console-xterm > .console-session-host > .xterm {
254+
height: 100%;
255+
}
256+
#console-xterm {
257+
width: 100%;
252258
height: 100%;
253259
}

gearbox/static/js/console/console.js

Lines changed: 66 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -237,6 +237,15 @@
237237
this.ws.send(JSON.stringify({ t: 'resize', cols: cols, rows: rows }));
238238
};
239239

240+
// Emit a visible error line into the xterm buffer. The status dot
241+
// also reflects the state via color, but the terminal needs its own
242+
// surface so a failing session isn't a silent black void. Matches
243+
// the server-side 'err' frame path which already writes red text.
244+
ConsoleSession.prototype._writeError = function (msg) {
245+
if (!this.term) return;
246+
this.term.write('\r\n\x1b[31m[console] ' + msg + '\x1b[0m\r\n');
247+
};
248+
240249
ConsoleSession.prototype.connect = function () {
241250
this._ensureTerm();
242251
if (this.ws) { try { this.ws.close(); } catch (_) {} }
@@ -255,6 +264,7 @@
255264
self.mode = '';
256265
self.uid = null;
257266
self.errorMsg = 'disabled on agent';
267+
self._writeError('console disabled on this agent');
258268
self._setStatus('error');
259269
return;
260270
}
@@ -264,6 +274,7 @@
264274
})
265275
.catch(function (e) {
266276
self.errorMsg = e.message;
277+
self._writeError('capabilities fetch failed: ' + e.message);
267278
self._setStatus('error');
268279
});
269280
};
@@ -304,6 +315,7 @@
304315
};
305316
ws.onerror = function () {
306317
self.errorMsg = 'socket error';
318+
self._writeError('websocket error');
307319
self._setStatus('error');
308320
};
309321
ws.onclose = function () {
@@ -399,31 +411,50 @@
399411
if (!handle) return;
400412
const self = this;
401413
let dragging = false;
414+
let pointerID = null;
402415
let startY = 0;
403416
let startH = 0;
404417

405-
handle.addEventListener('mousedown', function (e) {
418+
// Pointer Events with setPointerCapture so drag state survives
419+
// the cursor leaving the window — mouseup-outside used to leave
420+
// the dock in a stuck "still resizing" state. visibilitychange
421+
// and blur are additional belt-and-suspenders cleanups.
422+
const stopDrag = function () {
423+
if (!dragging) return;
424+
dragging = false;
425+
try { if (pointerID !== null) handle.releasePointerCapture(pointerID); } catch (_) {}
426+
pointerID = null;
427+
document.body.style.cursor = '';
428+
document.body.style.userSelect = '';
429+
prefSet('dockHeight', self.dockHeight);
430+
const s = self._active();
431+
if (s) s.fit();
432+
};
433+
434+
handle.addEventListener('pointerdown', function (e) {
406435
dragging = true;
436+
pointerID = e.pointerId;
407437
startY = e.clientY;
408438
startH = self.dockHeight;
409439
document.body.style.cursor = 'row-resize';
410440
document.body.style.userSelect = 'none';
441+
try { handle.setPointerCapture(e.pointerId); } catch (_) {}
411442
e.preventDefault();
412443
});
413-
window.addEventListener('mousemove', function (e) {
414-
if (!dragging) return;
444+
handle.addEventListener('pointermove', function (e) {
445+
if (!dragging || e.pointerId !== pointerID) return;
415446
const dy = startY - e.clientY;
416447
self.dockHeight = clampInt(startH + dy, DOCK_MIN, window.innerHeight - 100);
417448
self._applyDockHeight();
418449
});
419-
window.addEventListener('mouseup', function () {
420-
if (!dragging) return;
421-
dragging = false;
422-
document.body.style.cursor = '';
423-
document.body.style.userSelect = '';
424-
prefSet('dockHeight', self.dockHeight);
425-
const s = self._active();
426-
if (s) s.fit();
450+
handle.addEventListener('pointerup', stopDrag);
451+
handle.addEventListener('pointercancel', stopDrag);
452+
// If the user alt-tabs or the page is hidden mid-drag, treat it
453+
// as drop — without these the body cursor/userSelect overrides
454+
// can linger after the user returns.
455+
window.addEventListener('blur', stopDrag);
456+
document.addEventListener('visibilitychange', function () {
457+
if (document.hidden) stopDrag();
427458
});
428459
};
429460

@@ -665,10 +696,14 @@
665696
? s.label + ' #' + labelIdx[s.label]
666697
: s.label;
667698

699+
// Tab wrapper is layout-only — the focusable element with
700+
// role="tab" is the label button. This keeps ARIA semantics
701+
// happy: nested interactive children (dot, close) sit inside
702+
// the wrapper but are siblings of the role="tab" element,
703+
// not nested inside it. See Copilot review on PR #141.
668704
const tab = document.createElement('div');
669705
tab.className = 'console-tab' + (s.id === self.activeID ? ' active' : '');
670-
tab.setAttribute('role', 'tab');
671-
tab.setAttribute('aria-selected', s.id === self.activeID ? 'true' : 'false');
706+
tab.setAttribute('role', 'presentation');
672707

673708
// Per-tab status dot. Doubles as a reconnect button when the
674709
// session is closed/errored — clicking switches active to
@@ -678,17 +713,22 @@
678713
dot.type = 'button';
679714
dot.className = 'console-tab-dot is-' + s.status;
680715
const isReconnectable = s.status === 'closed' || s.status === 'error';
716+
const dotStateText = ({
717+
connected: 'Connected',
718+
connecting: 'Connecting',
719+
idle: 'Idle',
720+
closed: 'Closed',
721+
error: 'Error: ' + (s.errorMsg || 'unknown'),
722+
})[s.status] || s.status;
681723
if (isReconnectable) {
682724
dot.classList.add('is-clickable');
683-
dot.title = (s.status === 'error'
684-
? 'Error: ' + (s.errorMsg || 'unknown')
685-
: 'Closed') + ' — click to reconnect';
725+
dot.title = dotStateText + ' — click to reconnect';
726+
dot.setAttribute('aria-label', display + ' — ' + dotStateText + '. Click to reconnect.');
727+
dot.removeAttribute('aria-disabled');
686728
} else {
687-
dot.title = ({
688-
connected: 'Connected',
689-
connecting: 'Connecting…',
690-
idle: '',
691-
})[s.status] || s.status;
729+
dot.title = dotStateText;
730+
dot.setAttribute('aria-label', display + ' — ' + dotStateText);
731+
dot.setAttribute('aria-disabled', 'true');
692732
}
693733
dot.addEventListener('click', function (e) {
694734
e.stopPropagation();
@@ -700,6 +740,11 @@
700740
label.className = 'console-tab-label';
701741
label.textContent = display;
702742
label.title = display + ' (' + s.status + ')';
743+
// Label button carries the role="tab" semantics so screen
744+
// readers expose the tab list correctly (one role="tab" per
745+
// tab, focusable, with aria-selected reflecting active).
746+
label.setAttribute('role', 'tab');
747+
label.setAttribute('aria-selected', s.id === self.activeID ? 'true' : 'false');
703748
label.addEventListener('click', function () { self._setActive(s.id); });
704749

705750
const close = document.createElement('button');

0 commit comments

Comments
 (0)