feat: add pointer drag seeking to section map - #11
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe Section Map now supports pointer-based drag previews and seeking, including a time label, pointer capture, click suppression, cancellation, and screen-change cleanup. CommonJS test hooks expose the new handlers and drag state, with expanded behavioral coverage. ChangesSection Map Drag Interaction
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant Pointer
participant SectionMap
participant FeedBack
Pointer->>SectionMap: Drag across section map
SectionMap->>SectionMap: Update marker and time label preview
Pointer->>SectionMap: Release pointer
SectionMap->>FeedBack: Seek with sectionmap-drag
FeedBack-->>SectionMap: Continue playback marker updates
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
screen.js (1)
233-245: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueOptional: cache the label width for the duration of the drag.
label.offsetWidthis read on everypointermove, forcing a layout each frame on a touch hot path. The label text width does change as the time string changes, but you could measure once per drag (or only whentextContentchanges) instead.🤖 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 `@screen.js` around lines 233 - 245, Optimize _smPreviewTimeLabel by avoiding a fresh label.offsetWidth layout read on every pointermove: cache the measured width for the drag, or invalidate and remeasure only when the displayed text changes. Ensure the cached measurement is reset when a new drag begins and preserve the existing clamping and positioning behavior.
🤖 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 `@tests/screen.test.js`:
- Around line 310-334: Update the move coordinates in the vertical pointer
movement test around _smOnPointerMove so horizontal displacement exceeds
SM_DRAG_THRESHOLD_PX while vertical displacement remains larger than horizontal.
Keep the assertions unchanged so the test specifically exercises the axis-lock
branch rather than only the drag-threshold guard.
---
Nitpick comments:
In `@screen.js`:
- Around line 233-245: Optimize _smPreviewTimeLabel by avoiding a fresh
label.offsetWidth layout read on every pointermove: cache the measured width for
the drag, or invalidate and remeasure only when the displayed text changes.
Ensure the cached measurement is reset when a new drag begins and preserve the
existing clamping and positioning behavior.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 80c5c630-6e9c-4ab5-8753-43dd3d66a02f
📒 Files selected for processing (2)
screen.jstests/screen.test.js
Signed-off-by: saleemk <saleem@shadowforge.co.za>
3cbea12 to
8a77ddc
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
What
Add Pointer Events drag seeking to Section Map for touch, pen, and mouse input.
m:ss / m:sslabel shows the target timeWhy
Section Map currently provides discrete click and tap seeking. Adding drag preview and release-to-seek makes precise navigation easier across pointer input types.
Keeping this behavior inside Section Map also ensures that the plugin owning the control owns its interaction lifecycle, without requiring another plugin to intercept or recreate its behavior.
Testing
Automated:
node --check screen.jsnode --test tests/screen.test.jsManual:
Summary by CodeRabbit
New Features
Bug Fixes
Tests