Conversation
…w touch drags A button resolves `width: auto` to fit-content rather than filling its containing block, so `Sidebar.Button appearance="solid"` rendered at its text width and sat at the left edge of the rail with up to 80px of dead space beside it. Sidebar.Link already carried `w-full` for this reason; the solid branch had no width at all. It takes `w-[calc(100%-16px)]` rather than `w-full` because the solid treatment also carries `mx-2`. The resize handle had no `touch-action`. A touch drag was claimed by the browser as a scroll and arrived as `pointercancel`, so dragging the rail did nothing on a touchscreen. `preventDefault` in the pointerdown handler does not suppress that; only the CSS property does. resolveCrossingWidth guarded the downward crossing with `hasIntentBelow` but not the upward one, so rotating a tablet from portrait to landscape replaced a width the user had just dragged with the remembered expanded width. Both directions now honour the flag.
commit: |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (4)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. Summary
Verification
WalkthroughThe sidebar updates its solid button width, prevents touch gesture cancellation during resizing, and preserves user-selected widths when crossing the responsive threshold. ChangesSidebar behavior
Priority: ⬇️ Low — Impact reflects low issue severity. Severity of issue fixed: Low Merge Risk: ⚪ Minimal · up to Solid sidebar buttons now fill the rail within their margins, touch resizing avoids browser scroll cancellation, and user-selected widths persist across responsive threshold changes. No current merge-blocking risk remains. Warning Some tools did not complete. Review the errors below. 🔧 ESLint
ESLint install timed out. The project may have too many dependencies for the sandbox. Comment |
1 participant
Follow-up to #448. Three defects in the sidebar surfaced while reviewing x2-seller#6897, which worked around all of them in the consumer.
Solid button never filled the rail
A
<button>resolveswidth: autoto fit-content rather than filling its containing block.Sidebar.Linkcarriesw-fullfor that reason; thesolidbranch ofSidebar.Buttonhad no width at all, so it rendered at its text width and sat at the left edge with dead space beside it.Measured against a 188px containing block, before this change:
Takes
w-[calc(100%-16px)]rather thanw-fullbecause the solid treatment also carriesmx-2, and 16px is that margin on the config's px spacing scale.Resize handle ignored touch
The handle had no
touch-action, so a touch drag was claimed by the browser as a scroll and arrived aspointercancel. Dragging the rail did nothing on a touchscreen.preventDefaultin the pointerdown handler does not suppress that; only the CSS property does.finishResizealready handled the cancel path, so the gesture failed silently rather than latching.Rotation discarded a chosen width
resolveCrossingWidthguarded the downward crossing withhasIntentBelowbut not the upward one. Dragging the rail to 120px in portrait and rotating to landscape replaced it withlastExpandedWidth. Both directions now honour the flag, covered by a new case insidebarWidth.test.js.Verification
34 tests pass in
sidebarWidth.test.js, eslint clean,npm run buildclean. The rail-fill and centring numbers above come from measuring the real component in Chrome at each band.