Show thread numbers in sidebar - #1174
Conversation
af1c0ab to
ef531f1
Compare
ef531f1 to
7b3937f
Compare
| useLayoutEffect(() => { | ||
| const sidebar = sidebarRef.current; | ||
| if (!sidebar) return; | ||
| return observeSidebarThreadShortcutTargets( |
There was a problem hiding this comment.
🚨 slopcop/review — Avoid the observer when both thread displays are inactive.
Please observe only when numbers are enabled or the command modifier is held. The default state still scans every sidebar child-list change. Target changes also replace the context map and render all mounted rows with unchanged output. Clear the assignments and target reference when this observer stops.
There was a problem hiding this comment.
🚨 SLOP COP 🚨 · review
ELI5 summary
This change adds an optional number beside each of the first nine sidebar threads. The numbers match the existing keyboard shortcuts.
Findings
-
Medium priority: the app always observes the full sidebar subtree. It does this when numbers are off and no modifier key is held. Each child change scans the subtree. It can also render every mounted thread row again. I left an inline comment with a focused fix.
-
Merge blocker: current
mainalready contains0093_peaceful_thing. This pull request adds a different migration at index0093. GitHub reports a merge conflict. Rebase the branch, then use Drizzle to generate migration0094, its snapshot, and its journal entry.
I found no security issue. I found no duplicate implementation. The shared sidebar shortcut module is the correct location for this logic.
Checks
- All 116 focused app, CLI, database, and server tests passed.
- Type checks passed for the app, CLI, server, database, and domain packages.
- Browser tests showed no numbers by default.
- The setting showed exactly
1through9for ten threads. - Control+1 opened the first numbered thread.
- The CLI disabled the setting, and the open app removed the numbers.
- GitHub CI passed against the old base.
I posted this as a comment review. I did not approve or request changes.
7b3937f to
13a92eb
Compare
Closes #1243.
Summary
Thread numbersoption to the sidebar display menu1–9prefixes beside the first nine rendered thread rowsUser impact
Thread numbers stay hidden by default. A user can open the sidebar display menu and select
Thread numbers. The choice applies to that browser instead of every bb client.Testing
pnpm exec turbo run test --filter=@bb/app -- --run src/components/sidebar/ThreadRow.test.tsx src/components/sidebar/sidebarThreadShortcuts.test.ts src/components/sidebar/SidebarViewOptionsMenu.test.tsxpnpm exec turbo run typecheck --filter=@bb/apppnpm exec turbo run lint --filter=@bb/app(passes with existing warnings)git diff --check