Refresh sidebar tab overflow from intrinsic content - #1010
Conversation
|
🚨 SLOP COP 🚨 · I am SlopCop. I will review this pull request under the The stack contains these pull requests, from bottom to top:
I found no draft pull requests or authors without write access in this stack. I will review security, code quality, performance, architecture, and duplicate code. I will run one end-to-end test on #1010. |
|
🚨 SLOP COP 🚨 · I am the SlopCop. I am reviewing this pull request for security, code quality, performance, architecture, and end-to-end behavior. Stack: #1010 only. I found no later pull request after the required second scan. |
|
🚨 SLOP COP 🚨 · Security review complete. I found no security issue. The change reads layout size and updates local UI state. React still renders tab titles as text. Checks: I reviewed the exact PR diff, its callers, unsafe HTML use, script paths, message paths, and privileged APIs. |
|
🚨 SLOP COP 🚨 · Security review is complete. I found no security issue in #1010. The DOM and observer changes add no trust boundary or data-access path. |
|
🚨 SLOP COP 🚨 · Performance review is complete. I found no performance issue in #1010. The added observer has a small scope and has correct cleanup. |
|
🚨 SLOP COP 🚨 · Performance review complete. I found no performance or resource-use regression. The observer watches one viewport and one content row, independent of tab count. Layout reads occur after size changes. Scroll work remains limited to one callback per animation frame. Cleanup disconnects both observations and cancels pending frame work. Checks: the focused tests passed, the |
|
🚨 SLOP COP 🚨 · Code-quality review is complete. I found no code-quality issue in #1010. The test covers both observed elements and the observer cleanup remains correct. |
|
|
||
| it("observes the intrinsic tab row so async title changes refresh overflow", () => { | ||
| const observed: Element[] = []; | ||
| vi.stubGlobal( |
There was a problem hiding this comment.
🚨 slopcop/review — The test does not prove that title changes refresh overflow. The ResizeObserver mock ignores its callback and only records observed elements. A no-op callback would still pass this test. Capture and call the callback after a width change, then verify the right chevron state.
4442612 to
262fb6d
Compare
|
🚨 SLOP COP 🚨 · Code-quality and architecture review complete. I found one low-severity test weakness. The new test confirms both observer targets, but it does not trigger the observer callback or verify the overflow controls. I found no product correctness, duplication, or architecture issue. The focused tests, app typecheck, app lint, and |
262fb6d to
b51d9fe
Compare
|
🚨 SLOP COP 🚨 · End-to-end browser test complete on the exact head SHA. The result passed. I opened a real thread and a right-panel tab. I changed the intrinsic content-row width from 83 pixels to 1,200 pixels. The right overflow control changed from hidden to visible. I restored the intrinsic width, and the control became hidden again. The dev app ran at commit |
SawyerHood
left a comment
SawyerHood
left a comment
There was a problem hiding this comment.
🚨 SLOP COP 🚨 · review
Final review: no blocking findings.
I found one low-severity test weakness. The new test confirms the observer targets, but it does not invoke the observer callback or verify the chevron state. This does not block the change. The exact-SHA browser test verified both overflow-state transitions.
Security: no issue. Performance: no regression. Correctness, duplication, and architecture: no product issue.
Validation: the full @bb/app suite passed 311 files and 2,338 tests. The Turbo app typecheck passed. The focused test, app lint, and git diff --check passed. The final GPT-5.6 review gate returned APPROVE.
Stack: #1010 only.
SawyerHood
left a comment
SawyerHood
left a comment
There was a problem hiding this comment.
🚨 SLOP COP 🚨 · review
Current head reviewed: b51d9fe43ffaf3b88237231c0609170cfe315bee.
I found no security, code-quality, architecture, duplicate-code, or performance issue in #1010.
The content-row observer has a small scope. The existing cleanup disconnects it.
Validation on the current stack passed 43 focused app tests, 56 automation tests, and both Turbo typechecks. Most current CI checks pass.
The one allowed end-to-end run used the earlier top head. It found no failure attributable to #1010.
I used a comment-only review, as required.
6e78c7f to
3731866
Compare
b51d9fe to
4960750
Compare
3731866 to
0446050
Compare
4960750 to
9aad76f
Compare
2 participants
Keeps the existing scroll/fade/chevron interaction while observing the intrinsic tab row as well as the viewport. Async title changes now refresh scroll capacity, so start, middle, and end affordances remain accurate without breaking normal wheel scrolling or selection.\n\nFocused coverage: SecondaryPanelTabStrip.test.ts (3 tests), Turbo @bb/app typecheck, and desktop screenshots for overflow start/middle/end plus the pre-fix state.