Repository navigation
feat(activity-feed): open the version a timestamped comment was made on and seek to it - #4918
Conversation
…on and seek to it Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
WalkthroughComment timestamps now retain their file-version IDs. The activity feed routes timestamp selection and markers to the matching preview pane or file version. Sidebar navigation passes version selections to ChangesVersion-aware comment timestamps
Priority: ➖ Normal Estimated code review effort: 4 (Complex) | ~45 minutes Change: Feature Sequence Diagram(s)sequenceDiagram
participant FeedItemRow
participant ActivityFeedV2
participant ComparedPreview
participant ActivitySidebar
participant ContentPreview
FeedItemRow->>ActivityFeedV2: Select comment timestamp and file version
ActivityFeedV2->>ComparedPreview: Seek when the timestamp version matches the compared pane
ActivityFeedV2->>ActivitySidebar: Request another version when it is represented in the feed
ActivitySidebar->>ContentPreview: Pass the selected version and timestamp
Suggested reviewers: Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 16 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Title check The title clearly describes the main change: opening the version associated with a timestamped comment and seeking to its timestamp. Description check The description is complete and relevant. It explains the behavior, implementation changes, limitations, and test coverage. The repository template contains guidance only and does not require addition… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Description check
-
Explanation
The description is complete and relevant. It explains the behavior, implementation changes, limitations, and test coverage. The repository template contains guidance only and does not require additional sections.
-
✨ Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Warning
Some tools did not complete. Review the errors below.
🔧 ast-grep (0.45.3)
-
src/elements/content-preview/__tests__/ContentPreview.test.js
-
ast-grep timed out on this file
-
🔧 Biome (2.5.15)
-
src/elements/content-preview/ContentPreview.js
-
File contains syntax errors that prevent linting: Line 904: Illegal return statement outside of a function; Line 37: 'import { type x ident }' are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.; Line 68: 'import type' are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.; Line 69: 'import type' are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.; Line 70: 'import type' are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.; Line 71: 'import type' are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.; Line 72: 'import type' are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.; Line 73: 'import type' are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.; Line 74: 'import type' are a TypeScript only feature. Convert your f
... [truncated 19228 characters] ...
)
but instead found:; Line 1886: Expected a JSX attribute but instead found ')'.; Line 1884: Illegal return statement outside of a function; Line 1886: Unexpected token. Did you mean{'}'}or}?; Line 1886: Unexpected token. Did you mean{'>'}or>?; Line 2006: Expected a statement but instead found '}'.; Line 2009: 'export type' declarations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.; Line 2027: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.; Line 2029: Expected an expression but instead found '?'.; Line 2029: expected:but instead found;`; Line 2030: Expected an expression but instead found '?'.; Line 907: Illegal return statement outside of a function
src/elements/content-sidebar/ActivitySidebar.js
-
File contains syntax errors that prevent linting: Line 1211: Unexpected token. Did you mean
{'>'}or>?; Line 423: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.; Line 423: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.; Line 423: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.; Line 423: return type annotation are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.; Line 444: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.; Line 444: return type annotation are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.; Line 462: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.; Line 462: return type annotation are... [truncated 27672 characters] ...
feature. Convert your file to a TypeScript file or remove the syntax.; Line 399: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.; Line 399: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.; Line 399: expected
,but instead found?; Line 399: expected,but instead foundFunction; Line 399: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.; Line 399: expected,but instead found?; Line 399: expected,but instead foundFunction; Line 399: return type annotation are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.; Line 1393: expected}but instead found;
src/elements/content-sidebar/ContentSidebar.js
-
File contains syntax errors that prevent linting: Line 179: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.; Line 39: 'import type' are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.; Line 40: 'import type' are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.; Line 41: 'import type' are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.; Line 42: 'import type' are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.; Line 43: 'import type' are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.; Line 44: 'import type' are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.; Line 45: 'import type' are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.; Line 46: 'import t
... [truncated 4077 characters] ...
cript only feature. Convert your file to a TypeScript file or remove the syntax.; Line 351: type annotation are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.; Line 369: return type annotation are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.; Line 421: type annotation are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.; Line 422: type annotation are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.; Line 433: expected
)but instead found:; Line 433: Unexpected token. Did you mean{'}'}or}?; Line 222: return type annotation are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
- 2 others
-
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.
A rabbit taps a timestamp bright,
And hops to frames in seconds’ flight.
The markers find their version’s place,
Two preview panes keep time apace.
The burrow hums; the comments play,
Then carrots celebrate the day!
Comment @coderabbitai help to get the list of available commands.
Merge Queue Status
This pull request spent 4 hours 14 minutes 56 seconds in the queue, including 2 seconds running CI. Required conditions to merge
|
Summary
Clicking the timestamp on a comment made on a previous version of an audio or video file now opens that version and starts playback at the comment's timestamp. Before, it seeked within whichever version was on screen.
Timestamped comments already record their version in the markup (
#[timestamp:ms,endTimestamp:ms,versionId:id]). This PR reads that version id and uses it in the activity feed and in preview.Changes
Opening a comment's version
extractTimestampMarkupreturns the comment'sversionId, and the transformer exposes it asannotationTimestampVersionId.ActivityFeedV2calls a newonCommentVersionSelectprop.ActivitySidebarroutes to that version the same way it opens an annotation from another version: through the annotations path, or throughinternalSidebarNavigationHandlerwhen the router is disabled. It then passes the timestamp up.ContentPreviewsetsstartAtfor the next load, so the version opens at the timestamp.Clearing
startAtContentPreviewnow clearsstartAtafter one load, and whenever the file changes, so a later load (including the annotation path) doesn't reuse an old seek.Markers per version
ActivityFeedV2into auseCommentMarkerViewerhook. The hook re-attaches when Preview replaces the viewer for another version, and detaches from destroyed viewers.Side-by-side compare
getComparedPreviewprop. It's passed only while comparing.Previous version on screen
Known limitations
Testing
startAtafter one load and when the file changesstartAtwhile comparingSummary by CodeRabbit