feat(content-sidebar): add drag-to-resize handle behind feature flag#4559
feat(content-sidebar): add drag-to-resize handle behind feature flag#4559jmcbgaston wants to merge 3 commits into
Conversation
Adds a pointer- and keyboard-accessible resize handle on the left edge of the sidebar. Current default width becomes the minimum; maximum is clamped at 50% of the viewport. Width state lives on the `Sidebar` component and is session-only (no persistence). The new `SidebarResizeHandle` component is a `role="separator"` with live `aria-valuenow` / `aria-valuemin` / `aria-valuemax`, supports ArrowLeft / ArrowRight / Home / End for keyboard resize, and uses pointer capture during drag. Gated by `isFeatureEnabled(features, 'contentSidebar.resizable.enabled')` and additionally restricted to large / x-large viewports via the existing `withMediaQuery` HOC — small / medium viewports keep the original bottom-sheet layout. SCSS overrides for `.bcs-content` and `.bcs-activity-feed` are scoped under `.bcs-is-resizable` so flag-off behavior is unchanged.
WalkthroughAdds a pointer-driven SidebarResizeHandle, wires resizable behavior behind a feature flag and viewport check in Sidebar, stores/clamps user width in state, and updates SCSS so sidebar content and activity feed fill the resized container. ChangesResizable Sidebar Feature
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/elements/content-sidebar/SidebarResizeHandle.js`:
- Around line 39-64: The Flow errors come from TypeScript event types and
optional-call syntax; update handlePointerUp, handlePointerDown and
handleKeyDown to use Flow's SyntheticPointerEvent and SyntheticKeyboardEvent
types (instead of React.PointerEvent/React.KeyboardEvent) and replace every
optional-call like target.hasPointerCapture?.(...) and
event.currentTarget.setPointerCapture?.(...) and onResizeStart?.() /
onResizeEnd?.() with explicit null/undefined checks (e.g., if (target && typeof
target.hasPointerCapture === 'function')
target.hasPointerCapture(event.pointerId); and if (onResizeStart)
onResizeStart();). Ensure startXRef/current, startWidthRef/current,
setIsDragging, handlePointerMove remain unchanged but used with the new
Flow-compatible event types.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 02b0b1ec-8256-4d64-b578-9830c44e20d1
📒 Files selected for processing (6)
src/elements/content-sidebar/ContentSidebar.scsssrc/elements/content-sidebar/Sidebar.jssrc/elements/content-sidebar/SidebarContent.scsssrc/elements/content-sidebar/SidebarResizeHandle.jssrc/elements/content-sidebar/SidebarResizeHandle.scsssrc/elements/content-sidebar/activity-feed/activity-feed/ActivityFeed.scss
- Make `maxWidth` reactive by reading `viewWidth` from withMediaQuery (previously read `window.innerWidth` at render time, leaving the cap stale across browser resizes) - Drop unused `onResizeStart` / `onResizeEnd` props from `SidebarResizeHandle` — no caller passes them - Simplify SCSS selectors: `.bcs-is-resizable.bcs-is-wider .bcs-content` was redundant with `.bcs-is-resizable .bcs-content` (same specificity, source order wins) - Replace TS-style React event types with Flow's Synthetic* and replace optional-chain calls with explicit guards so Flow typecheck passes - Strip keyboard interaction for MVP; mark the handle `aria-hidden="true"` so AT users aren't shown an unfocusable widget - Add `SidebarResizeHandle.test.js` covering render, drag-grow, drag-shrink, min/max clamping, dragging class, and listener cleanup - Add a `resizable` describe block to `Sidebar.test.js` covering FF gate, viewport gate, open-state gate, inline-width application, and maxWidth-from-viewport behavior
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/elements/content-sidebar/SidebarResizeHandle.js`:
- Around line 58-60: Flow is complaining because setPointerCapture (called on
event.currentTarget in SidebarResizeHandle.js) is typed to accept a string while
event.pointerId is a number; fix by adding a narrow type assertion/cast so Flow
accepts the runtime numeric pointerId (e.g. coerce event.pointerId to any or the
appropriate expected type) before calling event.currentTarget.setPointerCapture,
leaving the runtime value unchanged and keeping the guard that setPointerCapture
exists.
- Around line 38-46: The runtime guards using typeof on event.target should be
replaced by an instanceof Element check to satisfy Flow's typing: in the pointer
event handler inside SidebarResizeHandle (where event.target, event.pointerId,
hasPointerCapture and releasePointerCapture are used), first assert that
event.target instanceof Element, then call
target.hasPointerCapture(event.pointerId) and
target.releasePointerCapture(event.pointerId) — removing the typeof checks so
Flow knows target is an Element before calling those methods.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 07925335-2612-4e8b-9552-cd6af9208e25
📒 Files selected for processing (7)
src/elements/content-sidebar/Sidebar.jssrc/elements/content-sidebar/SidebarContent.scsssrc/elements/content-sidebar/SidebarResizeHandle.jssrc/elements/content-sidebar/SidebarResizeHandle.scsssrc/elements/content-sidebar/__tests__/Sidebar.test.jssrc/elements/content-sidebar/__tests__/SidebarResizeHandle.test.jssrc/elements/content-sidebar/activity-feed/activity-feed/ActivityFeed.scss
✅ Files skipped from review due to trivial changes (1)
- src/elements/content-sidebar/SidebarContent.scss
| const {target} = event; | ||
| if ( | ||
| target && | ||
| typeof target.hasPointerCapture === 'function' && | ||
| typeof target.releasePointerCapture === 'function' && | ||
| target.hasPointerCapture(event.pointerId) | ||
| ) { | ||
| target.releasePointerCapture(event.pointerId); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check Flow's type definitions for pointer capture APIs
rg -A5 "hasPointerCapture|releasePointerCapture" flow-typed/ node_modules/flow-bin/Repository: box/box-ui-elements
Length of output: 126
🏁 Script executed:
#!/bin/bash
# First, locate the file and examine its current state
find . -name "SidebarResizeHandle.js" -type fRepository: box/box-ui-elements
Length of output: 117
🏁 Script executed:
#!/bin/bash
# Check Flow configuration and type definitions
git ls-files | grep -E "flow|\.flowconfig" | head -20Repository: box/box-ui-elements
Length of output: 802
🏁 Script executed:
#!/bin/bash
# Look for Flow type definitions related to pointer events and Element
rg -l "PointerEvent|hasPointerCapture" --type ts --type tsx --type js --type jsx | head -20Repository: box/box-ui-elements
Length of output: 91
🏁 Script executed:
#!/bin/bash
# Examine the actual SidebarResizeHandle.js file
cat -n src/elements/content-sidebar/SidebarResizeHandle.jsRepository: box/box-ui-elements
Length of output: 3548
🏁 Script executed:
#!/bin/bash
# Check Flow configuration
cat -n .flowconfigRepository: box/box-ui-elements
Length of output: 3768
🏁 Script executed:
#!/bin/bash
# Search for PointerEvent type definitions in Flow typed files
rg "PointerEvent" flow-typed/Repository: box/box-ui-elements
Length of output: 45
🏁 Script executed:
#!/bin/bash
# Look for PointerEvent and Element type definitions in Flow stubs
rg "Element|PointerEvent" flow-typed/ --type jsRepository: box/box-ui-elements
Length of output: 2939
🏁 Script executed:
#!/bin/bash
# Check the React synthetic event type definitions
rg "SyntheticPointerEvent" . --type js -B 2 -A 5 | head -40Repository: box/box-ui-elements
Length of output: 803
🏁 Script executed:
#!/bin/bash
# Search for other uses of pointer capture methods in the codebase
rg "hasPointerCapture|releasePointerCapture|setPointerCapture" . --type js -B 2 -A 2Repository: box/box-ui-elements
Length of output: 1568
🏁 Script executed:
#!/bin/bash
# Check if there are any Flow suppression comments in this file
grep -n "FlowFixMe\|flowfixme\|TODO.*Flow" src/elements/content-sidebar/SidebarResizeHandle.jsRepository: box/box-ui-elements
Length of output: 45
🏁 Script executed:
#!/bin/bash
# Verify Flow's built-in type definitions for Element and EventTarget
# Check if Flow includes Pointer Capture in the standard library
rg "hasPointerCapture" . --type js --type flow 2>/dev/null | grep -v node_modules | head -20Repository: box/box-ui-elements
Length of output: 45
Use instanceof Element check instead of typeof guards for pointer capture methods.
Flow's type system correctly identifies that EventTarget (the type of event.target in a PointerEvent) doesn't have hasPointerCapture and releasePointerCapture methods—these exist only on Element. The current typeof checks work at runtime but don't satisfy Flow's static type analysis.
Replace the runtime guards with an instanceof Element check to properly narrow the type:
Proposed fix
const handlePointerUp = React.useCallback(
(event: PointerEvent) => {
setIsDragging(false);
- const {target} = event;
+ const target = event.target;
if (
- target &&
- typeof target.hasPointerCapture === 'function' &&
- typeof target.releasePointerCapture === 'function' &&
+ target instanceof Element &&
target.hasPointerCapture(event.pointerId)
) {
target.releasePointerCapture(event.pointerId);
}
window.removeEventListener('pointermove', handlePointerMove);
window.removeEventListener('pointerup', handlePointerUp);
},
[handlePointerMove],
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const {target} = event; | |
| if ( | |
| target && | |
| typeof target.hasPointerCapture === 'function' && | |
| typeof target.releasePointerCapture === 'function' && | |
| target.hasPointerCapture(event.pointerId) | |
| ) { | |
| target.releasePointerCapture(event.pointerId); | |
| } | |
| const target = event.target; | |
| if ( | |
| target instanceof Element && | |
| target.hasPointerCapture(event.pointerId) | |
| ) { | |
| target.releasePointerCapture(event.pointerId); | |
| } |
🧰 Tools
🪛 CircleCI: flow
[error] 41-41: Flow error: Property hasPointerCapture is missing in EventTarget [1]. (Property hasPointerCapture is missing in EventTarget)
[error] 42-42: Flow error: Property releasePointerCapture is missing in EventTarget [1]. (Property releasePointerCapture is missing in EventTarget)
[error] 43-43: Flow error: Cannot call target.hasPointerCapture because the parameter types of an unknown function [1] are unknown. (Cannot call target.hasPointerCapture...)
[error] 45-45: Flow error: Cannot call target.releasePointerCapture because the parameter types of an unknown function [1] are unknown. (Cannot call target.releasePointerCapture...)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/elements/content-sidebar/SidebarResizeHandle.js` around lines 38 - 46,
The runtime guards using typeof on event.target should be replaced by an
instanceof Element check to satisfy Flow's typing: in the pointer event handler
inside SidebarResizeHandle (where event.target, event.pointerId,
hasPointerCapture and releasePointerCapture are used), first assert that
event.target instanceof Element, then call
target.hasPointerCapture(event.pointerId) and
target.releasePointerCapture(event.pointerId) — removing the typeof checks so
Flow knows target is an Element before calling those methods.
| if (typeof event.currentTarget.setPointerCapture === 'function') { | ||
| event.currentTarget.setPointerCapture(event.pointerId); | ||
| } |
There was a problem hiding this comment.
Flow type error: pointerId type mismatch.
Flow reports that setPointerCapture expects a string but event.pointerId is a number. This is a Flow type definition issue – the DOM PointerEvent API uses numeric pointerIds, but Flow's definition may be incorrect or outdated.
🔧 Proposed fix using type assertion
const handlePointerDown = (event: SyntheticPointerEvent<HTMLDivElement>) => {
event.preventDefault();
startXRef.current = event.clientX;
startWidthRef.current = width;
setIsDragging(true);
if (typeof event.currentTarget.setPointerCapture === 'function') {
- event.currentTarget.setPointerCapture(event.pointerId);
+ event.currentTarget.setPointerCapture((event.pointerId: any));
}
window.addEventListener('pointermove', handlePointerMove);
window.addEventListener('pointerup', handlePointerUp);
};Verify the correct type for pointerId in Flow:
#!/bin/bash
# Check Flow's PointerEvent type definition
rg -A10 "type.*PointerEvent" flow-typed/ node_modules/flow-bin/ | rg "pointerId"🧰 Tools
🪛 CircleCI: flow
[error] 59-59: Flow error: Cannot call event.currentTarget.setPointerCapture with event.pointerId; number is incompatible with string. (Cannot call setPointerCapture...)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/elements/content-sidebar/SidebarResizeHandle.js` around lines 58 - 60,
Flow is complaining because setPointerCapture (called on event.currentTarget in
SidebarResizeHandle.js) is typed to accept a string while event.pointerId is a
number; fix by adding a narrow type assertion/cast so Flow accepts the runtime
numeric pointerId (e.g. coerce event.pointerId to any or the appropriate
expected type) before calling event.currentTarget.setPointerCapture, leaving the
runtime value unchanged and keeping the guard that setPointerCapture exists.
1 participant
Description
Adds a pointer-driven drag-to-resize handle on the left edge of the
ContentSidebar. The current default width becomes the minimum; users can grow the sidebar up to 50% of the viewport. Width is session-only (no persistence) — resets on reload.The new
SidebarResizeHandleis a mouse-only widget markedaria-hidden="true"(keyboard support deferred to a follow-up). It uses pointer capture during drag.Gating:
isFeatureEnabled(features, 'contentSidebar.resizable.enabled')— flag-off is a no-oplarge/xlargeviewports via the existingwithMediaQueryHOC. Small / medium viewports keep the original bottom-sheet layout..bcs-contentand.bcs-activity-feedare scoped under.bcs-is-resizableso flag-off behavior is unchanged.Reflow of the preview viewer happens automatically via the existing
react-measure→preview.resize()chain — no new wiring needed.Test plan
features.contentSidebar.resizable.enabled = true, the handle renders on the left edge of the sidebarfeatures.contentSidebar.resizable.enabled = false, no handle renders and sidebar widths are unchanged≤ 767pxviewport, no handle renders and sidebar collapses to bottom-sheet layoutSemantic release type
feat- New feature