Skip to content

Commit 848b7bc

Browse files
sarg3ntclaude
andcommitted
fix: address Copilot review findings on PR #70
- handler.go: /bx clears the active-box cookie whenever the cookie is present, not only when it resolves to an enabled box. Otherwise a stale cookie (referencing a deleted/disabled box) survives and the first-login auto-select stays blocked because hasCookieSet remains true. - base.templ: encodeGearsJSON now filters out disabled gears server- side, matching its own comment. Reduces payload and keeps the command palette catalog free of empty entries. - bx/pages.templ: drop the duplicate <link rel=stylesheet> for datagrid.css — layouts.Base already includes it globally. Add aria-label="Filter boxes view" to #bx-view-filter so screen readers announce it unambiguously. - shortcut-help.js: SHORTCUTS entries gain an explicit `join` field ("+" for chord, "then" for sequence, "/" for either-of). The old separator logic always emitted a single space regardless of intent — chords like Cmd+K rendered as "Cmd K". Also extend the Esc- overlay-exemption check to cover the narrow-viewport Filters sheet (.filters-open on #header-page-content) and the sidebar's right- click context menu — pressing Esc to close those used to fall through to history.back(). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent aef20f9 commit 848b7bc

4 files changed

Lines changed: 36 additions & 12 deletions

File tree

gearbox/internal/framework/handler/handler.go

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -384,12 +384,17 @@ func (h *Handler) InjectIntegrationStatus(next http.Handler) http.Handler {
384384
}
385385
}
386386
if r.URL.Path == "/bx" || r.URL.Path == "/bx/" {
387-
// /bx is the all-boxes view — clear any sticky selection so the
388-
// chip reads "All boxes" and the sidebar hides box-scoped gears.
389-
if activeBox != nil {
387+
// /bx is the all-boxes view — clear any sticky selection so
388+
// the chip reads "All boxes" and the sidebar hides box-scoped
389+
// gears. Clear whenever the cookie is present, not just when
390+
// it resolved to an enabled box: otherwise a stale cookie
391+
// (referencing a deleted/disabled box) survives indefinitely
392+
// AND blocks the first-login auto-select branch below
393+
// because hasCookieSet stays true.
394+
if hasCookieSet {
390395
clearActiveBoxCookie(w, r)
391-
activeBox = nil
392396
}
397+
activeBox = nil
393398
}
394399
if activeBox != nil {
395400
ctx = auth.SetSelectedBox(ctx, activeBox)

gearbox/internal/framework/templates/layouts/base.templ

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,9 @@ func encodeGearsJSON(ctx context.Context) string {
9494
integrations, ok := auth.GetGearOrderFromContext(ctx)
9595
if ok {
9696
for _, i := range integrations {
97+
if !i.Enabled {
98+
continue
99+
}
97100
if !canViewIntegration(ctx, i.Name) {
98101
continue
99102
}

gearbox/internal/gears/bx/pages.templ

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@ type IndexPageData struct {
3737

3838
templ IndexPage(d IndexPageData) {
3939
@layouts.Base("Boxes", d.User, "/bx") {
40-
<link rel="stylesheet" href="/static/css/components/datagrid.css"/>
4140
<div class="px-6 py-6">
4241
@pageHeader(d.Total)
4342
if d.Total == 0 {
@@ -78,6 +77,7 @@ templ pageHeader(total int) {
7877
</div>
7978
<select
8079
id="bx-view-filter"
80+
aria-label="Filter boxes view"
8181
class="datagrid-view-filter"
8282
onchange="window.bxOnViewFilterChange && window.bxOnViewFilterChange(this.value)"
8383
>

gearbox/static/js/common/shortcut-help.js

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,17 @@
1212
(function () {
1313
'use strict';
1414

15+
// `join` controls the visual separator between kbd chips for
16+
// multi-key shortcuts: '+' for a chord (press together), 'then' for
17+
// a sequence (press in order), '/' for "either of these keys".
18+
// Single-key rows leave `join` unset and render no separator.
1519
const SHORTCUTS = [
1620
{ keys: ['?'], label: 'Show this help' },
17-
{ keys: ['Cmd/Ctrl', 'K'], label: 'Open command palette' },
18-
{ keys: ['g', 'b'], label: 'Switch box (palette)' },
21+
{ keys: ['Cmd/Ctrl', 'K'], label: 'Open command palette', join: '+' },
22+
{ keys: ['g', 'b'], label: 'Switch box (palette)', join: 'then' },
1923
{ keys: ['/'], label: 'Focus the page search input' },
2024
{ keys: ['Esc'], label: 'Close dialog · or blur input · or go back' },
21-
{ keys: ['↑', '↓'], label: 'Navigate dialog items' },
25+
{ keys: ['↑', '↓'], label: 'Navigate dialog items', join: '/' },
2226
{ keys: ['↵'], label: 'Select highlighted item' },
2327
{ keys: ['Tab'], label: 'Cycle focus within a dialog' },
2428
{ keys: ['Ctrl+↵'], label: 'In palette: switch box but keep palette open' },
@@ -42,11 +46,10 @@
4246
const keys = document.createElement('span');
4347
keys.className = 'flex items-center gap-1';
4448
s.keys.forEach(function (k, i) {
45-
if (i > 0 && (s.keys.length > 1)) {
46-
// Inter-key separator: "+" for chord, " then " for sequence.
49+
if (i > 0 && s.join) {
4750
const sep = document.createElement('span');
48-
sep.className = 'text-[10px] text-white/40 px-0.5';
49-
sep.textContent = (k === '↓' || k === '↑' || /^[A-Z]$/.test(s.keys[0])) ? ' ' : ' ';
51+
sep.className = 'text-[10px] text-white/50 px-0.5';
52+
sep.textContent = s.join;
5053
keys.appendChild(sep);
5154
}
5255
const kbd = document.createElement('kbd');
@@ -130,6 +133,19 @@
130133
const el = document.getElementById(ids[i]);
131134
if (el && !el.classList.contains('hidden')) return true;
132135
}
136+
// Narrow-viewport Filters sheet popped out under the header
137+
// (`.filters-open` on #header-page-content). Treat as an
138+
// overlay so pressing Esc to close it doesn't fall through
139+
// to history.back().
140+
const headerContent = document.getElementById('header-page-content');
141+
if (headerContent && headerContent.classList.contains('filters-open')) {
142+
return true;
143+
}
144+
// Right-click "Reorder gears" context menu on the sidebar.
145+
const ctxMenu = document.getElementById('sidebar-context-menu');
146+
if (ctxMenu && !ctxMenu.classList.contains('hidden')) {
147+
return true;
148+
}
133149
return false;
134150
}
135151
function isBlurrableTarget(el) {

0 commit comments

Comments
 (0)