-
Notifications
You must be signed in to change notification settings - Fork 2.9k
fix: LEAP-1931: Correct frame offset calculation in Timeline #7244
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Adjusted the frame offset check to account for `currentOffsetX`, ensuring proper behavior when determining the valid offset range.
✅ Deploy Preview for label-studio-docs-new-theme canceled.
|
✅ Deploy Preview for heartex-docs canceled.
|
hlomzik
approved these changes
Mar 19, 2025
/git merge
|
nass600
approved these changes
Mar 19, 2025
/git merge
|
/fm sync |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Reason for change:
The frame offset calculation in the Timeline component did not account for
currentOffsetX
, which led to improper determination of valid offset ranges. This caused inconsistent behavior in seeking the right video frame for displaying.Before:

Solution:
The check for frame offsets was adjusted to include
currentOffsetX
. This ensures accurate calculations and resolves the inconsistency.After:

Rollout strategy:
This code will be rolled out as part of the next release and does not require feature flags or environment variables.
Testing:
Risks:
This change modifies a critical calculation in the Timeline. There is a minor risk of unforeseen edge cases that might have been missed.
Reviewer Notes:
From the code perspective
offset
contains the value of the current mouse x coordinate related to the timeline itself pluscurrentOffsetX
which is basicallyscrollLeft
. Although it's unclear why that method was calledgetMouseToFrame
, it's pretty clear that the next condition was working fine only when the timeline's frame is placed at the left edge.