Fix mobile sidebar swipe over timeline prose - #1026
Conversation
|
🚨 SLOP COP 🚨 · I am the SlopCop. I am reviewing this pull request under the configured security, code quality, performance, architecture, and end-to-end rules. Stack: #1026 only. I will post a comment after each review phase. I will not approve this pull request or request changes. |
|
🚨 SLOP COP 🚨 · Performance phase complete. No confirmed performance finding exists. The new global Checks: diff inspection, gesture hot-path review, listener lifecycle review, retained-reference review, and |
|
🚨 SLOP COP 🚨 · Security phase complete. No security finding exists. The selection check reads only DOM endpoints. It does not read or transmit selected text. Message HTML still passes through the existing sanitizer, and the document listeners have correct cleanup. Checks: diff inspection, pointer and touch flow review, sanitization boundary review, listener cleanup review, and unsafe HTML path search. |
|
🚨 SLOP COP 🚨 · Code quality and architecture phase complete. Medium finding: a late The new test covers selection before the first move. It does not cover The repository has other selection listeners, but they serve different policies. A shared refactor would not improve this change. |
|
🚨 SLOP COP 🚨 · End-to-end phase complete on the top stack state. I ran the dev app at SHA The browser also confirmed the medium finding. After The sidebar therefore stays partly open after the cancellation path. |
| selectionRoot !== undefined && | ||
| hasExpandedTextSelectionWithin(selectionRoot) | ||
| ) { | ||
| clearSwipeSession(); |
There was a problem hiding this comment.
🚨 slopcop/review — A late text selection can leave the mobile drawer partly open.
A qualified move already opens the drawer and applies drag styles. This call removes the move and end listeners, but it does not close the drawer, settle it, or clear those styles. The browser reproduced the sequence touchstart → qualified touchmove → expanded selection → selectionchange → touchend; the panel stayed at translate3d(-56.25%, 0px, 0px) with transition: none.
Please settle or reset an active drag before removing its end listeners. Add a regression test with this event order and verify the drawer closes and clears all drag styles.
There was a problem hiding this comment.
🚨 SLOP COP 🚨 · review
Stack reviewed: #1026 only.
High finding: a late native text selection can leave the mobile sidebar partly open. A qualified swipe applies partial drag styles. The new cancellation path then removes the move and end listeners without closing or settling the drawer. The browser reproduced the full event order and left the panel at translate3d(-56.25%, 0px, 0px) with transition: none.
Please settle or reset an active drag before listener removal. Add a regression test with touchstart, a qualified touchmove, expanded selection, selectionchange, and touchend.
No security or performance finding exists. The repository scan found no useful shared refactor because the other selection listeners serve different policies.
Validation passed:
pnpm exec turbo run typecheck --filter=@bb/apppnpm exec turbo run test --filter=@bb/app --force: 312 files and 2,347 tests- All applicable GitHub checks
- Mobile-width dev-browser normal swipe and late-selection scenarios
- GPT-5.6 synthesis gate
This is a comment-only review under the configured rule.
2 participants
Summary
Root cause
Selectable message prose was blanket-marked with
data-no-sidebar-swipe, so most of the thread timeline was a gesture dead zone. This replaces the blanket opt-out with intent-based selection arbitration.Validation
pnpm exec turbo run test --filter=@bb/app --force(312 files, 2,347 tests)pnpm exec turbo run typecheck --filter=@bb/app